Bug 196266 - Web Inspector: Elements tab: Classes toggle should use accent color on hover
Summary: Web Inspector: Elements tab: Classes toggle should use accent color on hover
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P3 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-26 12:35 PDT by Matt Baker
Modified: 2019-04-08 18:32 PDT (History)
4 users (show)

See Also:

Attachments
[Image] Classes button hover color (68.10 KB, image/png)
2019-03-26 12:35 PDT, Matt Baker 		no flags Details
Patch (3.81 KB, patch)
2019-03-26 12:37 PDT, Matt Baker 		no flags Details | Formatted Diff | Diff
[Image] With patch applied (565.45 KB, image/png)
2019-03-26 12:39 PDT, Matt Baker 		no flags Details
[Image] With patch applied (selected) (673.66 KB, image/png)
2019-03-26 17:02 PDT, Matt Baker 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2019-03-26 12:35:04 PDT 
Created attachment 365985 [details]
[Image] Classes button hover color

.
Comment 1 Radar WebKit Bug Importer 2019-03-26 12:36:21 PDT 
<rdar://problem/49286006>
Comment 2 Matt Baker 2019-03-26 12:37:41 PDT 
Created attachment 365986 [details]
Patch
Comment 3 Matt Baker 2019-03-26 12:39:59 PDT 
Created attachment 365988 [details]
[Image] With patch applied
Comment 4 Radar WebKit Bug Importer 2019-03-26 12:40:34 PDT 
<rdar://problem/49286158>
Comment 5 Devin Rousso 2019-03-26 13:37:11 PDT 
(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.
Comment 6 Matt Baker 2019-03-26 13:48:57 PDT 
(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.
Comment 7 Matt Baker 2019-03-26 17:02:03 PDT 
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.
Comment 8 Devin Rousso 2019-03-29 13:35:58 PDT 
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`).
Comment 9 WebKit Commit Bot 2019-04-08 18:32:00 PDT 
Comment on attachment 365986 [details]
Patch

Clearing flags on attachment: 365986

Committed r244063: <https://trac.webkit.org/changeset/244063>
Comment 10 WebKit Commit Bot 2019-04-08 18:32:01 PDT 
All reviewed patches have been landed.  Closing bug.