Bug 197955 – Web Inspector: Dark Mode: Fix colors in network table waterfall container

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Jamal Nasser]

Created attachment 370055 [details]
[Screenshot] Bug

The waterfall-container area under the network tab is currently using colors that don't suit the dark mode well.
Specifically, changed the blue circles to be brighter and the 'fullscreen' grey area to be dimmer. The latter is also now using a preexisting color variable instead of a custom color.

[Jamal Nasser]

Created attachment 370056 [details]
[Screenshot] Patch applied

[Jamal Nasser]

Created attachment 370060 [details]
Patch

[Devin Rousso]

Comment on attachment 370060 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370060&action=review

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:211
> +    background-color: var(--dashboard-background-color);

We probably don't want to reuse a variable like this, since the fullscreen event area has nothing to do with the dashboard.  Also, FWIW I think we might want to make it slightly lighter, almost halfway between what you have now `hsl(0, 0%, 36%)` and what was there before `hsla(0, 0%, 75%, 0.75)`.  Maybe a regular `grey` or a `hsla(0, 0%, 50%)`?

[Devin Rousso]

Comment on attachment 370060 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370060&action=review

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:187
> +    background-color: var(--selected-background-color-active);

This color is WAY better :)

[Jamal Nasser]

Created attachment 370137 [details]
Patch

[Jamal Nasser]

(In reply to Devin Rousso from comment #3)
> Comment on attachment 370060 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370060&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:211
> > +    background-color: var(--dashboard-background-color);
> 
> We probably don't want to reuse a variable like this, since the fullscreen
> event area has nothing to do with the dashboard.  Also, FWIW I think we
> might want to make it slightly lighter, almost halfway between what you have
> now `hsl(0, 0%, 36%)` and what was there before `hsla(0, 0%, 75%, 0.75)`. 
> Maybe a regular `grey` or a `hsla(0, 0%, 50%)`?

50% is a little too bright in my opinion. This is what 40% looks like: https://i.imgur.com/d8Q1V8A.png

I think that fits better than 50%:
https://i.imgur.com/Rz0RRuV.png

[Jamal Nasser]

Created attachment 370139 [details]
Patch

[Devin Rousso]

Comment on attachment 370139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370139&action=review

r=me, nice fix!

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:211
> +    background-color: var(--network-waterfall-fullscreen-area-color);

There's no reason to make a variable if it's only really used once (or even twice).  Please just inline it.

[Jamal Nasser]

Created attachment 370143 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 370143 [details]
Patch

Clearing flags on attachment: 370143

Committed r245484: <https://trac.webkit.org/changeset/245484>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50909676>