Closed Bug 1568786 Opened 4 months ago Closed 4 months ago

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

Categories

(DevTools :: General, task, P3)

task

Tracking

(firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: fvsch, Assigned: fvsch)

Details

Attachments

(1 file)

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);
  …
}

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

Trying out this change with a patch.

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?

Priority: -- → P3
Pushed by florens@fvsch.com:
https://hg.mozilla.org/integration/autoland/rev/40c9d69858a4
Move global CSS variables before theme variables; r=mtigley
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
Assignee: nobody → florens
You need to log in before you can comment on or make changes to this bug.