Bug 154302

Summary: Web Inspector: add CSS variables for common border/background colors
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, hyatt, joepeck, koivisto, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Patch] Proposed Fix
none
[Patch] Proposed Fix
none
[Image] body.window-inactive * {} vs. body.window-inactive {}
none
[Patch] Proposed Fix none

Description Matt Baker 2016-02-16 11:35:52 PST
* SUMMARY
Add CSS variables for common border/background colors. The following colors are used in dozens of places and should be CSS variables:

--border-color: hsl(0, 0%, 70%);
--border-inactive-color: hsl(0, 0%, 85%);
--panel-background-color: hsl(0, 0%, 94%);
Comment 1 Radar WebKit Bug Importer 2016-02-16 11:36:22 PST
<rdar://problem/24680944>
Comment 2 Matt Baker 2016-02-16 11:46:01 PST
Created attachment 271459 [details]
[Patch] Proposed Fix
Comment 3 Timothy Hatcher 2016-02-16 11:50:59 PST
Comment on attachment 271459 [details]
[Patch] Proposed Fix

In theory we can just have —border-color, and override the color var in body.window-inactive { }. Then delete all the rules with body.window-inactive!
Comment 4 Matt Baker 2016-02-16 21:43:24 PST
Created attachment 271535 [details]
[Patch] Proposed Fix
Comment 5 Matt Baker 2016-02-16 21:44:47 PST
(In reply to comment #3)
> Comment on attachment 271459 [details]
> [Patch] Proposed Fix
> 
> In theory we can just have —border-color, and override the color var in
> body.window-inactive { }. Then delete all the rules with
> body.window-inactive!

Worked perfectly! It felt great removing all those rules.
Comment 6 Timothy Hatcher 2016-02-16 22:30:25 PST
Comment on attachment 271535 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/Variables.css:58
> +body.window-inactive * {

You should be able to remove the *. Vars are inherited, so it can stay on the body.
Comment 7 Matt Baker 2016-02-16 23:35:19 PST
Created attachment 271543 [details]
[Image] body.window-inactive * {} vs. body.window-inactive {}

Without selecting all elements "*", a few borders aren't styled correctly. Weird.
Comment 8 Joseph Pecoraro 2016-02-17 01:08:39 PST
Comment on attachment 271535 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/Variables.css:29
> +    --z-index-resizer: 256;`

This character looks errant. cq- to fix it.
Comment 9 Joseph Pecoraro 2016-02-17 01:13:47 PST
(In reply to comment #7)
> Created attachment 271543 [details]
> [Image] body.window-inactive * {} vs. body.window-inactive {}
> 
> Without selecting all elements "*", a few borders aren't styled correctly.
> Weird.

When you inspect the nav bar / other things that have a darker border, what is the cascade? I assume that this is just a specificity issue, and that adding the * gave this just enough extra specificity? Otherwise, I don't see why it would change things.

--

Using CSS Variables allowed us to reduce 23, often complex, rules down to 1. This is a great story for custom properties and their cascade rules.
Comment 10 Timothy Hatcher 2016-02-17 07:11:27 PST
(In reply to comment #9)
> (In reply to comment #7)
> > Created attachment 271543 [details]
> > [Image] body.window-inactive * {} vs. body.window-inactive {}
> > 
> > Without selecting all elements "*", a few borders aren't styled correctly.
> > Weird.
> 
> When you inspect the nav bar / other things that have a darker border, what
> is the cascade? I assume that this is just a specificity issue, and that
> adding the * gave this just enough extra specificity? Otherwise, I don't see
> why it would change things.

Note you might need my most recent changes for the variable rules to show up in the sidebar. Does !important on the --border-color help?

> Using CSS Variables allowed us to reduce 23, often complex, rules down to 1.
> This is a great story for custom properties and their cascade rules.

Yep!
Comment 11 Matt Baker 2016-02-17 11:53:56 PST
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #7)
> > > Created attachment 271543 [details]
> > > [Image] body.window-inactive * {} vs. body.window-inactive {}
> > > 
> > > Without selecting all elements "*", a few borders aren't styled correctly.
> > > Weird.
> > 
> > When you inspect the nav bar / other things that have a darker border, what
> > is the cascade? I assume that this is just a specificity issue, and that
> > adding the * gave this just enough extra specificity? Otherwise, I don't see
> > why it would change things.
> 
> Note you might need my most recent changes for the variable rules to show up
> in the sidebar. Does !important on the --border-color help?

Sadly, no.
Comment 12 Matt Baker 2016-02-17 11:55:30 PST
Created attachment 271573 [details]
[Patch] Proposed Fix
Comment 13 WebKit Commit Bot 2016-02-17 12:29:49 PST
Comment on attachment 271573 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 271573

Committed r196713: <http://trac.webkit.org/changeset/196713>
Comment 14 WebKit Commit Bot 2016-02-17 12:29:55 PST
All reviewed patches have been landed.  Closing bug.