Closed Bug 1399028 Opened 2 years ago Closed 2 years ago

Photon update - DevTools tabs colors

Categories

(DevTools :: General, defect, P1)

defect

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(2 files)

Should match https://docs.google.com/document/d/1AJ7oHC7akXJiliheHjhFc4fTonaN31Eo_P2M8gssL8s/edit

- change the icon color when selected
- change the text color when selected
- on hover should only change the background color + have a top border
Summary: DevTools tabs photon colors → Photon update - DevTools tabs colors
Duplicate of this bug: 1399030
For the record, just stumbled upon https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-context-properties 
We could use this instead of filters to assign opacity and fill colors to our SVGs.
Comment on attachment 8907061 [details]
Bug 1399028 - fix assert messages in browser_theme.js test;

https://reviewboard.mozilla.org/r/178782/#review183990
Attachment #8907061 - Flags: review+
Comment on attachment 8906936 [details]
Bug 1399028 - photon update - devtools icon, tab and accordion colors;

https://reviewboard.mozilla.org/r/178682/#review184022

::: devtools/client/themes/toolbars.css:19
(Diff revision 3)
> -  --icon-filter: none;
> -  --checked-icon-filter: url(chrome://devtools/skin/images/filters.svg#checked-icon-state);
> -
>    --separator-border-image: linear-gradient(transparent 4px, rgba(0,0,0,.1) 4px, rgba(0,0,0,.1) calc(100% - 4px), transparent calc(100% - 4px));
> +
> +  --toolbar-selected-color: var(--blue-60);

This should be moved below --searchbox color variables with a new line separating them.

::: devtools/client/themes/toolbars.css:19
(Diff revision 3)
> -  --icon-filter: none;
> -  --checked-icon-filter: url(chrome://devtools/skin/images/filters.svg#checked-icon-state);
> -
>    --separator-border-image: linear-gradient(transparent 4px, rgba(0,0,0,.1) 4px, rgba(0,0,0,.1) calc(100% - 4px), transparent calc(100% - 4px));
> +
> +  --toolbar-selected-color: var(--blue-60);

I think we should after all just have these be a theme variables like in revision 2. Since we have --theme-toolbar-color in variables.css, we should also have the --theme-toolbar-selected-color there as well. I realized that I defined --theme-toolbar-color without adding it to the browser_theme.js test.

::: devtools/client/themes/variables.css:24
(Diff revision 3)
>  :root.theme-light {
>    --theme-body-background: white;
>    --theme-sidebar-background: white;
>    --theme-contrast-background: #e6b064;
>  
> +  /* Icon filters */

Let's move these Icon filters after Images. It's current placement hinders the flow of looking at the color variables.
Comment on attachment 8906936 [details]
Bug 1399028 - photon update - devtools icon, tab and accordion colors;

https://reviewboard.mozilla.org/r/178682/#review184092
Attachment #8906936 - Flags: review?(gl) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7af29b83ac3
photon update - devtools icon, tab and accordion colors;r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5aedf62e0af
fix assert messages in browser_theme.js test;r=gl
https://hg.mozilla.org/mozilla-central/rev/d7af29b83ac3
https://hg.mozilla.org/mozilla-central/rev/c5aedf62e0af
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Depends on: 1399886
Depends on: 1413478
Depends on: 1424234
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.