Components: Add themeable background color #45466
Conversation
|
|
|
Size Change: +725 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
| @@ -7,11 +7,11 @@ | |||
| } | |||
|
|
|||
| .components-tooltip .components-popover__content { | |||
| background: $gray-900; | |||
| background: $components-color-foreground; // TODO: Discuss with designers. | |||
There was a problem hiding this comment.
Tooltips could be a case where designers want a different shade when in dark mode.
There was a problem hiding this comment.
Yes indeed, they can be much lighter. I don't think that needs to block anything here, though, as the white text inside a dark background will always be legible.
| background, | ||
| ...props | ||
| }: WordPressComponentProps< ThemeProps, 'div', true > ) { | ||
| const themeVariables = generateThemeVariables( { accent, background } ); |
There was a problem hiding this comment.
The next thing we should do is to take these themeVariables values and make them available to descendant components via Context. This will allow us to resolve the remaining TODO comments in Button, where Button needs to access the color values directly and compute its non-standard colors in JS.
There was a problem hiding this comment.
Here's a few considerations (almost a brainstorming sessions) that came to mind when thinking about using React Context:
- are we ok exposing the variables both in React Context and as CSS variables ?
- Which one is the source of truth?
- Is there a scenario in which they could not be in sync ?
- CSS vars will be accessible to DOM descendants, while React Content will be accessible to all react child components — which gives us the chance of using one or the other when styling Slot-Fill components (e.g Popover)
There was a problem hiding this comment.
After ruminating on this, I'll backtrack — we shouldn't use React Context in combination with the original DOM-based scoping system 😅 It would cause a lot of complications like you mention.
So I guess we really need to avoid Sass color functions as much as possible, and maybe consider exposing HSL-split CSS vars (--h --s --l) for things that are absolutely necessary 😕
| } | ||
|
|
||
| &:disabled, | ||
| &:disabled:active:enabled, | ||
| &[aria-disabled="true"], | ||
| &[aria-disabled="true"]:enabled, // This catches a situation where a Button is aria-disabled, but not disabled. | ||
| &[aria-disabled="true"]:active:enabled { | ||
| // TODO: This needs to be accent-inverted |
There was a problem hiding this comment.
The three TODO comments in this stylesheet flag the parts that rely on Sass functions, which will not work when the underlying values are CSS variables.
There are two ways we can address these:
- Work with designers and see if any of the Sass function parts can be replaced with the standard gray-100 etc. colors. This is the easiest and preferred way when designing themeable components.
- In the Button component, access the raw theme color values (not CSS variables) via Context, and calculate the necessary colors in JS.
There was a problem hiding this comment.
Work with designers and see if any of the Sass function parts can be replaced with the standard gray-100 etc. colors. This is the easiest and preferred way when designing themeable components.
Agreed, this would be my preferred option too — it would simplify the code and its readability too.
There was a problem hiding this comment.
Can you expand a bit on what the ask is here? Is it changing out of the main color?
I like to think that we can reach a point where we have a set of system colors with their known contrast properties in light and dark mode (the grays), and then provide a single accent color to a component, and whatever mode it's in defines what colors get applied. That is, ideally I should never have to feed more than a single color along with the dark or light mode prop to a component, and it handles the rest itself. Is that useful?
There was a problem hiding this comment.
Can you expand a bit on what the ask is here? Is it changing out of the main color?
To be clear, I'm not asking for anything specific yet in this PR 🙂 We'll make follow-up PRs with more detail, suggest some alternatives, and ping the design team there for discussion.
As an overview of the problem though, we can no longer easily use Sass functions like this, which require the color to be predetermined:
☝️ This button's text color needs to be either white or black depending on the arbitrary accent color. But we can't use CSS variables as inputs to Sass functions. Things like rgba(var(--my-color), 0.4) are not possible.
gutenberg/packages/components/src/button/style.scss
Lines 131 to 132 in 2b74866
gutenberg/packages/components/src/button/style.scss
Lines 246 to 252 in 2b74866
☝️ Similarly in these two examples, we can't do things like lighten(var(--gray-700), 5%).
the dark or light mode prop
I'm using the term "dark mode" for convenience, but we are currently working toward a more generic solution which allows consumers to pick their own background color. In theory this means any color, but in practice it means consumers should be able to choose their own shade of "dark" when doing "dark mode". An app may want #0a0a0a, another may want #1d1f21, etc.
mirka
changed the title
|
This is really really cool @mirka! I haven't had time to dive too deep into this but if you haven't already I'd recommend checking out how https://linear.app/ handles themes as they take a similar approach that works really well in practice. There is also https://hihayk.github.io/shaper/ which takes a single hue to generate background and foreground scales including dark mode. By default the grey scales it generates are desaturated. It would be nice to work towards a point where we can turn a single accent colour into a wheel of functional scales (like Radix) including neutral/grey that the community can latch on to. |
|
By the way, there is probably some learnings we can take from this work and apply to themes in future. e.g. generate standardised, functional colour scales that patterns can reference and theme authors can lean on. |
|
Thank you for the links, @SaxonF ! Your input / feedback can be really valuable here :)
Absolutely! Although let's keep in mind that the work that we're currently doing is aimed exclusively at the |
|
Thanks @SaxonF, I took a look at your links. I really liked Shaper, it's a great way to visualize how design system variables interact! The "Specs" mode and the naming scheme are pretty good references too. Shaper's color system seems quite cohesive and flexible, largely thanks to the saturation parameter. I hadn't considered using saturation as a key variable. We are currently working toward a little bit more low-level flexibility (i.e. arbitrary background color support), but if it turns out that we do need the predictability of a dark/light "boolean" background, Shaper's approach could be a good fallback plan. |
|
@ciampo This is ready for another 👀 |
There was a problem hiding this comment.
Great, great job. The color algorithm section is well written and already looks great, especially given it's a first iteration! And I couldn't spot any regressions when manually testing the Button Storybook examples.
I left a few new minor comments and replied to some older ones, but we're very close from being able to land this PR.
(could you also add a CHANGELOG entry?)
I also went ahead and added a few more points to the tracking issue so that we don't lose track of all the possible directions discussed.
| // Darkness of COLORS.gray[ 900 ], relative to #fff | ||
| const limit = 0.884; | ||
|
|
||
| const direction = colord( background ).isDark() ? 'lighten' : 'darken'; |
There was a problem hiding this comment.
Currently, gray-100 could be the brightest or the darkest color in the scale of grays, depending on whether the background color is classified as "dark" by colord.
Could this cause confusion in the consumers of the theme? My experience as a developer has taught me that usually 100 is always associated to a brighter color, and 900 to a darker.
There was a problem hiding this comment.
I don't have an alternative for that problem specifically. Though, I'm expecting that most usages won't be using these gray colors directly, but instead through more semantically named variables (e.g. border, lightBorder, secondaryText, etc).
There was a problem hiding this comment.
Ok. Let's get a feeling for it and see if we need to make any adjustments (at most we could add a line to our docs ?)
| 400: 0.2, | ||
| 600: 0.42, | ||
| 700: 0.543, | ||
| 800: 0.821, |
There was a problem hiding this comment.
Out of curiosity, any reason why we don't generate a gray-900 color?
There was a problem hiding this comment.
In the context of our color system, gray-900 is always the same as foreground. So it was redundant.
There was a problem hiding this comment.
Are these opacity values? Just curious on how the hex colors are calculated. This is cool stuff.
There was a problem hiding this comment.
These are "darkness" values, i.e. the inverse of the Lightness value in HSL.
gray-100 #f0f0f0 is hsl(0, 0%, 94%) (1 - 0.94 = 0.06, etc)
gray-200 #e0e0e0 is hsl(0, 0%, 88%) and so on...
That said, we might eventually switch from HSL to LCH for internal calculations so the lightness differences are a bit more reliable than HSL.
There was a problem hiding this comment.
Very cool, thank you! I like the sound of that, in the past I explored HSL for component theming, and the opportunities are pretty cool. That's not to say the approach I explored there is the right one — just that it's nice to be able to feed a single hue value, and have the rest refer to that.
Edit: Especially useful if it means we can retire light/dark versions of the spot color, so we have just a single spot color.
|
Thanks @ciampo for all the collab here! I've added the final tweaks and a changelog entry. |
* WIP * Tweak theme presets * Check default accent color * Fix math * Refactor * Fix props passing * Simplify background switcher * Allow undefined values in validation * Add unit tests * Add description * Test more shade generation * Check for background colors with no good foreground colors * Test using color variables * Add story for seeing generated colors * Fix typo * Memoize * Fixup * Add more contrast validation * Show issues in stories * Add tests * Update readme * Update TODO comments * Fixup * Fixup test * Add changelog * Disable toolbar addon for Theme stories
Part of #44116
What?
Extends the
Themecomponent to support an arbitrarybackgroundcolor. We then generate the rest of the necessary foreground/grayscale colors based on that.The Button component has been updated to support these new theme colors. (Except the
disabledandisBusystates, which we will address later.Why?
It is cumbersome and error-prone for a consumer to have to generate all of the necessary theme colors. If possible, we would prefer for automatic generation to work in the majority of cases.
How?
See
theme/color-algorithms.tsfor the logic.Testing Instructions
npm run storybook:devand go to the stories for Button.Use the Theme add-on in the toolbar to switch between some preset themes.
Note that some styles are not theme-ready yet (
disabledandisBusystates) and will be addressed separately.Screenshots or screencast
Added a story for the
Themecomponent to see the generated colors.CleanShot.2022-11-10.at.06.37.21.mp4
Added a background underlay to the theme add-on in the toolbar.
CleanShot.2022-11-10.at.06.39.21.mp4