Add wrapper and utilities around `EuiThemeProvider` #117368

pgayvallet · 2021-11-03T16:07:34Z

Summary

Stage 1 of #108972

This PR is the first stage to leverage the EuiTheme via EuiThemeProvider introduced by EUI.

Once the PR is merged, I will open a distinct issue for stage 2, where all application owners will have to adapt their application mounting and their usages of toMountPoint to leverage this new API.

changes:

core

Add a new theme service to core's client-side API
- Introduce a CoreTheme type, containing all the theme-related informations required by EUI (currently only the darkMode uiSetting value)
- The service exposes a theme$ observable emitting the theme
Adapt core's internal usages of react to wrap the nodes with a new <CoreThemeProvider> HOC. The HOC consumes core's CoreTheme and wrap the underlying react node with <EuiThemeProvider>.
Also expose the theme$ observable from AppMountParameter, to ease usage with plugin applications.

kibana_react

Add a new <KibanaThemeProvider> HOC, and the associated wrapWithTheme utility function
Adapt toMountPoint to accept an (optional, at least for now) theme$ parameter
- when provided, the react node will be wrapped with <KibanaThemeProvider>

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

…heme-provider

jportner

Code review only, Security changes to add themeServiceMock to tests LGTM

elasticmachine · 2021-11-09T19:20:48Z

Pinging @elastic/fleet (Team:Fleet)

sabarasaba

stack management changes LGTM

…heme-provider

thompsongl

Looks great, @pgayvallet! Confirmed that variables are accessible via useEuiTheme in chrome/ui and that color mode switching works.

What is the scope of part 2? Update every single plugin, I assume?

thompsongl · 2021-11-11T16:40:39Z

src/core/public/theme/core_theme_provider.tsx

