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

fix: Make new comment indicator easier to distinguish #1812

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jsit
Copy link
Contributor

@jsit jsit commented Jul 4, 2023

Description

The "(2 new)" comments indicator is too hard to miss now that it's just gray. This changes that.

Screenshots

Before

Screenshot 2023-07-04 at 1 12 40 PM Screenshot 2023-07-04 at 1 12 24 PM

After

Screenshot 2023-07-04 at 1 11 31 PM Screenshot 2023-07-04 at 1 11 20 PM

Comment on lines +763 to 765
<span className="badge text-bg-info">
({this.unreadCount} {I18NextService.i18n.t("new")})
</span>
Copy link
Member

Choose a reason for hiding this comment

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

I recall before we used the dull color, we colored the font instead of giving it a background. Maybe try that instead while keeping the italic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this:

Screenshot 2023-07-04 at 1 26 10 PM Screenshot 2023-07-04 at 1 26 01 PM

Copy link
Member

Choose a reason for hiding this comment

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

Which theme color is that? I was thinking of using the info color.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's "danger" -- "info" feels semantically correct, but because we're using these theme colors more or less arbitrarily, it leads to some potentially confusing clashes:

Screenshot 2023-07-04 at 1 50 56 PM Screenshot 2023-07-04 at 1 50 45 PM

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the UI used to use text-warning:

<span className="small text-warning">
    ({this.unreadCount} {i18n.t("new")})
</span>

@dessalines
Copy link
Member

This one I'm not a fan of. We had it as a badge color, but it just looks so much better as the same muted color as the comment count. It just needs emphasized in some way (either via italics like it was), or some other font emphasis.

@jsit
Copy link
Contributor Author

jsit commented Jul 4, 2023

@dessalines Thanks for weighing in. Two different options:

Screenshot 2023-07-04 at 6 44 57 PM Screenshot 2023-07-04 at 6 46 50 PM

@jsit jsit marked this pull request as draft July 4, 2023 22:47
@dessalines
Copy link
Member

dessalines commented Jul 4, 2023

I'll go with the majority, but my vote would be for the original, simple italics:

I also like the idea of minimizing colors except for user and community names. And using default or muted colors for everything else. Even the featured title could be done just with a different font emphasis or style.

@jsit
Copy link
Contributor Author

jsit commented Jul 4, 2023

I don't feel strongly about this either. Would be fine with closing this.

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.

None yet

3 participants