Bug 195190 – Web Inspector: system accent color follow-ups

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

[Matt Baker]

A few follow-ups to https://webkit.org/b/193507.

- Use accent color for selected timeline record bars
- Use accent color for selected console item "border"
- Increase contrast of activated toolbar button item (Inspect Element button)
- Pressed ScopeBarItem accent color should be same brightness as pressed RadioButtonNavigationItem

[Radar WebKit Bug Importer]

<rdar://problem/48492280>

[Matt Baker]

Created attachment 363270 [details]
Patch

[Matt Baker]

Created attachment 363271 [details]
[Image] New accent color uses

[Matt Baker]

Comment on attachment 363270 [details]
Patch

Reviewable, but investigating a ButtonToolbarItem issue.

[Matt Baker]

Created attachment 363284 [details]
Patch

[Matt Baker]

Created attachment 363285 [details]
[Image] Inspect Element button

Comparison of light/dark, new/old.

[Matt Baker]

(In reply to Matt Baker from comment #6)
> Created attachment 363285 [details]
> [Image] Inspect Element button
> 
> Comparison of light/dark, new/old.

The CSS shown to the left refers to the "Patch" column only.

[Devin Rousso]

Comment on attachment 363284 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css:73
> +        filter: brightness(1.35);

Nice!

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:45
> +    background-color: var(--selected-text-background-color) !important;

This is almost too light now.  It makes it harder to notice for small record bars.  Can you darken it a bit?

[Nikita Vasilyev]

Comment on attachment 363284 [details]
Patch

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

Looks all good to me!

>> Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css:73
>> +        filter: brightness(1.35);
> 
> Nice!

I honestly didn't notice that the color was different — it just looked right!

>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:45
>> +    background-color: var(--selected-text-background-color) !important;
> 
> This is almost too light now.  It makes it harder to notice for small record bars.  Can you darken it a bit?

Too light on what configuration?

It doesn't seem too light to me. I tried both Dark and Light modes with various accent colors.

[Nikita Vasilyev]

Created attachment 363776 [details]
[Image] Selected timeline segment with patch applied

Looks good to me.

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #10)
> Created attachment 363776 [details]
> [Image] Selected timeline segment with patch applied
> 
> Looks good to me.
I personally think that that doesn't match the "pattern" of the other records.  The difference between the border and the content of the selected record is greater than that of the non-selected records.

Basically, I think we should make the light-blue slightly darker (or make the border slightly lighter) so that the difference is less pronounced.

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #11)
> (In reply to Nikita Vasilyev from comment #10)
> > Created attachment 363776 [details]
> > [Image] Selected timeline segment with patch applied
> > 
> > Looks good to me.
> I personally think that that doesn't match the "pattern" of the other
> records.  The difference between the border and the content of the selected
> record is greater than that of the non-selected records.
> 
> Basically, I think we should make the light-blue slightly darker (or make
> the border slightly lighter) so that the difference is less pronounced.

I see what you mean now.

If anything, the difference should be more pronounced. Try selecting red bars with red accent colors!

[Matt Baker]

Comment on attachment 363284 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:45
>>> +    background-color: var(--selected-text-background-color) !important;
>> 
>> This is almost too light now.  It makes it harder to notice for small record bars.  Can you darken it a bit?
> 
> Too light on what configuration?
> 
> It doesn't seem too light to me. I tried both Dark and Light modes with various accent colors.

I see what you mean, but I think the current approach is fine for now - it's simple and uses unmodified system colors.

[WebKit Commit Bot]

Comment on attachment 363284 [details]
Patch

Clearing flags on attachment: 363284

Committed r242578: <https://trac.webkit.org/changeset/242578>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.