Bug 193804 – Web Inspector: Improve Dark Mode appearance within Memory timeline

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Improve Dark Mode appearance within Memory timeline

- Some text shows up as Black in dark mode
- Improve gray text in general
- Improve max comparison graph which used gray colors

[Radar WebKit Bug Importer]

<rdar://problem/47535060>

[Joseph Pecoraro]

Created attachment 360055 [details]
[IMAGE] Dark Mode - Before

[Joseph Pecoraro]

Created attachment 360056 [details]
[IMAGE] Dark Mode - After

[Joseph Pecoraro]

Created attachment 360057 [details]
[PATCH] Proposed Fix

[Matt Baker]

Comment on attachment 360057 [details]
[PATCH] Proposed Fix

r=me

One nit is that the 'Maximum' legend item in the 'Max Comparison' chart looks odd since the swatch lacks a border in dark mode. Maybe a thing gray border, matching the appearance of the doughnut chart, would work.

Other than that, it looks great!

[Matt Baker]

(In reply to Matt Baker from comment #5)
> Comment on attachment 360057 [details]
> [PATCH] Proposed Fix
> 
> r=me
> 
> One nit is that the 'Maximum' legend item in the 'Max Comparison' chart
> looks odd since the swatch lacks a border in dark mode. Maybe a thing gray

thin gray border

[Nikita Vasilyev]

Comment on attachment 360057 [details]
[PATCH] Proposed Fix

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

Looks great!

"Maximum Size" and "Maximum CPU Usage" labels should also be more Dark Mode friendly but it doesn't have to be in the same patch.

> Source/WebInspectorUI/UserInterface/Views/Variables.css:40
> +    --text-color-secondary: hsl(0, 0%, 50%);

Nice!

[Matt Baker]

(In reply to Matt Baker from comment #5)
> Comment on attachment 360057 [details]
> [PATCH] Proposed Fix
> 
> r=me
> 
> One nit is that the 'Maximum' legend item in the 'Max Comparison' chart
> looks odd since the swatch lacks a border in dark mode. Maybe a thing gray
> border, matching the appearance of the doughnut chart, would work.
> 
> Other than that, it looks great!

Nevermind! It's been so long since I've used light mode that I forgot that we don't show a swatch for 'Maximum'.

[Joseph Pecoraro]

Comment on attachment 360057 [details]
[PATCH] Proposed Fix

Going to fix a few more things.

[Joseph Pecoraro]

Created attachment 360070 [details]
[IMAGE] Dark Mode - After (2)

[Joseph Pecoraro]

Created attachment 360071 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Timelines edit mode had some issues, and I fixed the legend's that Nikita pointed out.

[EWS Watchlist]

Attachment 360071 [details] did not pass style-queue:


ERROR: Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:246:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Matt Baker]

Comment on attachment 360071 [details]
[PATCH] Proposed Fix

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

r=me, just resolve the style issue before landing.

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:247
> +    }

Fix whitespace.

[Joseph Pecoraro]

Comment on attachment 360071 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:247
>> +    }
> 
> Fix whitespace.

I'm not sure which whitespace you're referring to. The leading whitespace is because this is inside of a `@media (prefers-color-scheme: dark) { ... }` block.

[Joseph Pecoraro]

Comment on attachment 360071 [details]
[PATCH] Proposed Fix

Oh a tab character

[Joseph Pecoraro]

Created attachment 360169 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 360169 [details]
[PATCH] For Landing

Clearing flags on attachment: 360169

Committed r240518: <https://trac.webkit.org/changeset/240518>