Closed
Bug 1568786
Opened 4 years ago
Closed 4 years ago
In devtools/client/themes/variables.css, move shared variables above theme-specific ones
Categories
(DevTools :: General, task, P3)
DevTools
General
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);
…
}
Assignee | ||
Comment 1•4 years ago
|
||
Assignee | ||
Comment 2•4 years ago
•
|
||
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?
Updated•4 years ago
|
Priority: -- → P3
Pushed by florens@fvsch.com: https://hg.mozilla.org/integration/autoland/rev/40c9d69858a4 Move global CSS variables before theme variables; r=mtigley
Comment 4•4 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox70:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
Updated•4 years ago
|
Assignee: nobody → florens
You need to log in
before you can comment on or make changes to this bug.
Description
•