Update the theme colors to rely on CSS variables #23048
Conversation
youknowriad
added
General Interface
youknowriad
requested review from
ajitbohra,
chrisvanpatten,
ellatrix,
gziolo,
mkaz,
mkevins,
nerrad,
ntwb,
pinarol,
Soean and
talldan
as code owners
|
HECK YES. I'm closing mine in favor of this. With regards to the darker versions — I think we might be able to retire those entirely! The editor only really needs a single spot color, and we don't really need the additional ones in practice. We did for wp-admin due to how the themes were created, but I think one is sufficient. Here's where they're used:
The next one here is lighter, but it doesn't have to be, there's no benefit to that: This one is also lighter, and doesn't have to be:
All those could use the same blue. I believe that initially we added at least a secondary color because in some cases like the Midnight theme, the red spot color conflicts with the "is-destructive" red color, so we had an alternate:
But the solution to that one, I think, is to maybe not use the red spot color from Midnight at all. Or if we do it, well, that's on you. Headings are colored, by the way, they probably shouldn't be:
I'm happy to jump in, maybe tomorrow, and help retire some of these, make sure things look good. But this is really nice work! |
|
There's a really weird failure when you run I tried debugging but I don't understand, when I run |
|
Nothing immediately comes to mind, I was going to run the CI and "Debug job", but I don't see this enabled on Travis CI for us, I also cannot find a setting to enable this 🤔
|
| @@ -4,7 +4,7 @@ | |||
| const { adminColorSchemes } = require( '@wordpress/base-styles' ); | |||
|
|
|||
| module.exports = [ | |||
| require( 'postcss-custom-properties' )(), | |||
| require( '@wordpress/postcss-themes' )( adminColorSchemes ), | |||
There was a problem hiding this comment.
If we switch these two presets priorities, the admin schemes won't work on IE11 (the default one still works) but we'll gain in terms of size of the production CSS build. I think it's fine to do it but I'll bring this to #core-css meetings.
There was a problem hiding this comment.
The postcss-themes plugin also causes issue with over-specificity of color scheme rules, that's why the inspector panel headings are colored. The generated rule body.admin-color-ectoplasm .components-button[aria-expanded="true"] is overriding the "correct" .components-panel__body-toggle.components-button rule, which is set to plain black, not a color scheme color.
If these are swapped, only the :root is generated for each scheme, which fixes that specificity issue.
IMO it's okay to only have default on IE11, since there are these other drawbacks to keeping support.
There was a problem hiding this comment.
I updated the setup, it's way better now. IE11 only shows the default colors.
|
Size Change: -9.37 kB (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
|
As you can see here #23048 (comment) this increases the CSS bundle size because we need fallbacks for the CSS variable usage. That said, if this is acceptable #23048 (comment) we might even gain in CSS size :) I do think it's fine to move forward either way though. |
|
I fixed the production build issue btw. |
youknowriad
force-pushed
the
update/theme-colors-css-variables
branch
from
9886e96
to
190b041
Compare
|
Thanks for your help here @ryelle and @jasmussen I'm really excited to see this land. |
|
This is great and should help in building a new more vivid color scheme :) |
|
This is all good, but why is converting admin colors to CSS variables a Gutenberg-specific thing? |
Gutenberg is in core, but it is something that is developed outside of WP and used by WP. The CSS meetings have been all about the audit of the admin CSS and implementing the admin color schemes with CSS variables, so it is being worked on in core also. Join in at the #core-css channel on Slack. |
|
Sorry, yes, I appreciate that, and sorry if I'm being dim.
|
|
Don't put "Gutenberg the editor" in the same box as "Gutenberg the plugin that is for more parts of the admin than just the editor". Right now, users can disable the block editor. That won't apply to the parts of Gutenberg that become the WP admin. |
|
However much the definition of the term "Gutenberg" expands - it could become the new OS for the ISS - my point still stands. |
|
I'm not sure why you are arguing on this issue. Please reference the issues at
Global Styles
|
youknowriad
removed
the
Needs Dev Note
Following these changes: - WordPress/gutenberg#23454 - WordPress/gutenberg#23648 - WordPress/gutenberg#23048
Following these changes: - WordPress/gutenberg#23454 - WordPress/gutenberg#23648 - WordPress/gutenberg#23048
Following these changes: - WordPress/gutenberg#23454 - WordPress/gutenberg#23648 - WordPress/gutenberg#23048
An alternative to #20460
This PR updates the theme (admin scheme) colors to use CSS variables. So to achieve high-luminance colors on a specific context, it should be as easy as doing something like
Notes