Skip to content
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

Closed
itsdouges opened this issue Dec 19, 2019 · 12 comments
Closed

RFC: Theming #18

itsdouges opened this issue Dec 19, 2019 · 12 comments
Labels
rfc 💬 Request for comments

Comments

@itsdouges
Copy link
Collaborator

itsdouges commented Dec 19, 2019

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:

{
  base: {},
  default: {},
  [optionalotherthemes]: {} // every other theme needs to have the same keys from default
}

Then use them in your CSS sourced from default:

css`
  color: primary; // or
  color: theme(primary);
`

Transforms to:

color: #fff;
color: var(--var-123abc);

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):

import { createThemeProvider } from '@compiled/css-in-js';

export const ThemeProvider = createThemeProvider();

// Optional "runtime" themes.
export const ThemeProvider = createThemeProvider({ ... });

Transforms to:

import { _TP } from '@compiled/css-in-js';

const theme = { base: { ... }, default: { '--var-abc123': '#000', ... }, ... };
// optional runtime themes would be merged in and converted to css variables

export const ThemeProvider = (props) => (
  <_TP theme={props.theme}>
   {props.children(theme[props.mode])
  <_TP>
);

And then your consumers use like:

import { ThemeProvider } from 'your-design-system';

const App = () => (
  <ThemeProvider theme="dark">
     {style => <div style={style}>...</div>
  </ThemeProvider>
);

Component theming

Would be the same as above with one key difference - the theme isn't sourced from the config. It's sourced inline.

import { createVariants } from '@compiled/css-in-js';

const useVariants = createVariants<'primary' | 'danger'>({
  default: { default: {}, [optionalotherthemes]: {} },
  [optionalotherthemes]: {},
});

<button style={useVariants('primary')} css={{ border: 'border', color: 'color' }}>
  blah
</button>
import { useMode } from '@compiled/style;

// transformed to css variables
const variants = { default: {}, ... };

const useVariants = (variant: string) => {
  const mode = useMode();
  const defaultVariant = variants.default;

  return { 
    ...defaultVariant.default,
    ...defaultVariant[mode],
    ...variants[variant][mode],
  };
};

The type safety aspect is missing a little. Perhaps instead of using string literals theme(primary) and variant(color) we could import them?

import { themeRefs, createVariants } from '@compiled/css-in-js'; 

const { useVariants, variant } = createVariants({});
const theme = themeRefs();

<div
  style={useVariants('primary')}
  css={{
    backgroundColor: variant('backgroundColor'),
    color: theme('primary'),
  }}
/>

???

Goals

  • Minimal use of react context
  • Css variables for passing style values around
  • Don't force specific markup to consumers
  • Ensure bleeding of css variables doesn't affect things it shouldn't

Lingering thoughts

  • What about runtime themes (i.e. they aren't known until runtime)?
@itsdouges itsdouges mentioned this issue Dec 19, 2019
10 tasks
@itsdouges itsdouges added this to the Phase 2 milestone Dec 19, 2019
@itsdouges itsdouges added the new feature 🆕 New feature or request label Jan 1, 2020
@JClackett
Copy link

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!

@itsdouges

This comment has been minimized.

@itsdouges itsdouges changed the title Create theming api RFC: Theming api Apr 6, 2020
@itsdouges itsdouges added rfc 💬 Request for comments and removed new feature 🆕 New feature or request labels Apr 6, 2020
@itsdouges itsdouges modified the milestones: Table stakes v0.x.x, v1.0 Apr 11, 2020
@itsdouges itsdouges changed the title RFC: Theming api RFC: Theming Apr 22, 2020
@flexdinesh
Copy link
Contributor

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.

@itsdouges
Copy link
Collaborator Author

This RFC covers both - you statically declare your modes + tokens and then at runtime you can switch between them using the createThemeProvider API.

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.

This was referenced Apr 30, 2020
@itsdouges itsdouges removed this from the v1.0 milestone Aug 2, 2020
@itsdouges itsdouges pinned this issue Aug 14, 2020
@bradleyayers
Copy link

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.

@itsdouges
Copy link
Collaborator Author

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.

@mikestopcontinues
Copy link

Regarding global theming, I think it's important to at least have the option to inject a global <style>:root {--vars}</style> element. Both when using runtime styles and when using multiple themes, you want to be able to use your theme vars to style your html, body, and potentially other global elements. It's also more performant to do so than sticking the vars on a dom node. (Especially when updating the vars—changing theme, live-edits, user input, etc.)

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 themeRefs().color.primary[0] rather than themeRefs()('color.primary[0]')? Better for auto-complete, and it might open up some cool possibilities for parsing constant values at compile-time. (e.g. `themeRefs().color[COLOR as ColorConst][0]')

Variants, also cool. There's a small typo in your code sample. The top-level object should say something like [optionalOtherVariant]. It also makes me think, would it be fair to drop {base} from the theme object, and merge into the default mode? Better symmetry between APIs.

Anyway, great project! I really enjoy using it.

@itsdouges
Copy link
Collaborator Author

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.

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.

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.

@okonet
Copy link

okonet commented Apr 25, 2021

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.

@mikestopcontinues
Copy link

@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

@itsdouges
Copy link
Collaborator Author

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.

@dddlr
Copy link
Collaborator

dddlr commented May 13, 2024

no longer on roadmap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc 💬 Request for comments
Projects
None yet
Development

No branches or pull requests

7 participants