-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 account initial as default avatar #4002
Use account initial as default avatar #4002
Conversation
Avatars was cutted in the top and bottom
The failed test seems unrelated. |
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.
LGTM, but I would opt for a different colour generation method based on the name.
private getColorTheme () { | ||
const themes = { | ||
abc: 'blue', | ||
def: 'green', | ||
ghi: 'purple', | ||
jkl: 'gray', | ||
mno: 'yellow', | ||
pqr: 'orange', | ||
stv: 'red', | ||
wxyz: 'dark-blue' | ||
} | ||
|
||
const theme = Object.keys(themes).find(chars => chars.includes(this.initial)) | ||
|
||
return themes[theme] | ||
} |
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.
Please use a simple hash-based colour generation function, which allows more diversity even for two names beginning by the same letter.
i.e. https://stackoverflow.com/questions/3426404/create-a-hexadecimal-colour-based-on-a-string-with-javascript
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 nice, but how do we ensure the contrast between text and bg is good? And can it be an ugly UI if we just generate random colors?
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.
You can generate colors in a restricted color space.
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 sorry but I don't understand how this should be implemented. Shall we define a set of color spaces and match those with a text color? It sounds tricky to me. And by using color spaces I'm not confident with that the contrast between bg and text is good enough, but maybe there's an easy way to ensure that?
Description
Change default avatar to account name initial.
Related issues
closes #3935
Has this been tested?
Screenshots