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

XEP-0392 implementation: colorize nicknames #3347

Closed
wants to merge 2 commits into from

Conversation

JohnXLivingston
Copy link
Contributor

This PR implements XEP-0392 (Consistent Color Generation) for message author nicknames, and for MUC occupant list.

This XEP compliance is tracked here: #1349

This PR does not implement the default avatar color (but it should be easy to add it).

I added a configuration settings to enable this feature (colorize_username). By default i set it to false, but this can be changed if you think it is worth enabling it be default.

Here are some screenshot (the ConverseJS theme is not the default one on my project, but it is close enough to have a good idea of the result):

image

image

Note: the XEP is currently in "last call for comment" state, ending the 2024-03-25

@jcbrand
Copy link
Member

jcbrand commented Mar 15, 2024

Thanks @JohnXLivingston, this is great, although if it was up to me, I'd only make default avatars coloured, and not the nicknames as well.

Personally I find the coloured nicknames quite garish. Perhaps the feature could be extended to allow either nicknames or avatars or both to be coloured.

That said, I could see someone defending colouring the nicknames as a security feature (i.e. making it more difficult to impersonate someone).

@@ -0,0 +1,33 @@
import { sha1 } from 'js-sha1';
import { Hsluv } from 'hsluv';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the build small and avoid loading unwanted code (e.g. if colorize_nicknames is false), I think we should see if this can be loaded dynamically.

See for example here:

const module = await import(/*webpackChunkName: "emojis" */ './emoji.json');

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import

Hopefully it just works and Webpack should know what to do (i.e. create a separate file for Hsluv in ./dist).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, great! Will fix. I just have to check if making the colorize function async is possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hsluv unminified is less that 12k, i think that it does not worth the overload of an additional http request.

@JohnXLivingston
Copy link
Contributor Author

Thanks @JohnXLivingston, this is great, although if it was up to me, I'd only make default avatars coloured, and not the nicknames as well.

In my project, i generate random avatars for user that don't have avatars. So i don't need colored avatars. (but that is just my case, and as i said, it will be easy to re-use the colorize function to apply to default avatar SVG).

Personally I find the coloured nicknames quite garish. Perhaps the feature could be extended to allow either nicknames or avatars or both to be coloured.

I also found it quite garish, but i got good feedbacks on Mastodon.

We could add another settings for avatars (colorize_default_avatar?), or change the settings to colorize that could get multiple values: none, default_avatar, username, both.
What do you prefer?
The second solution seems elegant, but will not work if we want to add a third object to colorize (i don't know if this will be the case one day).

That said, I could see someone defending colouring the nicknames as a security feature (i.e. making it more difficult to impersonate someone).

Yes, and i think it will be usefull in my use case.

@jcbrand jcbrand force-pushed the xep0392_nickname_color branch from 2edcfc1 to 7aea3c4 Compare March 24, 2024 10:38
@jcbrand
Copy link
Member

jcbrand commented Mar 25, 2024

@JohnXLivingston I've rebased this PR to get the tests green.

@jcbrand jcbrand force-pushed the xep0392_nickname_color branch from 7aea3c4 to 8253a30 Compare March 25, 2024 05:55
@jcbrand jcbrand marked this pull request as draft March 25, 2024 07:20
JohnXLivingston and others added 2 commits May 10, 2024 21:48
When calling `occupant.getColor()` I think a color should always be
returned, but this means making the function async, and thereby using
`until` in the templates.

This change also only generates the color on demand, i.e. lazily,
instead of eagerly.

The benefit is that we avoid generating colors that aren't being used
because the occupant isn't being shown anywhere in the UI.
@jcbrand jcbrand force-pushed the xep0392_nickname_color branch from d49d61e to b5b8d8a Compare May 10, 2024 20:12
@jcbrand jcbrand closed this Jun 4, 2024
@jcbrand jcbrand deleted the xep0392_nickname_color branch January 12, 2025 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants