[Emotion] Add Emotion theming support

[cee-chen]

Summary

See https://emotion.sh/docs/theming

This work will support:

• Consumers who are used to styled. CSS-in-JS syntax, particularly the ability to use (props) => props.theme.var (which Kibana has multiple uses of across their codebase)
• Consumers who want to use the css={} prop without needing to import or call useEuiTheme() separately, e.g. css={theme => ({ color: theme.euiTheme.colors.primary })

Resolved questions

Questions resolved in comment below: #6913 (comment)

Questions to answer:

• It's not clear to me why Greg/Caroline never mentioned Emotion theming or made use of Emotion's ThemeProvider before this. Was it intentional to avoid conflicts with other Emotion themes? Did they simply just not realize it was an option?
  • I went digging through our issues/discussions as well as the old weekly sync docs for CSS-in-JS, and found literally no mention of Emotion's ThemeProvider by Greg or Caroline :(
  • The only relevant discussion I found was Will EUI theme be compatible with the theme specification used in `theme-ui` and `styled system`? #5351, opened by a non-EUI dev
• Should we consider updating all current instances of EUI to make use of this new API?
  • The slightly nicer syntactical sugar is probably the primary benefit? 🤷
• The one major downside I see to this is that consumers who use other Emotion themes other than EUI's may prefer to use their own theme prop and not use ours 🤷
  • Should we add in some sort of configuration that allows turning off EuiEmotionThemeProvider to support consumers using their own emotion ThemeProvider?
  • CAVEAT: If we switch EUI's own CSS-in-JS Emotion to use the new Emotion theme context, and we also allow turning off the theme wrapper, we break our own styles
  • THOUGHT: We're passing the full { euiTheme, colorMode, modify } obj to Emotion's theme prop so in theory consumers could add their own custom theme vars to modify instead. Or alternatively, they can "override" Emotion's theme with <EuiThemeProvider><ThemeProvider theme={consumerTheme}>.... ooh, but then we run into shenanigans where EUI can potentially no longer trust Emotion's context to have the theme vars we need for EUI's components.

QA

• Go to https://eui.elastic.co/pr_6913/#/theming/theme-provider#consuming-with-emotions-theming to see the working CSS output
• Toggle between light and dark mode to see working conditional CSS (in first box, not second)

General checklist

• Checked in both light and dark modes
• [ ] Props have proper autodocs (using @default if default values are missing) and playground toggles
  • This is not configurable by prop, although we could expand EuiThemeProvider to be if necessary / if people request it in the future
• Added documentation
• Added or updated jest and cypress tests
• Checked for breaking changes and labeled appropriately
  • This could technically be considered a breaking change, but I'm going to mark it as just a feature for now, unless folks think otherwise
• A changelog entry exists and is marked appropriately

- [ ] Checked in mobile
- [ ] Checked in Chrome, Safari, Edge, and Firefox
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Checked for accessibility including keyboard-only and screenreader modes
- [ ] Updated the Figma library counterpart

[cee-chen]

Hey @elastic/eui-team and @tomsonpl 👋 As a heads up I'm opening this draft PR to get some thoughts on Emotion theme support before we decide on anything.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_6913/

[cee-chen]

I'm really 50/50 on baking this into EuiThemeProvider by default.

The other option would be to simply export <EuiEmotionThemeProvider> and allow consumers to opt into it, e.g.

<EuiThemeProvider>
  <EuiEmotionThemeProvider>
    // component that can use `css={theme => {}}` syntax
  </EuiEmotionThemeProvider>
</EuiThemeProvider>

The major downside to that is that consumers will need to remember to re-include EuiEmotionThemeProvider every time they nest EuiThemeProvider for component-level theming, i.e.

// Top level of app
<EuiProvider>
  <EuiEmotionThemeProvider>
    // the `theme` prop vars will be correct here

    <EuiThemeProvider colorMode="inverse">
      // the `theme` prop vars will NOT be correct here

      <EuiEmotionThemeProvider>
        // the `theme` prop vars WILL be correct here
      </EuiEmotionThemeProvider>
    </EuiThemeProvider>
  </EuiEmotionThemeProvider>
</EuiProvider>

The flip side is that baking in Emotion's theme context into EuiThemeProvider would be more inflexible for consumers who don't want our theme or want to use/set their own Emotion theme. Do we want to support those consumers though? see #5351

[1Copenut]

By baking in the entire thing, do we provide enough of an escape hatch for folks with the modify object? In my mind, EUI powers Kibana as its first order of business, so we should be tailoring our approach to that first and allow folks to mod if that's what they need to do.

To me it's like CRA. There's a clear set of defaults, but if folks absolutely need to mod things, they can eject and tailor the experience. The analogy isn't 1:1, but I look at it similarly in DX terms.

[cee-chen]

By baking in the entire thing, do we provide enough of an escape hatch for folks with the modify object?

IMO, I think it's enough of an escape hatch. The other option we can consider is adding an includeEmotionTheme prop, e.g.

<EuiThemeProvider includeEmotionTheme={false}>

I think with both of those options, we should have enough of an escape hatch for consumers who want to set their own Emotion theme/styling and not use EUI's.

edit: Ah, I just saw your comment down below where you don't think that prop is necessary. 🤔 I'd be good not including it for and waiting to see what feedback we get from consumers first!

[cee-chen]

I'm also pretty 50/50 on providing the entire theme context (i.e. colorMode and modify alongside with the euiTheme vars) or just euiTheme itself, i.e.

Suggested change

	const euiThemeContext = useEuiTheme();
	return <ThemeProvider theme={euiThemeContext}>{children}</ThemeProvider>;
	const { euiTheme } = useEuiTheme();
	return <ThemeProvider theme={euiTheme}>{children}</ThemeProvider>;

To be honest, I would assume most consumers won't need color mode info, but I think it might be syntactically confusing to return a theme prop that doesn't match the return structure of our own useEuiTheme() hook. 🤷

[1Copenut]

I'd plus one on having the return structure match our useEuiTheme() hook. It might be verbose, but I'm a big fan of consistency.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_6913/

[breehall]

Thank you for doing research into this! My first instinct is to say we should update EUI to match the new API. My gut says this because:

• There are already instances in Kibana that are used to styled. It feels like EUI could become a hinderance (rather than a helper) if we begin to force a specific syntax or workaround that is more complex than what's already in place.
• Wanting to use the css prop without having to import useEuiTheme() feels a good benefit.
• Decisions were made at the beginning of the project without the insight that we have now, and there's nothing wrong with going back and correcting.

However, one of the big caveats you mentioned:

If we switch EUI's own CSS-in-JS Emotion to use the new Emotion theme context, and we also allow turning off the theme wrapper, we break our own styles

This feels like a big downside and could make supporting it less fun and could open a backdoor we wouldn't want developers to use.

Do we know what level of effort this would cause to develop and how much tech debt this would cause down stream? If we decided not to go this route, we would likely need to provide education to make the process easier for Kibana devs.

If the syntactical sugar is the main benefit, I would consider the effort vs. the reward.

[1Copenut]

I left a couple of responses on Cee's original comments. These are extremely helpful for context and how you're looking at the challenge.

I agree with Bree that we should update EUI to match the new API. It'd be beneficial to allow precise modifications or larger overrides, but these feel like nice to haves. The example override in the docs shows a clear path to make modifications while meeting the primary objective adding styled.elem syntax to Emotion.

If folks want to modify, then they're free to do so, but I don't feel adding a shutoff for EuiEmotionThemeProvider is necessary. I think it's also worth discussing how far we're capable and willing to support modifications that Cee outlined could have unintended results.

[cee-chen]

Thanks y'all, this discussion was super helpful! Regarding switching EUI to the new css={theme => {}} syntax, although that was my first instinct as well, I'm now leaning very strongly away from that for a couple of reasons:

• Effort vs reward
  • It doesn't feel like it's worth the time to go back and convert all our existing Emotion conversions 😅
• Library-agnostic behavior
  • EUI should ideally remain agnostic of consumer behavior for our baseline components/styling (except in a singular place, which is allowing custom theme tokens overrides).
  • Us switching to Emotion's theme provider means that other consumers can (without realizing it) completely override our theme and break styling. e.g., if a consumer doesn't even realize we're providing this functionality and does something like <EuiProvider><ThemeProvider theme={theirCustomEmotionTheme}>, all of EUI's internal styling breaks if we've switched to Emotion's theme context, but does not break if we stick to our current useEuiTheme() pattern.
  • Again, the gain of the small syntactical sugar vs potential breakage just doesn't seem worth it, as opposed to sticking with the architecture we already have in place.

So quick TL;DR recap of my thoughts:

• We should bake in an Emotion theme provider into EuiThemeProvider by default, but for consumer usage/convenience only
• EUI itself will continue to use our EUI's internal theme context and not Emotion's, to reduce potential consumer overlap/pollution

Does that sound reasonable to folks? 🙏

[tomsonpl]

Hey, thanks for your inputs, I start to understand the issue clearly now :)
A bit of context, regarding the issue:
elastic/kibana#161179
I started looking into migrating osquery plugin from styled-components to emotion, and right away found issues with applying theme. Firstly there was no emotion's theme provider, secondly the types of emotion css do not accept theme in the callback.

I do not want to put any cumbersome work on your plate, I know you're busy :) But from kibana dev perspective, I would love to have access to theme globally through css={(theme) => {} }. I think it is unwieldy to use useEuiTheme hook in every component.

That's my 5 cents ;P Please let me know if I could be of any help, otherwise I'll just leave my PR pending until your discussion is resolved :)
Once again, I am very very thankful for your help here :)

[cee-chen]

@elastic/eui-team this now has tests, docs, and is ready for review :) The actual JS code is pretty laughably simple, so most of what's left to review at this point is copy and docs. I'd also be curious to hear if y'all would consider this a breaking change or not a breaking change, I have a very slight lean towards not, but would be good either way!

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_6913/

[1Copenut]

One small copy item and a comment/question about CodeSandbox. Will change my review to approved as soon as you're ready. Excellent work @Cee!

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_6913/

[1Copenut]

🚀 Love this update @Cee! Everything was easy to grok and the documentation makes it clear where the seams are between EUI's default theme and users' escape hatch for overrides.

[cee-chen]

Adding one quick note for reference - Trevor and I rubber-duckied in Slack where I debated moving the new EuiEmotionThemeProvider to our new src/services/emotion/ folder instead of living in src/services/theme/. We eventually decided against it as the new component is currently only used by EuiThemeProvider and isn't really meant to be used/exported publicly outside that.

If that ever changes in the future, we can consider moving EuiEmotionThemeProvider and its files to src/services/emotion/ and making it a public export later.

[cee-chen]

@tomsonpl - as a heads up for your Kibana PR, this work will be released next Tuesday, and then will get into Kibana sometime after that.

Once Kibana is has the latest EUI release with this PR, you should be able to update your PR to remove your manual emotion <ThemeProvider> call. You will need to tweak your theme usage a little bit w/ extra destructuring, this theme provider passes the full useEuiTheme() context and not just the theme variables, so you'll need to do, e.g. css={({ euiTheme }) => ({ color: euiTheme.color.primary })} instead of just css={(theme) => ({ color: theme.color.primary })}.

For typing, you'll also need to grab emotion.d.ts from the EUI library directly (or just copy ours and pass it to your tsconfig). I'd be happy to help out with updates to your PR, just LMK!

[tomsonpl]

This is just amazing, appreciate the work and the fact that you just took care of it right away :)
I'll adjust whenever it's available in Kibana. Big thanks!