Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[issue-2610] implement setting to override nick color #2614

Conversation

mitchnull
Copy link
Contributor

@mitchnull mitchnull commented Jan 3, 2021

Implement issue element-hq/element-web#2610

  • allow changing the nick color by clicking the "Override color" menu item
    in the room member detail page.

  • the override-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 [added menu item under More]
    • 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 mitchnull@gmail.com

Screenshot:
Screenshot_20210101-003601_Element-override-color

Pull Request Checklist

  • Changes has been tested on an Android device or Android emulator with API 21
  • UI change has been tested on both light and dark themes
  • Pull request is based on the develop branch
  • Pull request updates CHANGES.md
  • Pull request includes screenshots or videos if containing UI changes
  • Pull request includes a sign off

- 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 <mitchnull@gmail.com>
…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.
mitchnull added a commit to mitchnull/matrix-react-sdk that referenced this pull request Feb 6, 2021
im.vector.setting.override_colors is a map of userId -> colorSpec

colorSpec is either a color index (5) or a hexadecimal color (#rrggbb)

If colorSpec is a color index then this index will be used instead of
the userId hash to calculate the color-class for the nick-name span.

If colorSpec is a hexadecimal color code, then the span color will be
specified with a style.color attribute.

Setting the override color is currently only possible from the android
client. see element-hq/element-android#2614
…lement-android into feature/issue-2610-override-nick-color-via-user-account-data
…lement-android into feature/issue-2610-override-nick-color-via-user-account-data
…lement-android into feature/issue-2610-override-nick-color-via-user-account-data
…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
…ndroid into feature/issue-2610-override-nick-color-via-user-account-data
@bmarty bmarty added this to In progress in Android App Team via automation Oct 18, 2021
@bmarty bmarty moved this from In progress to Backlog (Unsorted) in Android App Team Oct 18, 2021
@daniellekirkwood daniellekirkwood added X-Needs-Design May require input from the design team X-Needs-Product Issue needs input from Product team labels Oct 19, 2021
@manuroe manuroe moved this from Backlog (Unsorted) to Need Design in Android App Team Oct 19, 2021
@bmarty bmarty added the Z-Community-PR Issue is solved by a community member's PR label Nov 30, 2021
@bmarty bmarty added this to Inbox in Element Android PRs lifecycle via automation Dec 31, 2021
@bmarty bmarty moved this from Inbox to To be finalized by us in Element Android PRs lifecycle Dec 31, 2021
@bmarty
Copy link
Member

bmarty commented Dec 31, 2021

Since the change of color is only client side and shared across user's device, I do not see a reason to not merge this PR.
I have quickly read the discussion in element-hq/element-meta#841.

TODO:

  • changelog has to be moved to a dedicated file
  • "the color change listener is now in AppStateHandler, not sure if this is the best place". Maybe better in a ViewModel used by the HomeActivity
  • fix conflict

@bmarty bmarty changed the base branch from develop to feature/bma/nick_color_final December 31, 2021 12:02
@bmarty
Copy link
Member

bmarty commented Dec 31, 2021

Will handle the remaining work on my branch.

@bmarty bmarty merged commit 7ce9ece into element-hq:feature/bma/nick_color_final Dec 31, 2021
Android App Team automation moved this from Need Design to Merged Dec 31, 2021
Element Android PRs lifecycle automation moved this from To be finalized by us to Done Dec 31, 2021
@bmarty bmarty mentioned this pull request Dec 31, 2021
@mitchnull
Copy link
Contributor Author

Will handle the remaining work on my branch.

thanks for picking this up!

baltitenger pushed a commit to mitchnull/matrix-react-sdk that referenced this pull request May 6, 2022
im.vector.setting.override_colors is a map of userId -> colorSpec

colorSpec is either a color index (5) or a hexadecimal color (#rrggbb)

If colorSpec is a color index then this index will be used instead of
the userId hash to calculate the color-class for the nick-name span.

If colorSpec is a hexadecimal color code, then the span color will be
specified with a style.color attribute.

Setting the override color is currently only possible from the android
client. see element-hq/element-android#2614
baltitenger pushed a commit to mitchnull/matrix-react-sdk that referenced this pull request May 6, 2022
im.vector.setting.override_colors is a map of userId -> colorSpec

colorSpec is either a color index (5) or a hexadecimal color (#rrggbb)

If colorSpec is a color index then this index will be used instead of
the userId hash to calculate the color-class for the nick-name span.

If colorSpec is a hexadecimal color code, then the span color will be
specified with a style.color attribute.

Setting the override color is currently only possible from the android
client. see element-hq/element-android#2614
baltitenger pushed a commit to mitchnull/matrix-react-sdk that referenced this pull request May 6, 2022
im.vector.setting.override_colors is a map of userId -> colorSpec

colorSpec is either a color index (5) or a hexadecimal color (#rrggbb)

If colorSpec is a color index then this index will be used instead of
the userId hash to calculate the color-class for the nick-name span.

If colorSpec is a hexadecimal color code, then the span color will be
specified with a style.color attribute.

Setting the override color is currently only possible from the android
client. see element-hq/element-android#2614
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-Needs-Design May require input from the design team X-Needs-Product Issue needs input from Product team Z-Community-PR Issue is solved by a community member's PR
Development

Successfully merging this pull request may close these issues.

None yet

4 participants