Remove/tweak dark light variables #23229
Conversation
jasmussen
added
[Type] Enhancement
|
Size Change: -1.26 kB (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
packages/base-styles/_mixins.scss
Outdated
|
|
||
| // Darker shades. | ||
| --wp-admin-theme-color-darker-10: hsl(#{$color-primary-h}, #{$color-primary-s}, #{$color-primary-l - 5}); | ||
| --wp-admin-theme-color-darker-20: hsl(#{$color-primary-h}, #{$color-primary-s}, #{$color-primary-l - 10}); |
There was a problem hiding this comment.
I'd love to get rid of these two variables as well at some point to allow changing the colors by just defining a single CSS variable :)
This is still a great improvement thought. I don't think hsl bring value though since we're able to use SASS here. What's the reasoning?
There was a problem hiding this comment.
I could make it a mixin.
Hsl is native browser, so we can actually retire the variables and have lighter and darker colors from a single color.
Will try in a bit
There was a problem hiding this comment.
Hsl is native browser, so we can actually retire the variables and have lighter and darker colors from a single color.
We could but only if we use three CSS variables instead of one which IMO is not great.
There was a problem hiding this comment.
I do see your point.
You could limit it to two:
--wp-admin-theme-color-hs: '200, 100%';
--wp-admin-theme-color: hsl( var(--wp-admin-theme-color-hs), 36% );
Then you could stitch together variables in the code like this, instead of defining them as new colors:
button {
background-color: hsl( var(--wp-admin-theme-color-hs), 26% ); // 10% darker
}
You could even do this:
$wp-admin-theme-color-10: hsl( var(--wp-admin-theme-color-hs), 26% );
Make sense?
There was a problem hiding this comment.
I think it's better but If I want to tweak the color, I'd have to define --wp-admin-theme-color-hs instead of --wp-admin-theme-color right?
I mean, we're kind of forcing third-party API to use HSL too; Maybe that's fine but how confident are we that this will last forever?
There was a problem hiding this comment.
Actually, I don't like that :) I'd rather have three variables over having to define the colors in specific format. And it's not even the full format.
There was a problem hiding this comment.
i guess what I'm saying is that i personally prefer to just leave it as is for now. Avoid hsl entirely.
There was a problem hiding this comment.
Alright, I don't disagree. I've reverted the HSL magic for now, and we're back to having two additional dark colors.
We could remove one of them because a hover color isn't strictly necessary. But for the time being, I've kept it.
| border-color: var(--wp-admin-theme-color-lighter-10); | ||
| color: rgba($white, 0.4); | ||
| background: var(--wp-admin-theme-color); | ||
| border-color: var(--wp-admin-theme-color); |
There was a problem hiding this comment.
I like the simplification here.
jasmussen
force-pushed
the
fix/remove-light-dark-variables
branch
from
d2a6bc4
to
14c6e65
Compare
|
Tests are passing! Yaay! |
The admin UI now uses CSS variables. But the SASS functions for lightening and darkening colors are not accurate, causing too saturated a disabled button:
Additionally, it added some complexity as the variables could not be calculated from a single color.
This PR fixes both of these, using a clever trick from https://stackoverflow.com/questions/1625681/dynamically-change-color-to-lighter-or-darker-by-percentage-css-javascript.
The light colors I simply retired in favor of opacity:
Now they match disabled buttons elsewhere in the admin.
The dark colors are still there:
But they now are rewritten using HSL so that the colors actually can be recalculated.