Convert stylesheets to use css custom properties #2755
Conversation
|
Thanks! Yeah, it's a bit big!
…have you seen a path to removing Sass entirely? That'd be fun. |
Cool, I'll work on breaking it up.
I think so. I haven't done a full audit or anything but css has come a long way. I think the blocking factor would be which old browsers does administrate need to support. |
edwardloveall
force-pushed
the
el-css-vars
branch
2 times, most recently
from
543aa98 to
e5a78ea
Compare
edwardloveall
changed the title
edwardloveall
changed the title
|
Alright here's a start with a much smaller scope. I see two ways forward:
Preference? I lean toward the one branch thing since it's less branch juggling for me, but you probably have more to keep track of as maintainer so I think this is up to you. |
|
One branch is better, I think, especially if it's easier for you. |
edwardloveall
force-pushed
the
el-css-vars
branch
from
682b0c6 to
06503e3
Compare
|
|
||
| &:hover { | ||
| color: mix($black, $action-color, 25%); | ||
| color: var(--action-color-active); |
There was a problem hiding this comment.
This changed ever so slightly. There were two selectors that used $action-color as their base. This one mixes in 25% $black and &:not(.link):hover in buttons.scss mixes in 20% $black. I went with the 20% in both places so we only had one "it's like $action-color but tinted" variable because button backgrounds are more prominant than text hovers.
edwardloveall
force-pushed
the
el-css-vars
branch
from
ccd3c7c to
229cf20
Compare
There was a problem hiding this comment.
As noted in the commit message, this is quite a bit bigger than it's scss counterpart. I was seeing if I could really remove all sass variables, and I did! Also, in about a year it's probably pretty safe to use color-mix and this gets way smaller (as seen in the comment at the start of the file).
|
Alright, this is ready. I tried to call out a couple places that are a bit messy or surprising. However, this replaces all sass variables with css variables. It's slightly more verbose because sass can still do more than css at this point, but give it about a year and (also, updated the PR description) |
|
Nice, this looks great! I'm going to sit on it for a little bit, as I work out where it fits inside the release. What else would we want to do for removing Sass? |
|
Of the top of my head, assuming you're threshold for browser support is baseline widely available:
|
|
Is this on track with version 1.0.0 release? |
|
Can we merge this? :) |
|
It looks like we've got some conflicts to fix, which @edwardloveall might be best placed to do. Then as long as we check off the last few things on the list above, we'll be in a good place to merge it. |
I'd like to move away from sass so that we can fully embrace css custom properties and variables. [color-mix](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value /color-mix) and [relative colors](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_colors/Rela tive_colors) are not yet [baseline widely available](https://developer.mozilla.org/en-US/docs/Glossary/Baseline/Co mpatibility) so I'm deciding to inline them. This inlines just one: a base border color in its hover state.
These are quite a bit more verbose due to the lack of css mixins and color functions. Once we feel like we can support color-mix, we can remove all the duplicate css.
sass has a function called [rgb], and so does [css]. If I try to use the css rgb in a sass context, sass tries to use it first. If I'm using the [relative color syntax], sass complains that I'm passing too many arguments. One way around this is to use a css property to define whatever I need and then use that instead. [rgb]: https://sass-lang.com/documentation/modules/#rgb [css]: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/rgb [relative color syntax]: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/rgb#using_relative_colors_with_rgb
We want to make way for a grey between grey-0 and the old grey-1
edwardloveall
force-pushed
the
el-css-vars
branch
from
229cf20 to
49e54b8
Compare
|
Rebased.
What do you think about merging this as-is? The list above was for completely removing sass. I could take it on in this PR, but there's a lot here already and all the changes are similar to each other. |
|
Thank you! Oh! Right, yes. I misread. Excellent. Yeah, we shouldn't do that in this PR for sure. I'll read over this again and then I think we can merge it. |
3 participants
Supporting #2732
This converts all sass color variables (and a couple other variables) to use native css variables instead.
There's a lot here! If you'd prefer, I can figure out how to break this up into smaller parts; maybe one per var or something. It also doesn't get rid of all sass variables. I could go even farther and do that if we want. Just let me know.Almost all variables are split into their own commit for easier review.
I also made sure you can still override variables and I'm happy to report that I followed the existing steps and it worked great.
A related consideration is I know administrate is in its 1.0-beta phase. Is this a breaking change worth making this late in the process? It will surely break current sass variable overrides as those no longer exist to be overridden.
Finally, this is also a very large step in the direction away from sass. While there would still be some work to do to fully remove sass, it's a lot more possible if you'd want to do that (in a separate PR).