Bug 193507 – Web Inspector: Use system accent color throughout UI

• Component: Web Inspector
• Status: RESOLVED FIXED

[Matt Baker]

Summary:
Use system accent color throughout UI. https://webkit.org/b/186609 exposed more semantic system colors to CSS. Using the system accent and highlight colors for some Inspector UI will provide a more seamless experience on macOS Mojave:

System accent color (-apple-system-control-accent):
- Breakpoints
- ScopeBar items
- NavigationBar button glyphs and radio buttons

System highlight color (-apple-system-selected-content-background):
- Table/TreeOutline selection

[Radar WebKit Bug Importer]

<rdar://problem/47327971>

[Matt Baker]

Created attachment 359300 [details]
Patch

[Matt Baker]

Created attachment 359303 [details]
[Image] Blue (default)

[Matt Baker]

Created attachment 359304 [details]
[Image] Purple

[Matt Baker]

Created attachment 359305 [details]
[Image] Pink

[Matt Baker]

Created attachment 359306 [details]
[Image] Orange

[Matt Baker]

Created attachment 359307 [details]
[Image] Breakpoints - green

[Blaze Burg]

Comment on attachment 359300 [details]
Patch

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

I think we need to extract the -apple- values into variables so that other ports can customize them. Other than that, this patch looks good to me. Nikita, please take a look as well.

> Source/WebInspectorUI/ChangeLog:18
> +        to pick up the system accent color.

Yay!

> Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.css:33
> +    filter: grayscale();

Nice idea.

> Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js:164
> +        this.status.classList.toggle(WI.BreakpointTreeElement.StatusImageResolvedStyleClassName, this._breakpoint.resolved && WI.debuggerManager.breakpointsEnabled);

Nice cleanup.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:65
> +    background-color: -apple-system-unemphasized-selected-content-background;

o_o

[Matt Baker]

