-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Theming #18
Comments
Dynamic themes would be quite nice, i.e. some data fetched in runtime controls the theme colors -useful for platforms / CMS's etc - would this be possible? Love the project! |
This comment has been minimized.
This comment has been minimized.
I think it would be nice to have a distinction between runtime theming and compile-time theming. Eg. Even if the user uses context provider, we could extract the theme tokens statically (which I think this RFC explains already). In case the user needs runtime theming (Eg. switch b/t dark and light modes), the extraction takes a different approach. |
This RFC covers both - you statically declare your modes + tokens and then at runtime you can switch between them using the What's not covered though is what if you want tokens that aren't known until runtime or want to be defined by our consumers which can definitely happen. Needs more thinking. |
An issue that would be good to consider is that when using themes based on React context, it propagate through portal boundaries, but with CSS variables it won't. So that'll be a bit of a gotcha when using portals. We're using portals extensively (e.g. for menus) at Dovetail and having theming propagate following React convention would be great. I'm not sure if in practice it would matter too much for me, would have to give it more thought. |
Hey @bradleyayers! Thanks for chiming in - definitely will need to consider this as we progress to the theming APIs 🙏. Atlassian utilises a lot of portalled components as well - so we'll want a good story here. |
Regarding global theming, I think it's important to at least have the option to inject a global When you separate out the global style vars use-case, I'm not sure much is left for the classic ThemeProvider. Though I do see it as a useful way to locally override the global theme. Like variants for a particular region of the page. I don't know why people would do this, but it's something only the library could support. All together, maybe something like: // declare the shape of the theme
const {GlobalStyleVars, LocalStyleVars} = createThemeVars({...});
// injects <style>:root {--vars}</style>
<GlobalStyleVars theme={mergeWithTheme} mode={'light' | 'dark'} />
// passes --vars down
<LocalStyleVars theme={runtimeObjectOnly}>
{(style) => <div style={style} />}
</LocalStyleVars> Some other thoughts: The type-safe var refs are really cool. Is it possible to do Variants, also cool. There's a small typo in your code sample. The top-level object should say something like Anyway, great project! I really enjoy using it. |
Hey! Thanks. Agreed when we start tackling theming we'll want to make sure theme variables are available in the head, which side-steps some gnarly issues with portaling.
Atlassian has a few use cases for this across a few products interestingly. Media components are always shown in "dark mode" for example. We should be able to ensure any known theme (CSS) variables are in the head. It's the "values not known until runtime" that is a bit more difficult. The easy answer is flushing it through inline styles, however the portaling issue comes up that needs either a good solution, or good example patterns to work with it. |
Could the problem be split into two? Compile-time and runtime? I think most of us here are interested in compile-time since runtime is well covered by existing CSS-in-JS solutions. |
@okonet I was actually thinking the same thing, except about theme modes, since that's not an everyone-thing either. But when I started thinking about the implementation, we're really only talking about a few lines. Same with runtime vars. Honestly I don't care how it works as long as I can do it. :D |
Supporting integrated theming is on the longer term roadmap however it may take a different form, I'm closing this issue to clean up the active issues but it can still be referenced in the future if needed. |
no longer on roadmap |
Fresh eyes needed. We should also take inspiration from https://theme-ui.com/
We can even create a TS language service to tell us what themes are available with intellisense!
Global theming
Define your tokens in your options:
Then use them in your CSS sourced from default:
Transforms to:
Hardcodes and use the variable so it works with/without a parent theme.
Prepare a theme provider for consumers (you should only ever create one of these):
Transforms to:
And then your consumers use like:
Component theming
Would be the same as above with one key difference - the theme isn't sourced from the config. It's sourced inline.
The type safety aspect is missing a little. Perhaps instead of using string literals
theme(primary)
andvariant(color)
we could import them????
Goals
Lingering thoughts
The text was updated successfully, but these errors were encountered: