Closed Bug 1412273 Opened 4 years ago Closed 4 years ago

CSS shapes editor swatch icon invisible in dark theme

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(firefox58 verified)

VERIFIED FIXED
Firefox 58
Tracking Status
firefox58 --- verified

People

(Reporter: sebo, Assigned: mpark)

References

(Blocks 1 open bug)

Details

(Whiteboard: [designer-tools])

Attachments

(2 files)

The swatch icon used to toggle the CSS shapes editor is invisible in the dark theme, because it's black on very dark gray.

Sebastian
Assignee: nobody → mpark
Status: NEW → ASSIGNED
Comment on attachment 8924617 [details]
Bug 1412273 - Adjust CSS shapes highlighter toggle to be visible in dark theme.

https://reviewboard.mozilla.org/r/195852/#review201164

I don't think we quite stop using SVG filters for these grid and shape buttons in the rules view. See https://bugzilla.mozilla.org/show_bug.cgi?id=1399886. I am gonna pass this onto jdescottes to advise.

::: devtools/client/themes/rules.css:6
(Diff revision 1)
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* CSS Variables specific to this panel that aren't defined by the themes */
>  .theme-light {

We should change this to :root { }

::: devtools/client/themes/rules.css:13
(Diff revision 1)
>    --rule-overridden-item-border-color: var(--theme-content-color3);
>    --rule-header-background-color: var(--theme-toolbar-background);
> +  --rule-shape-toggle-color: #0b0b0b;
>  }
>  
>  .theme-dark {

Change this to :root.theme-dark

::: devtools/client/themes/rules.css:20
(Diff revision 1)
>    --rule-overridden-item-border-color: var(--theme-content-color1);
>    --rule-header-background-color: #141416;
> +  --rule-shape-toggle-color: #fdfdfd;
>  }
>  
>  .theme-firebug {

Change this to :root.theme-firebug

::: devtools/client/themes/rules.css:26
(Diff revision 1)
>    --rule-highlight-background-color: var(--theme-highlight-yellow);
>    --rule-property-name: darkgreen;
>    --rule-property-value: darkblue;
>    --rule-overridden-item-border-color: var(--theme-content-color2);
>    --rule-header-background-color: var(--theme-header-background);
> +  --rule-shape-toggle-color: #0b0b0b;

Firebug theme actually have .theme-light.theme-firebug set at root. So, we don't need to repeat the light theme variable unless it is different from the light theme.
Attachment #8924617 - Flags: review?(gl) → review?(jdescottes)
Comment on attachment 8924617 [details]
Bug 1412273 - Adjust CSS shapes highlighter toggle to be visible in dark theme.

https://reviewboard.mozilla.org/r/195852/#review201318

The shape and grid toggles are still using the filters to indicate their "active" state, but that's compatible with this change as far as I can see.
I'm okay this change, and I just have one suggestion.

Right now the grid toggle uses the same color in all themes: #696969 . Maybe you could do the same here for consistency?
Since the shapes toggle reuses the shader editor tool icon (temporary I guess?), it is not as crisp as the grid icon but #696969 is still readable in all themes.

If you prefer to go with different colors for each theme, we could reuse photon colors here: --grey-90 for the light-theme and --grey-10 for the dark theme.
Attachment #8924617 - Flags: review?(jdescottes) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/90453cc7c3b4
Adjust CSS shapes highlighter toggle to be visible in dark theme. r=jdescottes
Keywords: checkin-needed
Attachment #8924617 - Flags: review?(gl)
https://hg.mozilla.org/mozilla-central/rev/90453cc7c3b4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Works for me using Nightly 59.0a1 (2017-11-13)! Thank you for the fast fix!

Sebastian
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.