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

Make ThemeContext into a proper component #3848

Closed
nyurik opened this issue Aug 1, 2020 · 1 comment
Closed

Make ThemeContext into a proper component #3848

nyurik opened this issue Aug 1, 2020 · 1 comment

Comments

@nyurik
Copy link
Contributor

nyurik commented Aug 1, 2020

Please add ThemeProvider to the published code, rather than limiting it to src-doc. Switching themes seems to be a very common scenario, and I suspect its implementation is fairly consistent -- a context consumer that would allow light/dark toggling, and a provider that should use the provided EUI styles with optionally adding a few app-specific ones.

Note that there is a bug -- the NPM package's @elastic/eui/eui.d.ts already includes ThemeProvider and many other src-doc files, thus IDEs suggest to add import { ThemeProvider } from '@elastic/eui'; which later does not work. See #3849

@snide
Copy link
Contributor

snide commented Aug 1, 2020

This was covered in some internal discussion, but posting here for visibility. Currently theme management is the responsibility of the application, not EUI. This will likely change as we move towards CSS-in-JS solutions, and we'd rather keep the discussion on those issues. Here is a good summary of some recent work around the subject. #3760 (comment)

Gonna close this one, but leave the linked issue (which is a bug) up.

@snide snide closed this as completed Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants