1215804 - Move tools CSS theme variables to variables.css

Status: NEW

(DevTools :: Shared Components, defect, P4)

Description

[Patrick Brosset <:pbro>]

Tool panels have their own CSS files. Often, these CSS files define their own CSS variables that depend on the current theme.
For example, in performance.css:

.theme-dark {
  --cell-border-color: rgba(255,255,255,0.15);
  --cell-border-color-light: rgba(255,255,255,0.1);
  --focus-cell-border-color: rgba(255,255,255,0.5);
  --row-alt-background-color: rgba(29,79,115,0.15);
  --row-hover-background-color: rgba(29,79,115,0.25);
}

.theme-light {
  --cell-border-color: rgba(0,0,0,0.15);
  --cell-border-color-light: rgba(0,0,0,0.1);
  --focus-cell-border-color: rgba(0,0,0,0.3);
  --row-alt-background-color: rgba(76,158,217,0.1);
  --row-hover-background-color: rgba(76,158,217,0.2);
}

This is probably fine as long as we have 2 themes, but that makes creating new themes really hard.
If someone wanted to create a new theme, they'd have to first define a new set of variables in variables.css and then find all other CSS files that define colors based on the current theme.

I think we should move the tool-specific color variables inside variables.css too.
It might make the file a lot bigger than it is today, but at least all colors relative to a theme would be in one place.

Related: today we have just 1 variables.css and 1 common theme css file per theme: light-theme.css and dark-theme.css.
I think we should have 1 variables css file *per* theme, and then just 1 common theme css file:
variables-light.css
variables-dark.css
variables-mynewtheme.css
and
theme.css, which would contain all of the code that's today duplicated between light-theme.css and dark-theme.css

Comment 1

[Brian Grinstead [:bgrins]]

Are you suggesting we delete light-theme.css and dark-theme.css?  At some point I'd like to do that, but it will be quite some work right now since there are a fair number of differences between them - run `sdiff devtools/client/themes/light-theme.css devtools/client/themes/dark-theme.css` to see side by side.

Just throwing in a couple more thoughts about the future plans for CSS in the tools:

* I'd like to break toolbars.inc.css up into a couple of pieces since it does way more than toolbars.  Many of the shared things in there could be the start of a theme.css.
* Get rid of preprocessor dependency especially for toolbars.inc.css and light-theme/dark-theme.css (Bug 1183280)

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #0)
> Related: today we have just 1 variables.css and 1 common theme css file per
> theme: light-theme.css and dark-theme.css.
> I think we should have 1 variables css file *per* theme, and then just 1
> common theme css file:
> variables-light.css
> variables-dark.css
> variables-mynewtheme.css
> and
> theme.css, which would contain all of the code that's today duplicated
> between light-theme.css and dark-theme.css

Having a single variables.css file is ideal for things like Bug 1211918.  I guess we could have variables.css imported from the themes and then that imports variables-light.css and variables-dark.css, if it didn't have a perf hit.

Comment 3

[Patrick Brosset <:pbro>]

So, about:
> I think we should move the tool-specific color variables inside variables.css too
Brian and I discussed on IRC:
* moving tool-specific variables into different files migh be making some promises about what's themeable that will break between versions,
* we also have to worry about naming conflicts / clarity about what they are used for (so then tool vars would maybe need to be prefixed like —rule-view-error-color or something)

So, the first thing we have to do in this bug is audit the CSS files that use the .light/.dark-theme classes and see what they do with it. It could be that they're actually used to define theme-related variables, in which case those could be moved to variables.css. But it could also well be that they should be replaced with existing theme variables instead.