1568786 - In devtools/client/themes/variables.css, move shared variables above theme-specific ones

Status: RESOLVED FIXED

(DevTools :: General, task, P3)

Description

[Florens Verschelde :fvsch]

The devtools/client/themes/variables.css file is structured as:

.theme-light {
  --theme-body-color: var(--grey-70);
  …
}

.theme-dark {
  --theme-body-color: var(--grey-40);
  …
}

:root {
  --grey-40: …;
  --grey-70: …;
  …
}

For contributors coming to the project, it would make more sense to organize this file from most generic to more particular, with the shared variables like Photon colors declared first and the .theme-(light|dark) ones coming after.

I've asked around and the current structure is just a result of how things acrued over time.

I propose changing the order to:

:root {
  --grey-40: …;
  --grey-70: …;
  …
}

.theme-light {
  --theme-body-color: var(--grey-70);
  …
}

.theme-dark {
  --theme-body-color: var(--grey-40);
  …
}

Comment 1

[Florens Verschelde :fvsch]

Attachment: Bug 1568786 - Move global CSS variables before theme variables; r=mtigley

Comment 2

[Florens Verschelde :fvsch]

So the only downside is that we risk to obfuscate history.

Trying out this change with a patch.

• Phabricator seems to understand that lines were moved.
• hg annotate shows all the moved lines as changed by the patch's changeset (all the :root {…} styles which are currently at the end).
• I suspect https://searchfox.org/mozilla-central/source/devtools/client/themes/variables.css uses hg annotate?

Thankfully the only affected lines are the ones for shared variables, and it's mostly Photon colors, so we don't hide the more relevant history. But still a trade-off to consider. Any thoughts?

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/40c9d69858a4
Move global CSS variables before theme variables; r=mtigley

Comment 4

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/40c9d69858a4