Nick color #4826

bmarty · 2021-12-31T14:23:25Z

Replacement for #2614 (see details there).

- allow changing the nick color by clicking the dispay-name in the room member detail page. - the ovirride-color can be specified as a hex string (#rrggbb) or as palette index (2) - entering an invalid color code or leaving the field blank reverts to the default hash-based nick color - the setting is stored in `account_data` as `im.vector.setting.override_colors` - future improvements / notes: - replace the text-based color entry with a proper color picker dialog - make the feature more discoverable - the color change listener is now in AppStateHandler, not sure if this is the best place - implement override color support in element-web / element-desktop, too Signed-off-by: Péter Radics <[email protected]>

…ndroid into feature/issue-2610-override-nick-color-via-user-account-data

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

Remove the click handler that opens the override color dialog from the display-name part on the member profile page, as this is not really discoverable and we have a proper menu item for it now.

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

…erge branch 'develop' of https://github.com./vector-im/element-android into feature/issue-2610-override-nick-color-via-user-account-data

…develop' of github.com.:mitchnull/element-android into feature/issue-2610-override-nick-color-via-user-account-data

…ndroid into feature/issue-2610-override-nick-color-via-user-account-data

…a-user-account-data

…ick-color-via-user-account-data [issue-2610] implement setting to override nick color

github-actions · 2021-12-31T15:20:33Z

Unit Test Results

  66 files ±0   66 suites ±0 52s ⏱️ +4s
135 tests ±0 135 ✔️ ±0 0 💤 ±0 0 ❌ ±0
418 runs ±0 418 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 02a8fd2. ± Comparison against base commit 5efe1f4.

♻️ This comment has been updated with latest results.

mitchnull · 2022-01-02T16:30:04Z

...main/java/im/vector/app/features/home/room/detail/timeline/helper/MatrixItemColorProvider.kt

+            null
+        } else {
+            try {
+                if (colorText.length == 1) {


This will break if/when the palette is extended to 16 or more colors. Maybe it would be better to just try to parse it as int, and if that works then it's a color index (from the palette) otherwise parse with Color.parseColor. Another option would be to check if the text starts with a digit to select the palette version.

I agree, but the plan is to change this dialog to do something more user friendly, so it's acceptable like that to me.

OK, cool, thanks.

being able to set the colour by index is quite cryptic, I wouldn't have expected an input with a hex string hint to also allow a 0-9 index

something a UI iteration could help to improve

Yes, this is really a hidden feature. Sort of easter egg.
I see in my dream a nice dialog with all the 8 predefined colors, a color picker and a field to manually enter a html color. Even a reset button, instead of validating a empty dialog to reset the color.
Also note that now using the word "black" or any other color that Color.parse() understand will also work, not sure for the other non Android-Element Matrix clients compatibility though.

bmarty · 2022-01-03T10:34:52Z

@ouchadam can you have a quick look on this PR please? Thanks.

ouchadam · 2022-01-05T12:15:21Z

...android/src/main/java/org/matrix/android/sdk/api/session/accountdata/UserAccountDataTypes.kt

@@ -27,4 +27,5 @@ object UserAccountDataTypes {
    const val TYPE_ALLOWED_WIDGETS = "im.vector.setting.allowed_widgets"
    const val TYPE_IDENTITY_SERVER = "m.identity_server"
    const val TYPE_ACCEPTED_TERMS = "m.accepted_terms"
+    const val TYPE_OVERRIDE_COLORS = "im.vector.setting.override_colors"


does this type need to be added to the matrix spec somewhere?

I have another PR that implements the same functionality in element-desktop (matrix-org/matrix-react-sdk#5626). As far as I understand keys under im.vector.setting are free-to-use by clients, so I don't think it's absolutely necessary to include it in the matrix spec (apart from maybe documenting its existence so that other clients can pick this up if they wish)

Yes, you are right, using domain prefixed type is fine. Nowadays we should maybe consider using io.element.prefix.
More details here: https://spec.matrix.org/latest/#namespacing

ouchadam · 2022-01-05T12:17:39Z

vector/src/main/java/im/vector/app/features/roommemberprofile/RoomMemberProfileFragment.kt

+        views.editText.setText(state.userColorOverride)
+        views.editText.hint = "#000000"
+
+        MaterialAlertDialogBuilder(requireContext())


allowing the user to provide a hex string is quite verbose, perhaps another iteration of this could use a colour wheel UI

I didn't include a full-fledged UI for this in my original PR because I didn't have the necessary expertise to do it quickly, and it wasn't clear that the PR / idea would be accepted at all (this is mostly a PoC and a working solution for us). I do agree that a proper color selector should be created for this (that can select a color from the current palette or select an arbitrary RGB color)

ouchadam · 2022-01-05T12:27:21Z

vector/src/main/java/im/vector/app/features/roommemberprofile/RoomMemberProfileViewModel.kt

+                .liveUserAccountData(UserAccountDataTypes.TYPE_OVERRIDE_COLORS)
+                .unwrap()
+                .map { it.content.toModel<Map<String, String>>() }
+                .map { userColorAccountDataContent ->


nit: could merge these maps

.map { val userColorAccountDataContent = it.content.toModel<Map<String, String>>() userColorAccountDataContent?.get(initialState.userId) }

Due to rework, thanks, will update

ouchadam · 2022-01-05T12:28:36Z

vector/src/main/res/values/strings.xml

@@ -2812,6 +2812,8 @@
    <string name="direct_room_profile_section_more_leave">Leave</string>
    <string name="room_profile_leaving_room">"Leaving the room…"</string>

+    <string name="room_member_override_nick_color">Override nick color</string>


is the nick terminology used in other places, or should this be display name?

Nick is used in the slash command /myroomnick but nowhere else AFAIK. The color will be used to tint the display name but also for the background of the default avatar. So not only the display name.
Maybe using "Override user color" is fine. Please vote with 👍 and 👎 !

no strong opinions from me on this 😄 maybe one for product cc @daniellekirkwood

ouchadam

LGTM 💯 small comment about the field UI label but we can always update it later

mitchnull and others added 24 commits January 3, 2021 18:52

[issue-2610] fix NoSuchElement error

889a9a1

[issue-2610] Merge branch 'develop' of github.com.:mitchnull/element-a…

0aee15f

…ndroid into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] Add Override Color menu item under More...

0e400dc

[issue-2610] Merge branch 'develop' of https://github.com./vector-im/e…

0109cde

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] remove extra semicolon

f7d8127

[issue-2610] remove click handler from display-name

cc15f9b

Remove the click handler that opens the override color dialog from the display-name part on the member profile page, as this is not really discoverable and we have a proper menu item for it now.

[issue-2610] change menu text to "Override nick color"

db97046

[issue-2610] Merge branch 'develop' of https://github.com./vector-im/e…

da10364

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] Merge branch 'develop' of https://github.com./vector-im/e…

6c26cfc

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] Merge branch 'develop' of https://github.com./vector-im/e…

ea01677

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] Merge branch 'develop' of https://github.com./vector-im/e…

1ec0956

…lement-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610-override-nick-color-via-user-account-data] [issue-2610] M…

