[EuiProvider] Add `globalStyles` prop for customization by thompsongl · Pull Request #5497 · elastic/eui

thompsongl · 2021-12-21T15:49:09Z

Summary

Exports EuiGlobalStyles to allow for custom composition
Adds globalStyles prop to EuiProvider to enable custom or no global styles
- For future cases where a consumer is using a totally custom theme, they will have control over global styles
- For Kibana, where multiple EuiProvider instances are loaded and we want to eliminate duplicating global styles
Updates @emotion dependencies

Checklist

Check against all themes for compatibility in both light and dark modes
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs and playground toggles
Added documentation
Checked Code Sandbox works for any docs examples
Added or updated jest and cypress tests
Checked for breaking changes and labeled appropriately
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately

kibanamachine · 2021-12-21T16:23:03Z

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

kibanamachine · 2021-12-22T18:23:25Z

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

kibanamachine · 2022-01-05T19:46:00Z

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

cchaos · 2022-01-06T16:02:53Z

src/components/provider/provider.tsx

 export const EuiProvider = <T extends {} = {}>({
  cache,
  theme = EuiThemeAmsterdam,
+  globalStyles: GlobalStyles = EuiGlobalStyles,


Should the default be undefined instead? My guess is that most consumer's of multiple providers won't realize they need to remove the global styles. But likely, they'll notice they need to add it in order to fix the base styles.

Or should we be recommending two different providers. For instance this is just the theme provider, so should Kibana's multiple instances just switch to using the theme provider?

The summary might be overly concerned with multiple providers. The reasons for having default global styles but allowing for their removal are that:

Custom themes need to remove default global styles

With the exception of Kibana, apps will only have one EuiProvider instance

In the future, EuiProvider will likely also include i18n and other features

The Kibana provider that wraps EuiProvider will need access to all that data

That is, EuiThemeProvider isn't enough, but we need a way to prevent duplicate global styles (again, this very likely only affect Kibana)

very likely only affect Kibana

This is specifically where I worry that Kibana engineers will not know that they need to remove the global styles, making them compound or override. What is your plan to educate those engineers about this workaround?

All instances of EuiProvider (4 right now; unlikely to increase) are in mount-point components maintained by the core team. They all use the same pattern and I can leave comments in the 3 that do not need global styles explaining the difference.

kibanamachine · 2022-01-12T16:16:56Z

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

cchaos

Ok, I tried to re-word that example text to better explain, based on your explanation.

cchaos · 2022-01-12T18:37:15Z

src-docs/src/views/provider/provider.md

+## Global styles

-A reset stylesheet and the global EUI styles are applied via Emotion. To prevent loading these styles from loading, pass `theme={null}` to the provider.
+A reset stylesheet and the global EUI styles are applied via Emotion. To prevent loading these styles from loading, pass `globalStyles={false}` to the provider. If you are using the legacy theme, we recommend passing `theme={null}` to achieve similar results.


Suggested change

A reset stylesheet and the global EUI styles are applied via Emotion. To prevent loading these styles from loading, pass `globalStyles={false}` to the provider. If you are using the legacy theme, we recommend passing `theme={null}` to achieve similar results.

The provider includes general reset and global styles, applied via Emotion. These only need to be applied **once** so to prevent these styles from loading in nested instances of the provider, pass `globalStyles={false}`.

We also really need to move this doc to real docs example page so that we can change the provider setup example based on the currently selected theme. I don't think anyone will catch that sentence you added.

Converted the docs and made the code blocks update based on the docs theme selections.

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

kibanamachine · 2022-01-13T00:39:33Z

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

kibanamachine · 2022-01-13T02:04:25Z

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

kibanamachine · 2022-01-13T18:14:19Z

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

kibanamachine · 2022-01-18T19:35:42Z

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

src-docs/src/views/provider/provider_example.js

kibanamachine · 2022-01-19T17:13:15Z

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

chandlerprall

Changes LGTM! Pulled & tested locally

thompsongl added 5 commits December 21, 2021 09:30

bump emotion deps

0b315d9

export EuiGlobalStyles

9c623be

allow custom or no global styles

a67c881

Merge remote-tracking branch 'upstream/main'

f8101c4

clean up

bc81ba8

thompsongl changed the title ~~Provider globalstyles~~ [EuiProvider] Add globalStyles prop for customization Dec 21, 2021

CL

a82f279

thompsongl added 2 commits December 22, 2021 11:48

Merge branch 'main' into provider-globalstyles

87bb5bf

docs

11051bb

thompsongl marked this pull request as ready for review December 22, 2021 17:52

Merge branch 'main' into provider-globalstyles

bb201f5

cchaos reviewed Jan 6, 2022

View reviewed changes

Merge branch 'main' into provider-globalstyles

689182c

cchaos reviewed Jan 12, 2022

View reviewed changes

thompsongl and others added 2 commits January 12, 2022 18:04

Update src-docs/src/views/provider/provider.md

3cd86d0

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

Merge branch 'main' into provider-globalstyles

8b4d7f8

convert docs; make context-aware

275476c

thompsongl requested a review from cchaos January 13, 2022 16:37

Merge branch 'main' into provider-globalstyles

fe99869

thompsongl added this to the Elastic Stack 8.1 milestone Jan 13, 2022

Merge branch 'main' into provider-globalstyles

3c537b7

thompsongl requested a review from chandlerprall January 18, 2022 19:03

chandlerprall reviewed Jan 18, 2022

View reviewed changes

src-docs/src/views/provider/provider_example.js Show resolved Hide resolved

Merge branch 'main' into provider-globalstyles

c389bb7

thompsongl requested a review from chandlerprall January 19, 2022 16:34

chandlerprall approved these changes Jan 19, 2022

View reviewed changes

thompsongl merged commit 55194c2 into elastic:main Jan 19, 2022

thompsongl mentioned this pull request Jan 24, 2022

Upgrade EUI to v46.1.0 elastic/kibana#123652

Merged

	A reset stylesheet and the global EUI styles are applied via Emotion. To prevent loading these styles from loading, pass `globalStyles={false}` to the provider. If you are using the legacy theme, we recommend passing `theme={null}` to achieve similar results.
	The provider includes general reset and global styles, applied via Emotion. These only need to be applied once so to prevent these styles from loading in nested instances of the provider, pass `globalStyles={false}`.

Conversation

thompsongl commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Dec 21, 2021

Uh oh!

kibanamachine commented Dec 22, 2021

Uh oh!

kibanamachine commented Jan 5, 2022

Uh oh!

cchaos Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

thompsongl Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cchaos Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

thompsongl Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 12, 2022

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

cchaos Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

thompsongl Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 13, 2022

Uh oh!

kibanamachine commented Jan 13, 2022

Uh oh!

kibanamachine commented Jan 13, 2022

Uh oh!

kibanamachine commented Jan 18, 2022

Uh oh!

Uh oh!

kibanamachine commented Jan 19, 2022

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thompsongl commented Dec 21, 2021 •

edited

Loading

thompsongl Jan 12, 2022 •

edited

Loading