-
Notifications
You must be signed in to change notification settings - Fork 15
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
Use consistent colors for users in project/volume charts #888
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if adding the whole md5 algorithm is worth it here. Also this could randomly produce very dark colors, right? What about reusing the random color function of new labels and then generating a map wit colors for each user on the fly? The only downside would be that colors of users are different between page loads...
So if we want consistent colors between page loads, md5 may be a valid option. But then I'd like to at least change it so the user ID is used for hashing instead of the name and the random label color function is used to get a bright color. The function could even be adapted to accept an optional seed.
What about this one? It uses a much simpler hashing. And it uses HSL color space to have fixed saturation and lightness to produce bright results. However, users with similar colors might occur. Using the id instead of name is not that simple as the information is not available at that point (yet). But I don't see a problem with name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user IDs are available but it might be a little complicated to get them where you need them. While I'd still prefer the IDs over the user names, I'm ok with the names for now, as they will work in most cases. We can open a follow-up issue to use the IDs at some point.
resources/assets/js/projects/components/charts/usernameToColor.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I increased the minimum lightness a bit because some colors were too dark for my taste. I only have one question (below) then it can be merged.
… into commonUserColorsCharts
One thing I now noticed is that if the users did not annotate in consecutive months, the x-axis is not "to scale", if you know what I mean. Do we think this as a bug or a feature? 😄 |
For now it's a feature but I guess it's rather a bug ;-) |
Ok then I'll create a follow-up issue. |
This way the colors are consistent fixes #497