1215557 - Common styles between dark and light theme should live in a different file

Status: NEW

(DevTools :: Shared Components, defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

We have non-color styles duplicated styles between light-theme.css and dark-theme.css that should be shared in one file, for styles that affect the overall structure, but not color. bgrins suggested a `shared.css` file.

Comment 1

[Brian Grinstead [:bgrins]]

Running sdiff devtools/client/themes/light-theme.css devtools/client/themes/dark-theme.css it looks like a few categories of differences:

1) A lot of differences with Tern styles
2) Different images / image regions for dark / light theme.  i.e. chrome://devtools/skin/tooltip/arrow-vertical-dark.png vs chrome://devtools/skin/tooltip/arrow-vertical-light.png.
3) Hardcoded colors (non-Tern)

We could start cleaning this up before moving things into a shared.css file.  Extracting hardcoded colors into variables would be a good start.  For 2, we already have a solution in place for this with the invert filter, so in both themes we could use the lighter variation of the image and then invert it for the light theme (may need new assets made in the mean time).

Comment 2

[Florens Verschelde :fvsch]

I'd like to work on this.

Would it be a good idea to take everything that is identical in light|dark-theme.css and move it:

• to a specific component stylesheet if it seems to override styles for a specific component
• to common.css otherwise
  ?

Tim, any advice?

Comment 3

[Tim Nguyen :ntim]

Hey Florens, thanks for looking into this! As Brian says in comment 1, it would be a good idea to first unify both files to be identical (by re-using or creating CSS variables).

Regarding component stylesheets vs common.css, it really depends on how we prefer maintaining this code:

Moving things in component stylesheets is less prone to leaving unused code behind, since component CSS files are removed along with their JS counterpart, but also more prone to styling inconsistencies/code duplication.

Having everything in common.css allows less code/variable duplication, hence less styling inconsistencies thanks to code sharing, but also is more prone to leaving unused code behind.

I would personally put what's unlikely to become unused in a shared.css/common.css file (body {margin: 0}, .theme-body, .theme-sidebar, .devtools-toolbar, etc.). And stuff specific to a component (.variable-or-property - eg. old variables view stuff, .rule/computedview-swatch) in the component CSS. I'm less categorical about the CM stuff because there really isn't any component CSS file for it and it's also unlikely going to become unused :).

But honestly, either way or a mix of both is fine as long as the concerns are addressed one way or another (doesn't necessarily have to be in how files are split, can also be in documentation/tooling etc). I've mentioned the preferences which work out well for me, since they try to minimize all the concerns I mentioned above without extra doc/tooling effort, but they might not work as well for others.