+  const coreTheme = useObservable(theme$, defaultTheme);
+  const euiTheme = useMemo(() => convertCoreTheme(coreTheme), [coreTheme]);
+  return (
+    <EuiThemeProvider colorMode={euiTheme.colorMode} theme={euiTheme.euiThemeSystem}>


Question for @cchaos: This is using the (soon-to-be) legacy theme until elastic/eui#5121 merges. Should we configure to use Amsterdam in in the meantime?

mshustov · 2021-11-11T17:34:52Z

src/core/public/notifications/toasts/toasts_service.tsx

-          dismissToast={(toastId: string) => this.api!.remove(toastId)}
-          toasts$={this.api!.get$()}
-        />
+        <CoreThemeProvider theme$={theme.theme$}>


we use the same pattern in all the integration points:

<i18n.Context> <CoreThemeProvider theme$={theme.theme$}> ...

Maybe it makes sense to provide it as a single CoreIntegrationPoint component?

src/core/public/theme/types.ts

mshustov · 2021-11-11T17:44:36Z

src/core/public/theme/theme_service.ts

+
+  public setup({ injectedMetadata }: SetupDeps): ThemeServiceSetup {
+    const theme = injectedMetadata.getTheme();
+    this.theme$ = new BehaviorSubject<CoreTheme>({ darkMode: theme.darkMode });


When are we going to support theme updates?

When we'll eventually allow theme customization by the user. This would allow, for example, to preview the theme overrides dynamically. Having an observable for the theme is to anticipate this, and to avoid a breaking change in the API later when we'll have to support it.

But for now, I guess using of({ darkMode: theme.darkMode }) instead of using a behavior subject would be perfectly fine, if that was the question

But for now, I guess using of({ darkMode: theme.darkMode }) instead of using a behavior subject would be perfectly fine, if that was the question

yes, let's start with more clear API.

pgayvallet · 2021-11-12T08:51:19Z

@thompsongl

What is the scope of part 2? Update every single plugin, I assume?

That's correct, part 2 is asking each team to update their applications and usages of toMountPoint to use the theme wrapper.

jloleysens

Proposed changes for app services LGTM

jloleysens · 2021-11-15T09:52:27Z

src/plugins/kibana_react/public/theme/kibana_theme_provider.tsx

+export const KibanaThemeProvider: FC<KibanaThemeProviderProps> = ({ theme$, children }) => {
+  const theme = useObservable(theme$, defaultTheme);
+  const colorMode = useMemo(() => getColorMode(theme), [theme]);
+  return <EuiThemeProvider colorMode={colorMode}>{children}</EuiThemeProvider>;
+};


this makes sense to me, I can see how this can be added without requiring refactors for consumers.

…heme-provider

kibana-ci · 2021-11-17T08:40:54Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`core`	315	320	+5
`kibanaReact`	301	305	+4
total			+9

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`core`	1027	1031	+4
`kibanaReact`	205	213	+8
total			+12

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`core`	132.3KB	132.4KB	+2.0B
`devTools`	2.3KB	2.3KB	+29.0B
`savedObjectsManagement`	82.6KB	82.7KB	+54.0B
total			+85.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	269.2KB	270.8KB	+1.6KB
`devTools`	9.3KB	9.3KB	+11.0B
`kibanaReact`	57.9KB	58.3KB	+410.0B
total			+2.0KB

Unknown metric groups

API count

id	before	after	diff
`core`	2309	2318	+9
`kibanaReact`	229	237	+8
total			+17

History

💚 Build #6955 succeeded 3a23426
💚 Build #6217 succeeded 04d1d5f
💚 Build #5382 succeeded 6762b3d
💚 Build #5291 succeeded 22719be
💔 Build #5074 failed 0225446
💔 Build #5035 failed 9861131

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* draft implementation * refactor and do not use COLOR_MODES_STANDARD * plug CoreThemeProvider to rendering service * plug CoreThemeProvider to toast service * plug CoreThemeProvider to overlay service * adapt tests * adapt application tests * adapt core apps * fix tests and snapshots * fix types * fix setup mock * fix more types * add theme support to `toMountPoint` * add theme start to public API * fix integration test * self-review, first batch * self-review, second batch * add tests for core theme provider * add theme service tests * add more tests * add test for to_mount_point * update generated doc * fix type in story file * fix type in fleet story context * use proper theme$ for devtool's mount * update generated doc * review comments * introduce CoreContextProvider Co-authored-by: Kibana Machine <[email protected]>

## Summary Fix #89340 Implements a third option, `system`, for the `theme:darkMode` uiSettings, which will follow the system's theme preference (light/dark) when Kibana loads. https://github.com/elastic/kibana/assets/1532934/82078697-8bf5-41df-add1-4ecfed6e1dea **Note: system theme refresh still requires the user to reload Kibana - please see the next section for the reasons if you're interested** ## How theming works in Kibana, again? This is an excellent question, thanks for asking. And the answer is, "well, it's complicated". We have multiples sources of "themed" styles in Kibana, ordered from "best" to "worse": #### 1. the EUI/JSS Theming It was initially implemented in #117368. All react applications and react mountpoints are supposed to be wrapped by a `KibanaThemeProvider` that bridges core's `theme$` values to the `EuiProvider`. https://github.com/elastic/kibana/blob/477505a2dd86d8f103f3aef16b3b4b76dff0b27a/packages/core/theme/core-theme-browser-internal/src/core_theme_provider.tsx#L11 This one was already dynamic and just works perfectly. If `core.theme.theme$` changes, the new values is received by the theme provider, which automatically changes the styles accordingly, creating this sexy "it just works" effect: https://github.com/elastic/kibana/assets/1532934/f3e61ca7-f3ed-4c37-aa46-76ee68c1a628 If everything theme-related was using this approach, dynamic theme reload would have been possible. However, Kibana has a lot of legacy, so as you can imagine, it wasn't that easy. So, **don't get false hopes** (as I did when I tried it...) from this video. Dynamic theme swap **could not** be implemented in this PR. And the reasons are just below. #### 2. Per-theme css files https://github.com/elastic/kibana/blob/6443b571642c453a9232a04297fddfb0a918c0dc/packages/core/rendering/core-rendering-server-internal/src/render_utils.ts#L40-L54 We have a bunch of dark/light variations of some css files that are computed in the rendering service, server-side, to be injected into the page template. Of course, this doesn't play well with dynamic theming, given the UI then doesn't know which css should be swapped, and which one should be added instead. However, porting the responsibilities of which theme css files to load to the browser was achievable, and done in this PR. core's browser-side `theme` provider now receives the list of theme files for both light and dark theme (via the injected metadata), and inject them in the document (instead of having them pre-injected in the page template by the rendering service server-side). So this one wasn't a blocker for dynamic theme reload. #### 3. Plugin styles This is where the problems start. Plugins can ship their own styles too, by importing them from components or from their entrypoint. E.g this component https://github.com/elastic/kibana/blob/f1dc1e1869566c1d61a08bb67eb3d4ac2f47834e/src/plugins/controls/public/control_group/component/control_group_component.tsx#L9 importing this file: https://github.com/elastic/kibana/blob/bafb23580b18781e711575cd8c0e17a6e3f4f740/src/plugins/controls/public/control_group/control_group.scss#L107-L110 Which relies on a theme variable, `$euiFormBackgroundColor` So how does it works under the hood? How is that `$euiFormBackgroundColor` variable interpolated? Using which theme? Well, technically, how the styles are effectively loaded differs slightly between dev and production (different webpack loaders/config), but in both cases, it depends on the `__kbnThemeTag__` variable that is injected to the global scope by the `bootstrap.js` script. This `__kbnThemeTag__` tag (apparently) **can** be modified after page load. However it doesn't magically reload everything, so styles already loaded for one theme will not reload. If a component and its imported styles were already compiled / injected, then they won't reload As a short video is better than 3 blocks of text, just see: https://github.com/elastic/kibana/assets/1532934/3087ffd6-80d8-42bf-ab17-691ec408ea6f That was the first blocker for supporting "dynamic" reloads of the system theme. #### 4. Static inline styles Last but not least, we have some static style injected in the template, that also depend on the current theme. https://github.com/elastic/kibana/blob/6443b571642c453a9232a04297fddfb0a918c0dc/packages/core/rendering/core-rendering-server-internal/src/views/styles.tsx#L52-L54 Of course this plays very badly with dynamic theming. And worth noting, those styles are used by the "Loading Elastic" screen, where Core (and therefore Core's theming service) isn't loaded yet, which made the things even more complicated. This was the second blocker for supporting "dynamic" reloads of the system theme. #### 5. `euiThemeVars` Actually TIL (not that I was asking for it) We're exposing the EUI theme variable via the `euiThemeVars` of the `@kbn/ui-theme` package: E.g. https://github.com/elastic/kibana/blob/c7e785383a87f7e18509c601a5c089c755ac028e/src/plugins/chart_expressions/expression_metric/public/components/metric_vis.tsx#L41 https://github.com/elastic/kibana/blob/c7e785383a87f7e18509c601a5c089c755ac028e/src/plugins/chart_expressions/expression_metric/public/components/metric_vis.tsx#L50 So I did my best, and made it that this export was a proxy, and that Core's theme service was dynamically swapping the target of the proxy depending on the system's theme... https://github.com/elastic/kibana/blob/b0a0017811d7402f76709af7ab1b8e12334e64a5/packages/kbn-ui-theme/src/theme.ts#L30-L42 Unfortunately it doesn't fully work for dynamic system theme switch, given modules/code can keep references of the property directly (e.g. the snippet a few lines on top), and there's nothing to dynamically reload components when the proxy changes. So yet another blocker for dynamic theme switch. ## Release Notes Add a new option, `system`, to the `theme:darkMode` Kibana advanced setting, that can be used to have Kibana's theme follow the system's (light or dark) --------- Co-authored-by: kibanamachine <[email protected]>

) ## Summary Fix elastic#89340 Implements a third option, `system`, for the `theme:darkMode` uiSettings, which will follow the system's theme preference (light/dark) when Kibana loads. https://github.com/elastic/kibana/assets/1532934/82078697-8bf5-41df-add1-4ecfed6e1dea **Note: system theme refresh still requires the user to reload Kibana - please see the next section for the reasons if you're interested** ## How theming works in Kibana, again? This is an excellent question, thanks for asking. And the answer is, "well, it's complicated". We have multiples sources of "themed" styles in Kibana, ordered from "best" to "worse": #### 1. the EUI/JSS Theming It was initially implemented in elastic#117368. All react applications and react mountpoints are supposed to be wrapped by a `KibanaThemeProvider` that bridges core's `theme$` values to the `EuiProvider`. https://github.com/elastic/kibana/blob/477505a2dd86d8f103f3aef16b3b4b76dff0b27a/packages/core/theme/core-theme-browser-internal/src/core_theme_provider.tsx#L11 This one was already dynamic and just works perfectly. If `core.theme.theme$` changes, the new values is received by the theme provider, which automatically changes the styles accordingly, creating this sexy "it just works" effect: https://github.com/elastic/kibana/assets/1532934/f3e61ca7-f3ed-4c37-aa46-76ee68c1a628 If everything theme-related was using this approach, dynamic theme reload would have been possible. However, Kibana has a lot of legacy, so as you can imagine, it wasn't that easy. So, **don't get false hopes** (as I did when I tried it...) from this video. Dynamic theme swap **could not** be implemented in this PR. And the reasons are just below. #### 2. Per-theme css files https://github.com/elastic/kibana/blob/6443b571642c453a9232a04297fddfb0a918c0dc/packages/core/rendering/core-rendering-server-internal/src/render_utils.ts#L40-L54 We have a bunch of dark/light variations of some css files that are computed in the rendering service, server-side, to be injected into the page template. Of course, this doesn't play well with dynamic theming, given the UI then doesn't know which css should be swapped, and which one should be added instead. However, porting the responsibilities of which theme css files to load to the browser was achievable, and done in this PR. core's browser-side `theme` provider now receives the list of theme files for both light and dark theme (via the injected metadata), and inject them in the document (instead of having them pre-injected in the page template by the rendering service server-side). So this one wasn't a blocker for dynamic theme reload. #### 3. Plugin styles This is where the problems start. Plugins can ship their own styles too, by importing them from components or from their entrypoint. E.g this component https://github.com/elastic/kibana/blob/f1dc1e1869566c1d61a08bb67eb3d4ac2f47834e/src/plugins/controls/public/control_group/component/control_group_component.tsx#L9 importing this file: https://github.com/elastic/kibana/blob/bafb23580b18781e711575cd8c0e17a6e3f4f740/src/plugins/controls/public/control_group/control_group.scss#L107-L110 Which relies on a theme variable, `$euiFormBackgroundColor` So how does it works under the hood? How is that `$euiFormBackgroundColor` variable interpolated? Using which theme? Well, technically, how the styles are effectively loaded differs slightly between dev and production (different webpack loaders/config), but in both cases, it depends on the `__kbnThemeTag__` variable that is injected to the global scope by the `bootstrap.js` script. This `__kbnThemeTag__` tag (apparently) **can** be modified after page load. However it doesn't magically reload everything, so styles already loaded for one theme will not reload. If a component and its imported styles were already compiled / injected, then they won't reload As a short video is better than 3 blocks of text, just see: https://github.com/elastic/kibana/assets/1532934/3087ffd6-80d8-42bf-ab17-691ec408ea6f That was the first blocker for supporting "dynamic" reloads of the system theme. #### 4. Static inline styles Last but not least, we have some static style injected in the template, that also depend on the current theme. https://github.com/elastic/kibana/blob/6443b571642c453a9232a04297fddfb0a918c0dc/packages/core/rendering/core-rendering-server-internal/src/views/styles.tsx#L52-L54 Of course this plays very badly with dynamic theming. And worth noting, those styles are used by the "Loading Elastic" screen, where Core (and therefore Core's theming service) isn't loaded yet, which made the things even more complicated. This was the second blocker for supporting "dynamic" reloads of the system theme. #### 5. `euiThemeVars` Actually TIL (not that I was asking for it) We're exposing the EUI theme variable via the `euiThemeVars` of the `@kbn/ui-theme` package: E.g. https://github.com/elastic/kibana/blob/c7e785383a87f7e18509c601a5c089c755ac028e/src/plugins/chart_expressions/expression_metric/public/components/metric_vis.tsx#L41 https://github.com/elastic/kibana/blob/c7e785383a87f7e18509c601a5c089c755ac028e/src/plugins/chart_expressions/expression_metric/public/components/metric_vis.tsx#L50 So I did my best, and made it that this export was a proxy, and that Core's theme service was dynamically swapping the target of the proxy depending on the system's theme... https://github.com/elastic/kibana/blob/b0a0017811d7402f76709af7ab1b8e12334e64a5/packages/kbn-ui-theme/src/theme.ts#L30-L42 Unfortunately it doesn't fully work for dynamic system theme switch, given modules/code can keep references of the property directly (e.g. the snippet a few lines on top), and there's nothing to dynamically reload components when the proxy changes. So yet another blocker for dynamic theme switch. ## Release Notes Add a new option, `system`, to the `theme:darkMode` Kibana advanced setting, that can be used to have Kibana's theme follow the system's (light or dark) --------- Co-authored-by: kibanamachine <[email protected]>

pgayvallet added 8 commits November 3, 2021 17:04

draft implementation

b103d39

refactor and do not use COLOR_MODES_STANDARD

72eeac4

plug CoreThemeProvider to rendering service

4ea2628

plug CoreThemeProvider to toast service

a6351ee

plug CoreThemeProvider to overlay service

9a235b9

adapt tests

941ccb3

adapt application tests

c324a6c

adapt core apps

77c8068

pgayvallet added release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.1.0 EUI labels Nov 4, 2021

pgayvallet and others added 18 commits November 4, 2021 14:33

fix tests and snapshots

0333d1f

fix types

2613db9

fix setup mock

b20bb4a

fix more types

74ee533

add theme support to toMountPoint

17044c5

add theme start to public API

2b67354

fix integration test

26bf7c8

Merge remote-tracking branch 'upstream/main' into kbn-108972-kibana-t…

31fe82e

…heme-provider

self-review, first batch

9dc6c50

self-review, second batch

be04814

add tests for core theme provider

d4aec89

add theme service tests

25dae34

add more tests

322e357

add test for to_mount_point

e422e70

Merge remote-tracking branch 'upstream/main' into kbn-108972-kibana-t…

a6a360f

…heme-provider

update generated doc

89713bf

fix type in story file

9861131

fix type in fleet story context

0225446

jportner approved these changes Nov 9, 2021

View reviewed changes

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Nov 9, 2021

juliaElastic approved these changes Nov 10, 2021

View reviewed changes

sabarasaba approved these changes Nov 10, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/main' into kbn-108972-kibana-t…

04d1d5f

…heme-provider

thompsongl reviewed Nov 11, 2021

View reviewed changes

mshustov approved these changes Nov 11, 2021

View reviewed changes

thompsongl mentioned this pull request Nov 11, 2021

[CSS-in-JS] Theme exports elastic/eui#5374

Merged

jloleysens approved these changes Nov 15, 2021

View reviewed changes

pgayvallet added 4 commits November 16, 2021 10:59

Merge remote-tracking branch 'upstream/main' into kbn-108972-kibana-t…

2a948ed

…heme-provider

update generated doc

3fd5e28

review comments

ba879be

introduce CoreContextProvider

3a23426

elastic deleted a comment from kibanamachine Nov 16, 2021

Merge remote-tracking branch 'upstream/main' into kbn-108972-kibana-t…

f620cff

…heme-provider

pgayvallet merged commit 04d5762 into elastic:main Nov 17, 2021

kibanamachine added the backport:skip This commit does not require backporting label Nov 17, 2021

This was referenced Nov 17, 2021

add theme$ to ManagementAppMountParams #118852

Merged

[meta] Adapt all existing applications to use the new KibanaThemeProvider #118866

Closed

thompsongl mentioned this pull request Nov 24, 2021

[CSS-in-JS] @emotion/react in src/ elastic/eui#5121

Merged

21 tasks

pgayvallet mentioned this pull request Dec 12, 2023

Implement system option for theme:darkMode uiSetting #173044

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wrapper and utilities around `EuiThemeProvider` #117368

Add wrapper and utilities around `EuiThemeProvider` #117368

pgayvallet commented Nov 3, 2021 •

edited

Loading

jportner left a comment

elasticmachine commented Nov 9, 2021

sabarasaba left a comment

thompsongl left a comment

thompsongl Nov 11, 2021

mshustov Nov 11, 2021

mshustov Nov 11, 2021

pgayvallet Nov 12, 2021

mshustov Nov 15, 2021

pgayvallet commented Nov 12, 2021

jloleysens left a comment

jloleysens Nov 15, 2021

kibana-ci commented Nov 17, 2021

API count

Add wrapper and utilities around EuiThemeProvider #117368

Add wrapper and utilities around EuiThemeProvider #117368

Conversation

pgayvallet commented Nov 3, 2021 • edited Loading

Summary

core

kibana_react

Checklist

jportner left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 9, 2021

sabarasaba left a comment

Choose a reason for hiding this comment

thompsongl left a comment

Choose a reason for hiding this comment

thompsongl Nov 11, 2021

Choose a reason for hiding this comment

mshustov Nov 11, 2021

Choose a reason for hiding this comment

mshustov Nov 11, 2021

Choose a reason for hiding this comment

pgayvallet Nov 12, 2021

Choose a reason for hiding this comment

mshustov Nov 15, 2021

Choose a reason for hiding this comment

pgayvallet commented Nov 12, 2021

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Nov 15, 2021

Choose a reason for hiding this comment

kibana-ci commented Nov 17, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Add wrapper and utilities around `EuiThemeProvider` #117368

Add wrapper and utilities around `EuiThemeProvider` #117368

pgayvallet commented Nov 3, 2021 •

edited

Loading