Bug 193804 - Web Inspector: Improve Dark Mode appearance within Memory timeline
Summary: Web Inspector: Improve Dark Mode appearance within Memory timeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-24 17:17 PST by Joseph Pecoraro
Modified: 2019-01-25 15:38 PST (History)
7 users (show)

See Also:


Attachments
[IMAGE] Dark Mode - Before (743.44 KB, image/png)
2019-01-24 17:19 PST, Joseph Pecoraro
no flags Details
[IMAGE] Dark Mode - After (993.61 KB, image/png)
2019-01-24 17:19 PST, Joseph Pecoraro
no flags Details
[PATCH] Proposed Fix (5.16 KB, patch)
2019-01-24 17:20 PST, Joseph Pecoraro
mattbaker: review+
Details | Formatted Diff | Diff
[IMAGE] Dark Mode - After (2) (707.11 KB, image/png)
2019-01-24 19:22 PST, Joseph Pecoraro
no flags Details
[PATCH] Proposed Fix (9.93 KB, patch)
2019-01-24 19:23 PST, Joseph Pecoraro
mattbaker: review+
Details | Formatted Diff | Diff
[PATCH] For Landing (9.93 KB, patch)
2019-01-25 14:52 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2019-01-24 17:17:54 PST
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
Comment 1 Radar WebKit Bug Importer 2019-01-24 17:18:27 PST
<rdar://problem/47535060>
Comment 2 Joseph Pecoraro 2019-01-24 17:19:48 PST
Created attachment 360055 [details]
[IMAGE] Dark Mode - Before
Comment 3 Joseph Pecoraro 2019-01-24 17:19:59 PST
Created attachment 360056 [details]
[IMAGE] Dark Mode - After
Comment 4 Joseph Pecoraro 2019-01-24 17:20:20 PST
Created attachment 360057 [details]
[PATCH] Proposed Fix
Comment 5 Matt Baker 2019-01-24 17:25:02 PST
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!
Comment 6 Matt Baker 2019-01-24 17:25:28 PST
(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
Comment 7 Nikita Vasilyev 2019-01-24 17:28:14 PST
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!
Comment 8 Matt Baker 2019-01-24 17:30:03 PST
(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'.
Comment 9 Joseph Pecoraro 2019-01-24 19:03:40 PST
Comment on attachment 360057 [details]
[PATCH] Proposed Fix

Going to fix a few more things.
Comment 10 Joseph Pecoraro 2019-01-24 19:22:59 PST
Created attachment 360070 [details]
[IMAGE] Dark Mode - After (2)
Comment 11 Joseph Pecoraro 2019-01-24 19:23:12 PST
Created attachment 360071 [details]
[PATCH] Proposed Fix
Comment 12 Joseph Pecoraro 2019-01-24 19:23:33 PST
Timelines edit mode had some issues, and I fixed the legend's that Nikita pointed out.
Comment 13 EWS Watchlist 2019-01-24 22:25:26 PST
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.
Comment 14 Matt Baker 2019-01-25 13:06:22 PST
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.
Comment 15 Joseph Pecoraro 2019-01-25 14:47:43 PST
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.
Comment 16 Joseph Pecoraro 2019-01-25 14:49:09 PST
Comment on attachment 360071 [details]
[PATCH] Proposed Fix

Oh a tab character
Comment 17 Joseph Pecoraro 2019-01-25 14:52:33 PST
Created attachment 360169 [details]
[PATCH] For Landing
Comment 18 WebKit Commit Bot 2019-01-25 15:31:22 PST
Comment on attachment 360169 [details]
[PATCH] For Landing

Clearing flags on attachment: 360169

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