Skip to content

Resolves #20: Generate random color for username and avatar background #27

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

Merged
merged 3 commits into from
Oct 11, 2021

Conversation

nicoschmdt
Copy link
Collaborator

Creates a list with six different colors and randomly chooses one for the username and one for the profile picture background

@nicoschmdt nicoschmdt requested a review from diogojs October 1, 2021 23:56
@JPTIZ JPTIZ added the enhancement New feature or request label Oct 2, 2021
@JPTIZ JPTIZ linked an issue Oct 2, 2021 that may be closed by this pull request
@JPTIZ JPTIZ requested a review from a team October 6, 2021 18:12
Copy link
Member

@tarcisioe tarcisioe left a comment

Choose a reason for hiding this comment

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

LGTM, but a strong suggestion would be to somehow derive the color from the user ID or the person's name, so you get more consistent results than just a random choice.

User ID is already an integer, it could be used to get a color with id % len(FOREGROUND_COLORS) as an example.

@diogojs
Copy link
Contributor

diogojs commented Oct 8, 2021

@nicoschmdt I agree with @tarcisioe's suggestion. It would also be better so the avatar and username would have the same color (the way you implemented we can get a blue avatar with a red username).

@nicoschmdt
Copy link
Collaborator Author

I committed the suggested changes, can you check if it needs some improvements?

@tarcisioe
Copy link
Member

I'd just refactor it to a function such as

def get_user_color(user_id: int) -> str:
    return FOREGROUND_COLORS[user_id % len(FOREGROUND_COLORS)]

to avoid error-prone duplication. Also, was this tested?

@tarcisioe
Copy link
Member

LGTM. @diogojs just give it a last review before we merge it.

@tarcisioe tarcisioe merged commit 900c5ab into master Oct 11, 2021
@tarcisioe tarcisioe deleted the add-different-colors-for-username-and-avatar branch October 11, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add different colors for user names and avatars
4 participants