Implement system option for theme:darkMode uiSetting

[pgayvallet]

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.

Screen.Recording.2023-12-12.at.16.45.15.mov

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.

kibana/packages/core/theme/core-theme-browser-internal/src/core_theme_provider.tsx

Line 11 in 477505a

import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme';

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:

Screen.Recording.2023-12-11.at.13.25.59.mov

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

kibana/packages/core/rendering/core-rendering-server-internal/src/render_utils.ts

Lines 40 to 54 in 6443b57

	return [
	...(darkMode
	? [
	`${bundlesHref}/kbn-ui-shared-deps-npm/${UiSharedDepsNpm.darkCssDistFilename(
	themeVersion
	)}`,
	`${bundlesHref}/kbn-ui-shared-deps-src/${UiSharedDepsSrc.cssDistFilename}`,
	`${baseHref}/ui/legacy_dark_theme.min.css`,
	]
	: [
	`${bundlesHref}/kbn-ui-shared-deps-npm/${UiSharedDepsNpm.lightCssDistFilename(
	themeVersion
	)}`,
	`${bundlesHref}/kbn-ui-shared-deps-src/${UiSharedDepsSrc.cssDistFilename}`,
	`${baseHref}/ui/legacy_light_theme.min.css`,

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

kibana/src/plugins/controls/public/control_group/component/control_group_component.tsx

Line 9 in f1dc1e1

import '../control_group.scss';

importing this file:

kibana/src/plugins/controls/public/control_group/control_group.scss

Lines 107 to 110 in bafb235

	.controlFrame__control {
	height: 100%;
	transition: opacity .1s;
	background-color: $euiFormBackgroundColor !important;

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:

Screen.Recording.2023-12-11.at.20.34.52.mov

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.

kibana/packages/core/rendering/core-rendering-server-internal/src/views/styles.tsx

Lines 52 to 54 in 6443b57

	html {
	background-color: ${darkMode ? '#141519' : '#F8FAFD'}
	}

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.

kibana/src/plugins/chart_expressions/expression_metric/public/components/metric_vis.tsx

Line 41 in c7e7853

import { euiThemeVars } from '@kbn/ui-theme';

kibana/src/plugins/chart_expressions/expression_metric/public/components/metric_vis.tsx

Line 50 in c7e7853

export const defaultColor = euiThemeVars.euiColorLightestShade;

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

kibana/packages/kbn-ui-theme/src/theme.ts

Lines 30 to 42 in b0a0017

	let isDarkMode = darkMode;
	export const _setDarkMode = (mode: boolean) => {
	isDarkMode = mode;
	};
	/**
	* EUI Theme vars that automatically adjust to light/dark theme
	*/
	export const euiThemeVars: Theme = new Proxy(isDarkMode ? euiDarkVars : euiLightVars, {
	get(accessedTarget, accessedKey, ...rest) {
	return Reflect.get(isDarkMode ? euiDarkVars : euiLightVars, accessedKey, ...rest);
	},
	});

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)

[pgayvallet]

/ci

[pgayvallet]

/ci

[pgayvallet]

/ci

[pgayvallet]

/ci

[pgayvallet]

/ci

[pgayvallet]

/ci

[azasypkin]

Changes in user_settings_service.ts LGTM.

[azasypkin]

optional nit: buildNum is unused.

[pgayvallet]

Blocked by #173529

[legrego]

Blocked by #173529

@pgayvallet now that this is resolved, are there any other blockers for this PR?

[pgayvallet]

are there any other blockers for this PR?

No, I just couldn't find the time to get back to it and resolve the conflicts, but it's on my TODO and will hopefully be done very soon!

[jloleysens]

Overall looks good to me! Thanks for all the comments. I left a minor comment about possibly unstyled content.

[jloleysens]

I know we are not doing this on-the-fly yet, but I am a bit concerned this will result in a "flash of unstyled content". It's a tricky one to solve for though. For now this is totally fine.

[pgayvallet]

Yeah, I shared the concerns, but in practice this is performed before the current app gets mounted, and during my tests I couldn't reproduce any css blinks. But yeah this is something we need to keep in mind

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 26f02dc

Failed CI Steps

• FTR Configs #43

Test Failures

• [job] [logs] FTR Configs #43 / Actions and Triggers app Rules list rules list bulk actions should apply filters to bulk actions when using the select all button

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
alerting	155	156	+1
core	502	504	+2
triggersActionsUi	688	689	+1
total			+4

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
@kbn/core-ui-settings-common	4	6	+2
@kbn/ui-theme	6	8	+2
total			+4

Async chunks

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

id	before	after	diff
kibanaOverview	51.9KB	51.9KB	+12.0B
security	579.8KB	579.8KB	+12.0B
total			+24.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
@kbn/core-injected-metadata-common-internal	3	4	+1

Page load bundle

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

id	before	after	diff
core	384.6KB	385.5KB	+947.0B
kbnUiSharedDeps-srcJs	2.9MB	2.9MB	+119.0B
total			+1.0KB

Unknown metric groups

API count

id	before	after	diff
@kbn/core-ui-settings-common	27	30	+3
@kbn/ui-theme	7	9	+2
total			+5

ESLint disabled in files

id	before	after	diff
@kbn/core-apps-server-internal	0	1	+1

References to deprecated APIs

id	before	after	diff
@kbn/monaco	0	2	+2
securitySolution	527	532	+5
total			+7

Total ESLint disabled count

id	before	after	diff
@kbn/core-apps-server-internal	0	1	+1

Unreferenced deprecated APIs

id	before	after	diff
@kbn/ui-theme	0	2	+2

History

• 💔 Build #194311 failed 344e1c0
• 💚 Build #183940 succeeded cc9fde6
• 💛 Build #183335 was flaky 2b75535
• 💔 Build #183233 failed 1b5cb27
• 💔 Build #183188 failed ff22176

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

[petrklapka]

@sebelga @pgayvallet - there's an incongruity between "system" and the toggle on the user menu only offering Dark/Light mode settings. Should we have an issue to reconcile this? If I set "system" in advanced settings, what should the behavior of the toggle on User Menu be? Should the option disappear from the menu?

[sebelga]

I think the user menu options should match the advanced settings and offer the third option "Sync with system".

[pgayvallet]

@petrklapka @sebelga this is being tracked by #173895

[petrklapka]

Thanks both. @JasonStoltz looks like this is already covered by the Core and Security folks.