Bug 197955

Summary: Web Inspector: Dark Mode: Fix colors in network table waterfall container
Product: WebKit Reporter: Jamal Nasser <jamaln>
Component: Web InspectorAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Screenshot] Bug
none
[Screenshot] Patch applied
none
Patch
none
Patch
none
Patch
none
Patch none

Description Jamal Nasser 2019-05-16 12:27:45 PDT
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.
Comment 1 Jamal Nasser 2019-05-16 12:28:24 PDT
Created attachment 370056 [details]
[Screenshot] Patch applied
Comment 2 Jamal Nasser 2019-05-16 12:44:11 PDT
Created attachment 370060 [details]
Patch
Comment 3 Devin Rousso 2019-05-16 14:11:58 PDT
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%)`?
Comment 4 Devin Rousso 2019-05-16 14:12:22 PDT
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 :)
Comment 5 Jamal Nasser 2019-05-17 13:09:03 PDT
Created attachment 370137 [details]
Patch
Comment 6 Jamal Nasser 2019-05-17 13:13:55 PDT
(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
Comment 7 Jamal Nasser 2019-05-17 13:17:51 PDT
Created attachment 370139 [details]
Patch
Comment 8 Devin Rousso 2019-05-17 13:33:56 PDT
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.
Comment 9 Jamal Nasser 2019-05-17 13:41:17 PDT
Created attachment 370143 [details]
Patch
Comment 10 WebKit Commit Bot 2019-05-17 16:07:27 PDT
Comment on attachment 370143 [details]
Patch

Clearing flags on attachment: 370143

Committed r245484: <https://trac.webkit.org/changeset/245484>
Comment 11 WebKit Commit Bot 2019-05-17 16:07:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2019-05-17 16:08:24 PDT
<rdar://problem/50909676>