-
Notifications
You must be signed in to change notification settings - Fork 338
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 muted display names, and always show instance for non-local. #1975 #2064 #2425
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 really like this. I think it's a good way of giving users important information while looking nice and not taking up more space. The other comments I made are nitpicks that aren't essential to address.
@@ -93,7 +76,10 @@ export class PersonListing extends Component<PersonListingProps, any> { | |||
icon | |||
/> | |||
)} | |||
<span>{displayName}</span> | |||
<span>{name}</span> | |||
{serverStr !== undefined && ( |
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.
Are there any falsey values you're trying to avoid matching?
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'll fix in a sec.
const classes = classNames( | ||
"person-listing d-inline-flex align-items-baseline", | ||
{ | ||
"text-muted": this.props.muted, | ||
"text-info": !this.props.muted, | ||
}, | ||
); |
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.
A ternary using muted
would be more concise.
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.
This is using that special classNames
function, it can't work in that way.
Description
Everyone seems to like this one from jerboa anyway, so I'll add it here.
When its a non-local instance, always show the instance, but make it smaller and muted.
Screenshots
Before
Didn't show instance name if a display name was set
After
Always shows it (for non-local)
(ignore the localhost string, that's just for testing)