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

[Emotion] Global styles aren't overridden by color mode changes #6353

Closed
chandlerprall opened this issue Nov 9, 2022 · 1 comment · Fixed by #6775
Closed

[Emotion] Global styles aren't overridden by color mode changes #6353

chandlerprall opened this issue Nov 9, 2022 · 1 comment · Fixed by #6775
Assignees

Comments

@chandlerprall
Copy link
Contributor

https://codesandbox.io/s/sweet-einstein-05j8k0

Given the setup,

<EuiProvider colorMode="light">
  <EuiThemeProvider colorMode="inverse">
    <EuiSelectableTemplateSitewide />
  </EuiThemeProvider>
</EuiProvider>

we get

Screen Shot 2022-11-09 at 8 20 21 AM

where the results' title text inherits its color from the global styles instead of the inverted color mode.

Proposed solution

portals, modals, any custom DOM
when document.createElement is used, the component creating it must get a class name with the current color mode values and apply it to the created element

theme provider
when setting colorMode and there is an existing theme context (not top level), it needs to insert a div and use that to set the color values for its children

@miukimiu
Copy link
Contributor

I never liked the idea of having to pass color default for components that have color inherit by default. As we can see in the following example: https://elastic.github.io/eui/#/theming/color-mode. It's not intuitive. It seems a bug in my opinion. As a user, I would expect to wrap EuiText in a color mode and it would work right away.

So I like the proposed solution:

when setting colorMode and there is an existing theme context (not top level), it needs to insert a div and use that to set the color values for its children

Having a new div can cause some layout issues. So we need to make sure this div has styles that fits different scenarios or it is easily customizable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants