1354472 - Poor contrast of DOMContentLoaded summary text in dark theme

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[Sebastian Zartner [:sebo]]

Attachment: Screenshot showing the poor contrast

The contrast of the blue DOMContentLoaded summary text on the dark gray background in the dark theme makes it hard to read.

Therefore the color should be changed to some lighter color.

Sebastian

Comment 1

[Tim Nguyen :ntim]

I've intentionally used #00f as colour because that's exactly the colour used in the waterfall ticks.
Definitively agree it could be improved though.

Comment 2

[Jens]

> I've intentionally used #00f as colour because that's exactly the colour used in the waterfall ticks.

Then the waterfall ticks should be changed as well I guess. This is so much into the face (or eyes ;) that I wonder why this bug is not P1 (or at least P2). Really severe UI bug, I cannot even read the "DOMContentLoaded" without moving very near to the monitor. At the same time, it's probably easy to fix I guess (or hope).

Maybe check Chrome, there the colors are readable in the dark theme, too. 

The Red is not great either in my opinion, could be lighter too in the dark theme.

Comment 3

[Tim Nguyen :ntim]

(In reply to Jens from comment #2)
> > I've intentionally used #00f as colour because that's exactly the colour used in the waterfall ticks.
> 
> Then the waterfall ticks should be changed as well I guess. This is so much
> into the face (or eyes ;) that I wonder why this bug is not P1 (or at least
> P2). Really severe UI bug, I cannot even read the "DOMContentLoaded" without
> moving very near to the monitor. At the same time, it's probably easy to fix
> I guess (or hope).

Changing the waterfall ticks isn't that trivial though.

Comment 4

[Jens]

(In reply to Tim Nguyen :ntim from comment #3)
> Changing the waterfall ticks isn't that trivial though.

Oh :-/ I'm curious why? I can't think of anything else than a CSS or JS variable to define a color. Even if it's a hard-coded color string I don't understand why this should be hard? Are we talking about the same thing, that is, single blue line?

Comment 5

[magicp]

I recommend below settings...

[netmonitor.css]

.status-bar-label.dom-content-loaded {
  color: var(--theme-highlight-blue);
}

.status-bar-label.load {
  color: var(--theme-highlight-red);
}

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1354472 - use readable colors for netmonitor in dark theme;

Review commit: https://reviewboard.mozilla.org/r/171286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171286/

Comment 7

[Julian Descottes [:jdescottes]]

(In reply to Tim Nguyen :ntim from comment #1)
> I've intentionally used #00f as colour because that's exactly the colour
> used in the waterfall ticks.
> Definitively agree it could be improved though.

The current colors are really not readable in dark theme so I think we should fix this asap and potentially uplift. 

We can have a follow up to make the colors of the lines match exactly the text. I think we would be fine hardcoding two colors for the lines in devtools/client/netmonitor/src/constants.js, one for light and one for dark theme. And listen to the theme-changed event to switch between the two. 

The patch here simply applies the suggestion from the last magicp's comment.

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8899956 [details]
Bug 1354472 - use readable colors for netmonitor in dark theme;

https://reviewboard.mozilla.org/r/171286/#review176462

IMO, we should really have a fix for the waterfall ticks landing at the same time, but this part looks good to me.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49655de50701
use readable colors for netmonitor in dark theme;r=ntim

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/49655de50701

Comment 11

[Sebastian Zartner [:sebo]]

(In reply to Tim Nguyen :ntim from comment #8)
> IMO, we should really have a fix for the waterfall ticks landing at the same
> time, but this part looks good to me.

Agree. Only changing the colors in the status bar causes an inconsistency between them. So, IMO, the patch should not have landed without changing those, too.

Unfortunately, as Julian already noted, they are defined as constants in JS[1]. Best approach would be to somehow reuse the colors defined in CSS.

Sebastian

[1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/src/constants.js#235,238

Comment 12

[Tim Nguyen :ntim]

(In reply to Sebastian Zartner [:sebo] from comment #11)
> Unfortunately, as Julian already noted, they are defined as constants in
> JS[1]. Best approach would be to somehow reuse the colors defined in CSS.
> 
> Sebastian
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/devtools/client/netmonitor/
> src/constants.js#235,238


I opened bug 1393395 for this.

Comment 13

[Julian Descottes [:jdescottes]]

(In reply to Sebastian Zartner [:sebo] from comment #11)
> (In reply to Tim Nguyen :ntim from comment #8)
> > IMO, we should really have a fix for the waterfall ticks landing at the same
> > time, but this part looks good to me.
> 
> Agree. Only changing the colors in the status bar causes an inconsistency
> between them. So, IMO, the patch should not have landed without changing
> those, too.
> 

I want to uplift this, that's why I don't want to have a bigger patch with JS changes here.

Regarding the inconsistency, the colors are still shades of blue and red. They're not the same shade of blue/red, agreed, but is it really inconsistent? In my opinion having unreadable text is a bug. Having the exact same shade for ticks and text is an improvement.

Sebastian: is there a particular theme/setting which makes it feel inconsistent?

Comment 14

[Tim Nguyen :ntim]

Attachment: Inconsistent colors

> Regarding the inconsistency, the colors are still shades of blue and red. They're not the same shade of blue/red, agreed, but is it really inconsistent? In my opinion having unreadable text is a bug. Having the exact same shade for ticks and text is an improvement.

It does look really inconsistent in the dark theme.

The red color looks OK, but the blue looks really different from the corresponding waterfall tick.

Comment 15

[Sebastian Zartner [:sebo]]

(In reply to Julian Descottes [:jdescottes] from comment #13)
> Sebastian: is there a particular theme/setting which makes it feel
> inconsistent?

As all themes use the same colors, the inconsistency exists in all three of them. As Tim says, the colors look quite different.

> I want to uplift this, that's why I don't want to have a bigger patch with
> JS changes here.

Ok, so I think a good compromise is to just change the two constants to the values used in the CSS variables[1], i.e.

  DOMCONTENTLOADED_TICKS_COLOR_RGBA: [0, 136, 204, 128]
  LOAD_TICKS_COLOR_RGBA: [237, 38, 85, 128]

(Maybe with adjusted alpha value if they are too pale.)

Sebastian

[1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/variables.css#43,48