(In reply to Brian Burg from comment #8)
> Comment on attachment 359300 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359300&action=review
> 
> I think we need to extract the -apple- values into variables so that other
> ports can customize them. Other than that, this patch looks good to me.
> Nikita, please take a look as well.

These are defined in CSSValueKeywords.in behind a guard,

#if defined(WTF_PLATFORM_MAC) && WTF_PLATFORM_MAC

so accommodations will need to be made.

[Nikita Vasilyev]

Comment on attachment 359300 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/RadioButtonNavigationItem.css:64
> +    filter: brightness(0.8);

This makes the default blue (light mode, blue highlight color, blue accent color) look too dark.

[Matt Baker]

I agree, I'll need to tweak the colors a little.

[Nikita Vasilyev]

Created attachment 359320 [details]
[Image] Light mode - blue of selected tab is too dark

The blue color of the selected tab is too dark.

Ideally, we'd want to make it darker only for certain accent colors (e.g. yellow).
I don't know how to do this.

If we can't, we should decide what do we care the most: matching system appearance or making custom accent colors more readable. I'm leaning towards the former.

[Matt Baker]

Created attachment 359322 [details]
[Image] Accent color comparisons

In general the light mode colors look too dark, and the dark mode selected and hover colors are too similar.

[Nikita Vasilyev]

Created attachment 359323 [details]
[Image] Bug — Network tab

[Matt Baker]

I agree with Brian that we need to break out the -apple-system- values. The approach I'm taking is to have a set of variables that parallel the system ones, that are initialized to the hardcoded values we've been using. Then on ports that support it (just Mojave for now), we override then.

I wanted to do something like this in Variables.css:

:root {
    --system-control-accent: hsl(212, 92%, 54%);
    ....
}

body:matches(.mojave) {
    --system-control-accent: -apple-system-control-accent;
    ...
}

:root {
    --glyph-color: var(--system-control-accent);
    ...
}

But of course this doesn't work; the Mojave override occurs at the body level, which has no effect on the variables scoped to the document (:root). I think this approach will require declaring all custom CSS properties at the body level rather than :root.

Any thoughts? Alternative approaches?

[Joseph Pecoraro]

(In reply to Matt Baker from comment #15)
> I agree with Brian that we need to break out the -apple-system- values. The
> approach I'm taking is to have a set of variables that parallel the system
> ones, that are initialized to the hardcoded values we've been using. Then on
> ports that support it (just Mojave for now), we override then.
> 
> I wanted to do something like this in Variables.css:
> 
> :root {
>     --system-control-accent: hsl(212, 92%, 54%);
>     ....
> }
> 
> body:matches(.mojave) {
>     --system-control-accent: -apple-system-control-accent;
>     ...
> }
> 
> :root {
>     --glyph-color: var(--system-control-accent);
>     ...
> }
> 
> But of course this doesn't work; the Mojave override occurs at the body
> level, which has no effect on the variables scoped to the document (:root).
> I think this approach will require declaring all custom CSS properties at
> the body level rather than :root.
> 
> Any thoughts? Alternative approaches?

Why not just:

    :root {
        --glyph-color: hsl(212, 92%, 54%);
    }

    body:matches(.mojave) {
        --glyph-color: -apple-system-control-accent;
    }

[Matt Baker]

Created attachment 362263 [details]
Patch

[Matt Baker]

(In reply to Matt Baker from comment #17)
> Created attachment 362263 [details]
> Patch

Getting pretty close. Still need to tweak a couple of things. ScopeBar/Radio button backgrounds for the inactive-window case are tricky.

[Timothy Hatcher]

I think we should use the accent colors directly whenever possible. That way we can get changes in the system for free and always match.

[Timothy Hatcher]

Comment on attachment 362263 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/Variables.css:173
> +body:matches(.mojave) {

This will need updated every year when we have new systems. You should invert this can make it something like: body.mac-platform:not(.sierra, .high-sierra)

Also you don't need to use :matches(.classname), since body.classname is the same thing. Using :matches() is for more complex selectors to avoid duplication. See: https://css-tricks.com/almanac/selectors/m/matches/

[Timothy Hatcher]

Comment on attachment 362263 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/Variables.css:173
>> +body:matches(.mojave) {
> 
> This will need updated every year when we have new systems. You should invert this can make it something like: body.mac-platform:not(.sierra, .high-sierra)
> 
> Also you don't need to use :matches(.classname), since body.classname is the same thing. Using :matches() is for more complex selectors to avoid duplication. See: https://css-tricks.com/almanac/selectors/m/matches/

Actually that should be: body.mac-platform:not(.sierra):not(.high-sierra), since :not() is not like :matches().

[Matt Baker]

Created attachment 362947 [details]
Patch

[Matt Baker]

Created attachment 362954 [details]
Patch

[Matt Baker]

Most recent changes:

1) Added system accent to
   - Console prompt ">"
   - Inline widgets (spring/curve and variable)
   - Rendering Frames timeline selected frame marker
   - DOM breakpoints

2) Radio/scope button background color is no longer darkened.
3) Pressed (:active) radio/scope button is now slightly less dark.

[Matt Baker]

Created attachment 362956 [details]
[Image] Light mode - default accent

[Matt Baker]

Created attachment 362957 [details]
[Image] Dark mode - green accent

[EWS Watchlist]

Comment on attachment 362954 [details]
Patch

Attachment 362954 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11285357

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall-no-ssrcs.https.html

[EWS Watchlist]

Created attachment 362963 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Matt Baker]

(In reply to Build Bot from comment #28)
> Created attachment 362963 [details]
> Archive of layout-test-results from ews125 for ios-simulator-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the
> ios-sim-ews.
> Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

Test failures are unrelated.

[Nikita Vasilyev]

I think this is good to land!

In the follow ups we should address:
— Selected console messages should use the selected accent/highlight color. We may have to disable syntax highlighting for selected messages — to be determined.
— In Dark Mode with the default accent color, our icon glyphs are too dark. The active glyph of Inspect Element is particularly bad.

[Nikita Vasilyev]

Created attachment 363035 [details]
[Image] Inspect Element button, Dark Mode, Blue accent

[Nikita Vasilyev]

Comment on attachment 362954 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:96
> +        Use system accent color for selection and hover styles. Since it isn't
> +        yet possible to derive new colors from the accent color in CSS, fake it
> +        with a ::before pseudo-element that can have have `filter` or `opacity`
> +        effects applied to it without altering the button text color.

It's possible to derive accent colors with getComputedStyle, adjust them with JS, and then store them as CSS variables.
I think it's worth exploring as a follow-up.

[Matt Baker]

(In reply to Nikita Vasilyev from comment #31)
> Created attachment 363035 [details]
> [Image] Inspect Element button, Dark Mode, Blue accent

We got pretty close to matching the equivalent UI used for Xcode's docking buttons by adjusting the accent color with brightness/saturate filters. It's a little hacky, and I'll need to confirm it works for all the system accents in both light/dark mode.

Will try this in a follow up.

[WebKit Commit Bot]

Comment on attachment 362954 [details]
Patch

Clearing flags on attachment: 362954

Committed r242118: <https://trac.webkit.org/changeset/242118>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.