Bug 196266 – Web Inspector: Elements tab: Classes toggle should use accent color on hover

• Component: Web Inspector
• Status: RESOLVED FIXED

[Matt Baker]

Created attachment 365985 [details]
[Image] Classes button hover color

.

[Radar WebKit Bug Importer]

<rdar://problem/49286006>

[Matt Baker]

Created attachment 365986 [details]
Patch

[Matt Baker]

Created attachment 365988 [details]
[Image] With patch applied

[Radar WebKit Bug Importer]

<rdar://problem/49286158>

[Devin Rousso]

(In reply to Matt Baker from comment #3)
> Created attachment 365988 [details]
> [Image] With patch applied
The color of the "Classes" button and the DOM element highlight seem _very_ different.  Why is that?  Shouldn't we make them the same?  At the very least, it should match the "Styles" button.

[Matt Baker]

(In reply to Devin Rousso from comment #5)
> (In reply to Matt Baker from comment #3)
> > Created attachment 365988 [details]
> > [Image] With patch applied
> The color of the "Classes" button and the DOM element highlight seem _very_
> different.  Why is that?  Shouldn't we make them the same?  At the very
> least, it should match the "Styles" button.

The screenshot shows the hover color, not the selected/active color.

[Matt Baker]

Created attachment 366025 [details]
[Image] With patch applied (selected)

Radio, scope, and the toggle buttons have identical selection/hover colors, based on the system accent color. Table and TreeOutline use a different (but related) system color for the selected item background.

[Devin Rousso]

Comment on attachment 365986 [details]
Patch

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

rs=me

> Source/WebInspectorUI/UserInterface/Views/GeneralStyleDetailsSidebarPanel.css:116
> +    content: "";

NIT: I normally put `content` after `width`/`height` (e.g. sizing), but before `border` (e.g. outside of the content).

> Source/WebInspectorUI/UserInterface/Views/GeneralStyleDetailsSidebarPanel.css:118
> +    left: 0;
> +    top: 0;

NIT: the order of these should be reversed.  I match the order of the shorthand=>longhand values (top right bottom left).

> Source/WebInspectorUI/UserInterface/Views/GeneralStyleDetailsSidebarPanel.css:123
> +    z-index: -1;

NIT: I normally put `z-index` alongside other "positional" properties (e.g. `top`, `left`).

[WebKit Commit Bot]

Comment on attachment 365986 [details]
Patch

Clearing flags on attachment: 365986

Committed r244063: <https://trac.webkit.org/changeset/244063>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.