1412273 - CSS shapes editor swatch icon invisible in dark theme

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[Sebastian Zartner [:sebo]]

Attachment: Invisible shapes editor swatch icon

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

Comment 1

[Mike Park [:mparkms]]

Attachment: Bug 1412273 - Adjust CSS shapes highlighter toggle to be visible in dark theme.

Review commit: https://reviewboard.mozilla.org/r/195852/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195852/

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

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.

Comment 3

[Julian Descottes [: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.

Comment 4

[Mike Park [:mparkms]]

Comment on attachment 8924617 [details]
Bug 1412273 - Adjust CSS shapes highlighter toggle to be visible in dark theme.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/195852/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90453cc7c3b4
Adjust CSS shapes highlighter toggle to be visible in dark theme. r=jdescottes

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/90453cc7c3b4

Comment 7

[Sebastian Zartner [:sebo]]

Works for me using Nightly 59.0a1 (2017-11-13)! Thank you for the fast fix!

Sebastian