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

web ui: Tweak colors in the dark theme to improve contrast ratio #11068

Merged
merged 4 commits into from
Aug 1, 2022

Conversation

jorgelbg
Copy link
Contributor

Some colors from the dark theme used in the query editor have a very low contrast ratio with the background.

For reference these are some of the colors from the promqlHighlighter with their respective contrast ratio calculated using the background color from boostrap-dark (#191D21):

#09885a // 3.78 poor
#a31515 // 2.16 very poor
#008080 // 3.55 poor
#008080 // 3.55 poor
#800000 // 1.55 very poor
#008080 // 3.55 poor
red // #FF0000 4.24 poor
#888' // 4.78 good

The low contrast was more noticeable with the red color(s) used for the label names.

The screenshots shown the result after applying the changes included in this PR. On top of the changes to the colors used in the editor I've also tweaked a couple of additional colors for checkboxes of the top and for the query stats.

before:
image

after:
image

Signed-off-by: Jorge Luis Betancourt Gonzalez jorge-luis.betancourt@trivago.com

Some colors from the dark theme used in the query editor have a very low
contrast ratio with the background.

Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>
@jorgelbg jorgelbg requested a review from juliusv as a code owner July 28, 2022 12:12
Copy link
Member

@juliusv juliusv left a comment

Choose a reason for hiding this comment

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

Thanks, yes that looks much better! Just a nit comment.

web/ui/react-app/src/pages/graph/ExpressionInput.tsx Outdated Show resolved Hide resolved
@juliusv
Copy link
Member

juliusv commented Jul 28, 2022

Oh, I also noticed that parentheses are very dark in dark mode when the cursor is currently over them:

parens

@jorgelbg
Copy link
Contributor Author

Oh, I also noticed that parentheses are very dark in dark mode when the cursor is currently over them:

should be fixed now, when the input component was focused it applied an additional style via a .cm-focused .cm-matchingBracket rule (which seems to be taken the color from browser/OS) and it was applied both in light and dark mode. I've applied the same style (when in dark mode) for both the focused and non focused states (looks like this now):

image

jorgelbg and others added 2 commits July 28, 2022 15:40
Co-authored-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>
Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>

'.cm-matchingBracket, &.cm-focused .cm-matchingBracket': {
color: '#1e293b',
backgroundColor: '#cbd5e1',
Copy link
Member

Choose a reason for hiding this comment

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

Nice. But in dark mode, the cursor is now very low contrast in comparison to the bright background of the parentheses it is on, making it hard to spot. How about making that background color just a tad less bright, like the #767676 one we have for selections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, with #767676 the contrast ratio between the highlighted text and the background is a bit too low (not critical IMHO considering that this not really for reading, but more for highlighting a position within the query). We could also make the text color completely white or black to make it easier to read. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

That's a good point. Looking at how it works for the light mode and playing with that, it would be nice to do the equivalent of that for dark mode:

  • Don't override the text color at all (keep it at its default white-ish tone), and the boldness still gets added anyway.
  • Set the background color to something subtle relative to the black background, like #616161.

parens

So basically remove the color line above and do backgroundColor: #616161. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, removing the color and using the new background color provides a good enough contrast in this case.

Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>
@juliusv
Copy link
Member

juliusv commented Aug 1, 2022

👍 Nice, thanks!

@juliusv juliusv merged commit 595d134 into prometheus:main Aug 1, 2022
Mama59 pushed a commit to Arnoways/prometheus that referenced this pull request Aug 25, 2022
…metheus#11068)

* Tweak colors in the dark theme to improve contrast

Some colors from the dark theme used in the query editor have a very low
contrast ratio with the background.

Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>

* Avoid duplicated function call when in dark mode

Co-authored-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>

* Apply styles for the matching bracket when focused in dark mode

Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>

* Improve style of the matching brackets when focused

Signed-off-by: Jorge Luis Betancourt Gonzalez <jorge-luis.betancourt@trivago.com>

Co-authored-by: Julius Volz <julius.volz@gmail.com>
@@ -239,3 +277,19 @@ export const promqlHighlighter = HighlightStyle.define([
{ tag: tags.invalid, color: 'red' },
{ tag: tags.comment, color: '#888', fontStyle: 'italic' },
]);

export const darkPromqlHighlighter = HighlightStyle.define([
{ tag: tags.name, color: '#000' },
Copy link
Member

Choose a reason for hiding this comment

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

Whoops, we just found out that this caused #11568. We are accidentally setting the text color to black on black here. Will send a fix.

juliusv added a commit that referenced this pull request Nov 11, 2022
The color should not be set explicitly at all. That way it simply inherits the
theme's default color, as before
#11068.

Fixes #11568

Signed-off-by: Julius Volz <julius.volz@gmail.com>
juliusv added a commit that referenced this pull request Nov 11, 2022
The color should not be set explicitly at all. That way it simply inherits the
theme's default color, as before
#11068.

Fixes #11568

Signed-off-by: Julius Volz <julius.volz@gmail.com>
juliusv added a commit that referenced this pull request Nov 11, 2022
The color should not be set explicitly at all. That way it simply inherits the
theme's default color, as before
#11068.

Fixes #11568

Signed-off-by: Julius Volz <julius.volz@gmail.com>

Signed-off-by: Julius Volz <julius.volz@gmail.com>
codesome pushed a commit to codesome/prometheus that referenced this pull request Nov 16, 2022
The color should not be set explicitly at all. That way it simply inherits the
theme's default color, as before
prometheus#11068.

Fixes prometheus#11568

Signed-off-by: Julius Volz <julius.volz@gmail.com>

Signed-off-by: Julius Volz <julius.volz@gmail.com>
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

2 participants