Add rudimentary theme support for Emotion components #36042
Conversation
sarayourfriend
added
[Package] Components
|
Size Change: -2 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
|
I think my main issue with this is that "theming" shouldn't be a "@wordpress/components" thing as we already have a more generic theming in WP (user profiles) that can apply cross packages. |
|
@youknowriad that's a good point... I wonder if there's a way to merge those approaches? One way or another The context for this is that someone is trying to use the SearchInput component in the big W menu, where it needs to be contextually in a "dark mode". Without hand overridding every single style (and having to maintain that every time a change to SearchInput is made) Emotion's theme support is the answer for this. It's even better than the situation with regular Sass where you'd have to maintain specific overrides for every single color based style on the SearchInput (which should be able to freely introduce or change where and how it applies colors without having to worry about how consumers are styling it). |
|
@sarayourfriend It makes sense to me to merge these two (a theme package or something like that that generates both CSS variables and potentially a react context if the variables are not enough for the components) That said, I'm still not sure whether "Dark Mode" is a "theme" or an option of any theme. In other words whether components should support both dark and light mode for all themes. |
|
If there are user profile themes, then "dark mode" doesn't really correspond to anything specific. But there are contexts in the Gutenberg UI where a "dark mode" needs to be applied to components (like the big W navigation menu). That's where cascading ThemeProviders would come into effect. We'd essentially have a root theme provider that injects the user profile theme into the context that would be used in all use cases where there wasn't a contextually specific theme applied. const baseTheme = createTheme( userProfileTheme );
const navigationTheme = createTheme( { colors: { black: 'white', white: 'black' } }, { isStatic: false } )
const App = () => (
<>
<ThemeProvider theme={ baseTheme }>
<Header>
<BigWMenu>
<Button pressed={ true } onClick={ setMenuOpen }>W</Button>
{ menuOpen && (
<ThemeProvider theme={ navigationTheme }>
<Navigation />
</ThemeProvider>
) }
</BigWMenu>
</Header>
<RestOfTheApp />
</ThemeProvider>
</>
);That would have to live somewhere like the |
This seems like a good idea. As I recall we were ready to dive head first into use purely CSS variables but then realized that every CSS variable is technically a public API, which gave us pause. That's why we ended up opting for the singleton JS objects of The benefits of CSS variables over React context (which is all the Emotion theme stuff really is, for those following along who aren't in the loop) is that we get native browser performance and zero additional processing time for styles (aside from what the browser will do to resolve variables, but we can trust that will be fast). Centralizing all theme variables, whether CSS or in-JS into a single package seems like a great idea. I know we were hesitant to introduce new packages in the past though, so not sure where we stand on that right now. |
|
I don't feel fully equipped to assess this yet, so definitely waiting for @ciampo's input!
I'm leaning toward all underlying values being CSS variables, too. Our underlying color system is not ready for a "public API" set of themable variables in a light/dark mode context though, with a bunch of shades of gray being used in an unsemantic way ( |
|
Thank you @sarayourfriend for experimenting with this approach! I would also personally prefer using CSS variables for theming:
As you also mentioned, though, we would need to come with a good strategy for which and how many variables to expose, how to name them, etc... Finally, it's still not 100% clear to me how we should tie
|
|
Closing this PR for lack of activity and because it seems like (I'm hoping) we'll take the CSS variable approach instead of using Emotion's theme context (which would be restrictive and silo theme styles into a specific, non-general, non-standard system). CSS Variables can also be used to easily theme "areas" of the app. It might be nice to create some kind of component that makes this easier through props and that can be type-checked, instead of relying on ad-hoc |
This is indeed the plan, and you already summed up the whys behind the decision very well (although we haven't started active work on it yet, mostly for lack of time/resources as we focused on Global Styles and the WordPress 5.9 release).
This is something that I didn't personally think of before, thank you for sharing it! We'll definitely consider this option too when working on this feature. Finally, I just wanted to thank you for taking the initiative of exploring using Emotion's theme context for styling components — even if it's not the path that we'll end up taking, I believe it was still a very valuable exploration! |
Description
Adds rudimantary theme support for Emotion components.
I've added two basic utilities for accessing the theme in Emotion styles:
useTheme: this is a simple wrapper around Emotion's ownuseThemefunction but will return the default theme if no theme has been provided. Emotion'sThemeContextis configured to return an empty object if no theme has been provided, so we can just check for the existence of the WordPressTheme specific properties (configandcolors) to see if we have a custom theme or an Emotion theme. This is safe becausecreateThemeis type safe and the@emotion/reactThemetype has been extended using TypeScript's interface merging to require theconfigandcolorsvalues, so you cannot pass anything that doesn't match theWordPressThemetype toThemeProvider'sthemeprop. For example<ThemeProvider theme={{ foo: 'bar' }}>would raise a type error. Unfortunately there is one flaw in this approach in thatThemeProvider's props type appliesPartialto theThemetype meaning you could pass an empty object to the ThemeProvider. Luckily that would be safe as long asuseThemeandsafeThemeare used consistently.safeTheme: This is a guard function to run the conditional described above used byuseTheme. The context for this function is inside ofstyledcomponents interpolated functions. For examplestyled.span`color: ${props => safeTheme(props.theme).colors.black};`safeThemeensures that you always have aWordPressThemeobject, either the one provided to aThemeProviderparent component or the default theme.There are some trade offs to this approach, in particular with
safeTheme. For one, we're running property checks every render on the object. For somestyledcomponents I suppose this could add a certain amount of runtime overhead. It would also be easy to accidentally forget to usesafeThemeand accessprops.themedirectly. I toyed with some ideas of how to prevent this but all of them ultimately ended in having to roll-our-ownstyledobject so they're not really viable solutions. There's no way to monkey patch or hook into howstyledprovides thethemeprop that I could tell, nor would I consider such a solution exactly "maintainable".safeThemeis probably the most future proof approach to this becuase it is, after all, just a function. Whatever method Emotion has in the future for providing the theme to a styled component for function interpolation, you'll always be able to pass it throughsafeTheme.useThemeseems perfectly safe to me, the only risk being if someone accidentally used the originaluseThemeexported from@emotion/react. Unfortunately I don't think we can add an ESLint rule to restrict the import because we can't restrict specific imports, just broad imports from entire modules.Hopefully we can catch those two things (accessing
props.themedirectly instead of throughsafeThemeand importing the wronguseTheme) in code reviews 🙂Oh, I also added some stories to demonstrate the two ways of using the Emotion theme.
How has this been tested?
Run
npm run storybook:devand visit http://localhost:50240/?path=/story/components-experimental-theme--default.Inspect the source code for the story and ensure that it makes sense and covers all the use cases. I'll also be adding unit tests shortly.
Types of changes
New feature
Checklist:
*.native.jsfiles for terms that need renaming or removal).