Skip to content

[EuiProvider] Add globalStyles prop for customization #5497

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.

Sign up for GitHub

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

Merged
thompsongl merged 16 commits into from
Jan 19, 2022
Merged

[EuiProvider] Add globalStyles prop for customization #5497

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0b315d9
bump emotion deps
thompsongl Dec 21, 2021
9c623be
export EuiGlobalStyles
thompsongl Dec 21, 2021
a67c881
allow custom or no global styles
thompsongl Dec 21, 2021
f8101c4
Merge remote-tracking branch 'upstream/main'
thompsongl Dec 21, 2021
bc81ba8
clean up
thompsongl Dec 21, 2021
a82f279
CL
thompsongl Dec 21, 2021
87bb5bf
Merge branch 'main' into provider-globalstyles
thompsongl Dec 22, 2021
11051bb
docs
thompsongl Dec 22, 2021
bb201f5
Merge branch 'main' into provider-globalstyles
thompsongl Jan 5, 2022
689182c
Merge branch 'main' into provider-globalstyles
thompsongl Jan 12, 2022
3cd86d0
Update src-docs/src/views/provider/provider.md
thompsongl Jan 13, 2022
8b4d7f8
Merge branch 'main' into provider-globalstyles
thompsongl Jan 13, 2022
275476c
convert docs; make context-aware
thompsongl Jan 13, 2022
fe99869
Merge branch 'main' into provider-globalstyles
thompsongl Jan 13, 2022
3c537b7
Merge branch 'main' into provider-globalstyles
thompsongl Jan 18, 2022
c389bb7
Merge branch 'main' into provider-globalstyles
thompsongl Jan 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## [`main`](https://github.com/elastic/eui/tree/main)

No public interface changes since `44.0.0`.
- Added `globalStyles` prop to `EuiProvider` to allow for global style customization ([#5497](https://github.com/elastic/eui/pull/5497))
- Exported `EuiGlobalStyles` component ([#5497](https://github.com/elastic/eui/pull/5497))

## [`44.0.0`](https://github.com/elastic/eui/tree/v44.0.0)

Expand Down
10 changes: 5 additions & 5 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@
"@elastic/charts": "^41.0.1",
"@elastic/datemath": "^5.0.3",
"@elastic/eslint-config-kibana": "^0.15.0",
"@emotion/babel-preset-css-prop": "^11.0.0",
"@emotion/cache": "^11.4.0",
"@emotion/eslint-plugin": "^11.0.0",
"@emotion/jest": "^11.1.0",
"@emotion/react": "^11.1.1",
"@emotion/babel-preset-css-prop": "^11.2.0",
"@emotion/cache": "^11.7.1",
"@emotion/eslint-plugin": "^11.7.0",
"@emotion/jest": "^11.7.1",
"@emotion/react": "^11.7.1",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.3",
"@svgr/core": "5.5.0",
"@svgr/plugin-svgo": "^4.0.3",
Expand Down
4 changes: 2 additions & 2 deletions src-docs/src/views/provider/provider.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ See [**EuiThemeProvider**](/#/theming/theme-provider) for full documentation as

It is not recommended to recreate the functionality of **EuiProvider** by composing its constituent parts. More context, functionality, and configurations will be added to **EuiProvider** in future releases. Nested instances of [**EuiThemeProvider**](/#/theming/theme-provider), however, are valid.

## Global reset
## 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.

@cchaos cchaos Jan 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}`.

@cchaos cchaos Jan 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@thompsongl thompsongl Jan 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


### `@emotion/cache` and style injection location

Expand Down
1 change: 1 addition & 0 deletions src/components/provider/__snapshots__/provider.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,7 @@ exports[`EuiProvider providing an @emotion cache config applies the cache to glo
"before": null,
"container": <head />,
"ctr": 0,
"insertionPoint": undefined,
"isSpeedy": false,
"key": "testing",
"nonce": undefined,
Expand Down
20 changes: 13 additions & 7 deletions src/components/provider/provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@
import React, { PropsWithChildren } from 'react';
import { CacheProvider, EmotionCache } from '@emotion/react';

import {
EuiGlobalStyles,
EuiGlobalStylesProps,
} from '../../global_styling/reset/global_styles';
import { EuiGlobalStyles, EuiGlobalStylesProps } from '../../global_styling';
import {
EuiThemeProvider,
EuiThemeProviderProps,
Expand All @@ -28,24 +25,33 @@ export interface EuiProviderProps<T>
* Pass `null` to remove all theming including global reset
*/
theme?: EuiThemeSystem | null;
/**
* Provide global styles via `@emotion/react` `Global` for your custom theme.
* Pass `false` to remove the default EUI global styles.
*/
globalStyles?: false | ((params: any) => JSX.Element | null);
/**
* Provide a cache configuration from `@emotion/cache`
*/
cache?: EmotionCache;
}

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

@cchaos cchaos Jan 6, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@thompsongl thompsongl Jan 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@cchaos cchaos Jan 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@thompsongl thompsongl Jan 12, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

colorMode,
modify,
children,
}: PropsWithChildren<EuiProviderProps<T>>) => {
return theme !== null ? (
return theme !== null && GlobalStyles !== false ? (
<EuiThemeProvider theme={theme} colorMode={colorMode} modify={modify}>
{cache ? (
<CacheProvider value={cache}>
<EuiGlobalStyles />
<GlobalStyles />
</CacheProvider>
) : (
<EuiGlobalStyles />
<GlobalStyles />
)}
{children}
</EuiThemeProvider>
Expand Down
9 changes: 9 additions & 0 deletions src/global_styling/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export * from './reset/global_styles';
1 change: 1 addition & 0 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ export * from './components';
export * from './services';
export * from './utils';
export * from './themes';
export * from './global_styling';
Loading