1019ffe

…erge branch 'develop' of https://github.com./vector-im/element-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610-override-nick-color-via-user-account-data] Merge branch '…

40d48cc

…develop' of github.com.:mitchnull/element-android into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] Merge branch 'develop' of github.com.:mitchnull/element-a…

79a3be3

…ndroid into feature/issue-2610-override-nick-color-via-user-account-data

[issue-2610] Merge branch 'develop' of github.com.:mitchnull/element-a…

bf919b8

…ndroid into feature/issue-2610-override-nick-color-via-user-account-data

Merge branch 'develop' into feature/issue-2610-override-nick-color-vi…

454baf8

…a-user-account-data

Merge pull request #2614 from mitchnull/feature/issue-2610-override-n…

7ce9ece

…ick-color-via-user-account-data [issue-2610] implement setting to override nick color

Changelog new management

1d3cc52

Move logic to dedicated ViewModel

364457d

Code cleanup

07d2a15

Move logic to ViewModel

ddadefd

Use color parser

1cb91ca

Fix latest small bugs

a7b72ed

bmarty marked this pull request as ready for review December 31, 2021 14:23

bmarty added 2 commits December 31, 2021 15:34

Fix latest small bugs

6d8b5db

Small cleanup

96d5652

mitchnull reviewed Jan 2, 2022

View reviewed changes

bmarty requested a review from ouchadam January 3, 2022 10:34

ouchadam reviewed Jan 5, 2022

View reviewed changes

Optimize call flow

51c9c2f

ouchadam approved these changes Jan 5, 2022

View reviewed changes

bmarty and others added 2 commits January 5, 2022 16:33

Merge branch 'develop' into feature/bma/nick_color_final

608e01a

Fix compilation issue after develop being merged.

02a8fd2

bmarty merged commit 17e485f into develop Jan 5, 2022

bmarty deleted the feature/bma/nick_color_final branch January 5, 2022 21:01

mitchnull mentioned this pull request Jan 13, 2022

Add username colour override matrix-org/matrix-react-sdk#5626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nick color #4826

Nick color #4826

bmarty commented Dec 31, 2021 •

edited

Loading

github-actions bot commented Dec 31, 2021 •

edited

Loading

mitchnull Jan 2, 2022 •

edited

Loading

bmarty Jan 3, 2022

mitchnull Jan 3, 2022

ouchadam Jan 5, 2022

bmarty Jan 5, 2022

bmarty commented Jan 3, 2022

ouchadam Jan 5, 2022

mitchnull Jan 5, 2022

bmarty Jan 5, 2022

ouchadam Jan 5, 2022

mitchnull Jan 5, 2022 •

edited

Loading

ouchadam Jan 5, 2022

bmarty Jan 5, 2022

bmarty Jan 5, 2022

ouchadam Jan 5, 2022

bmarty Jan 5, 2022

ouchadam Jan 5, 2022

ouchadam left a comment

Nick color #4826

Nick color #4826

Conversation

bmarty commented Dec 31, 2021 • edited Loading

github-actions bot commented Dec 31, 2021 • edited Loading

Unit Test Results

mitchnull Jan 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty commented Jan 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitchnull Jan 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam left a comment

Choose a reason for hiding this comment

bmarty commented Dec 31, 2021 •

edited

Loading

github-actions bot commented Dec 31, 2021 •

edited

Loading

mitchnull Jan 2, 2022 •

edited

Loading

mitchnull Jan 5, 2022 •

edited

Loading