From 1e5addb9f1a1e64d68c8e7e9a9c66dc4dc3f4392 Mon Sep 17 00:00:00 2001 From: Viraj Sanghvi Date: Mon, 19 Aug 2024 09:06:01 -0700 Subject: [PATCH] refactor: simplify theme configuration and defaulting (#7625) (#7746) --------- Signed-off-by: Viraj Sanghvi Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit bcdbbef7b5ef42b10bdf9aa65ef8b96c126dba82) --- changelogs/fragments/7625.yml | 5 + docs/theme.md | 162 ++++++++++++++++++ .../osd-optimizer/src/common/theme_tags.ts | 9 +- packages/osd-ui-shared-deps/index.d.ts | 10 +- packages/osd-ui-shared-deps/index.js | 5 + packages/osd-ui-shared-deps/theme.ts | 9 +- packages/osd-ui-shared-deps/theme_config.d.ts | 41 +++++ packages/osd-ui-shared-deps/theme_config.js | 44 +++++ .../ui_settings_client.test.ts.snap | 13 ++ src/core/public/ui_settings/types.ts | 7 + .../ui_settings/ui_settings_client.test.ts | 18 ++ .../public/ui_settings/ui_settings_client.ts | 37 ++-- .../ui_settings/ui_settings_service.mock.ts | 1 + .../server/rendering/rendering_service.tsx | 7 +- src/core/server/ui_settings/settings/theme.ts | 13 +- src/core/server/ui_settings/types.ts | 4 + .../ui_settings/ui_settings_client.test.ts | 19 ++ .../server/ui_settings/ui_settings_client.ts | 4 + .../server/ui_settings/ui_settings_config.ts | 4 +- .../ui_settings/ui_settings_service.mock.ts | 1 + .../ui/ui_render/bootstrap/startup.js.hbs | 8 +- src/legacy/ui/ui_render/ui_render_mixin.js | 31 ++-- .../public/header_user_theme_menu.tsx | 28 ++- .../dashboard_top_nav.test.tsx.snap | 6 + .../dashboard_empty_screen.test.tsx.snap | 3 + .../language_selector.test.tsx.snap | 2 + 26 files changed, 426 insertions(+), 65 deletions(-) create mode 100644 changelogs/fragments/7625.yml create mode 100644 docs/theme.md create mode 100644 packages/osd-ui-shared-deps/theme_config.d.ts create mode 100644 packages/osd-ui-shared-deps/theme_config.js diff --git a/changelogs/fragments/7625.yml b/changelogs/fragments/7625.yml new file mode 100644 index 000000000000..e4ce4fbe7e57 --- /dev/null +++ b/changelogs/fragments/7625.yml @@ -0,0 +1,5 @@ +refactor: +- Simplify theme configuration and defaulting ([#7625](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7625)) + +deprecate: +- Deprecating `CssDistFilename` exports in favor of `themeCssDistFilenames` in `@osd/ui-shared-deps` ([#7625](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7625)) \ No newline at end of file diff --git a/docs/theme.md b/docs/theme.md new file mode 100644 index 000000000000..96873d1cbdff --- /dev/null +++ b/docs/theme.md @@ -0,0 +1,162 @@ +# Theme System + +## Basic concepts + +### Theme definitions in OUI + +Themes are defined in OUI via https://github.com/opensearch-project/oui/blob/main/src/themes/themes.ts. When Building OUI, there are several theming artifacts generated (beyond the react components) for each mode (light/dark) of each theme: + +1. Theme compiled stylesheets (e.g. `@elastic/eui/dist/eui_theme_dark.css`). Consumed as entry files in [/packages/osd-ui-shared-deps/webpack.config.js](/packages/osd-ui-shared-deps/webpack.config.js) and republished by `osd-ui-shared-deps` (e.g. [UiSharedDeps.themeCssDistFilenames](/packages/osd-ui-shared-deps/index.js)). +2. Theme compiled and minified stylesheets (e.g. `@elastic/eui/dist/eui_theme_dark.min.css`). These appear unused by OpenSearch Dashboards +3. Theme computed SASS variables as JSON (e.g. `@elastic/eui/dist/eui_theme_dark.json`). Consumed by [/packages/osd-ui-shared-deps/theme.ts](/packages/osd-ui-shared-deps/theme.ts) and made available to other components via the mode and theme aware `euiThemeVars`. In general, these should not be consumed by any other component directly. +4. Theme type definition file for SASS variables as JSON (e.g. `@elastic/eui/dist/eui_theme_dark.json.d.ts`) + +Note that all of these artifacts should ideally only be imported or used directly in one place (by `osd-ui-shared-deps`). + +In addition to these artifacts, OpenSearch Dashboards also makes heavy use of the theme SASS variables and mixins as defined in the source files (e.g. `@elastic/eui/src/theme_dark.scss`). + +### Theme definitions in OpenSearch Dashboards + +1. Theme tags are defined in [/packages/osd-optimizer/src/common/theme_tags.ts](/packages/osd-optimizer/src/common/theme_tags.ts) corresponding to each mode (light/dark) of each OUI theme. +2. These tags must correspond to entrypoint SCSS files in [/src/core/public/core_app/styles/](/src/core/public/core_app/styles/_globals_v8dark.scss), because they are imported by all SCSS files as part of the `sass-loader` in [/packages/osd-optimizer/src/worker/webpack.config.ts](/packages/osd-optimizer/src/worker/webpack.config.ts) and [/packages/osd-optimizer/src/worker/theme_loader.ts](/packages/osd-optimizer/src/worker/theme_loader.ts). Note that the optimizer webpack will compile a separate stylesheet for each unique mode and theme combination. +3. OUI SCSS source files are also imported by `osd-ui-framework`, which generates the legacy KUI stylesheets (e.g. [/packages/osd-ui-framework/src/kui_next_dark.scss](/packages/osd-ui-framework/src/kui_next_dark.scss)). KUI is a UI library that predates EUI/OUI, and should be deprecated and fully removed via [#1060](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/1060). Because it's a legacy package it has its own build process that doesn't use webpack; it just [compiles the SCSS files with grunt](/packages/osd-ui-framework/Gruntfile.js). But similarly to 2., a separate stylesheet is generated for each mode and theme combination. + +### Thmemed assets in OpenSearch Dasboards + +In general, most themed assests can be found in [/src/core/server/core_app/assets](src/core/server/core_app/assets/fonts/readme.md) (it also includes non-themed assets such as `favicons`, which could easily be themed if desired in the future). + +Most of the graphics/images are only dark/light mode-specific, not theme-specific: + +1. `default_branding` marks +2. `logos` + +This directory also includes legacy CSS files ([/src/core/server/core_app/assets/legacy_dark_theme.css](/src/core/server/core_app/assets/legacy_dark_theme.css) and [/src/core/server/core_app/assets/legacy_light_theme.css](/src/core/server/core_app/assets/legacy_light_theme.css)), which predate even KUI, and are still used by some plugins (notably `discover`). See [#4385](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4385) for an experiment in removing these. Unlike KUI, they don't rely on OUI themes at all. + +Finally, font assets are a bit of a special case. Theme-specific fonts are defined by OUI, but it doesn't include the font definitions directly. Instead, the font assets are in [/src/core/server/core_app/assets/fonts](/src/core/server/core_app/assets/fonts/readme.md). The corresponding `@font-face` style definitions are generated at runtime via [/src/core/server/rendering/views/fonts.tsx](/src/core/server/rendering/views/fonts.tsx). + +## Theme settings + +## Theme loading + +```mermaid +sequenceDiagram + autonumber + critical Setup + core/server->>core/server/rendering: setup rendering service + core/server/rendering->>core/server: provide render() method + core/server->>core/server: setup legacy service + core/server->>legacy: create legacy server + legacy->>legacy: start ui mixin to
handle special routes + core/server->>core/server/core_app: setup core app + core/server/core_app->>core/server/core_app: register default routes + core/server/core_app->>core/server/core_app: register static asset dir routes + end + Browser->>core/server: OSD page request (e.g. /app/home#/ ) + core/server->>core/server/core_app: request to default route
(via `http` service) + core/server/core_app->>core/server: call renderCoreApp() + core/server->>core/server/rendering: call render() + critical Initial page bootstrap + core/server/rendering->>core/server/rendering: get theme settings from config + core/server/rendering->>core/server/rendering: assign branding values \
(including dark mode) + core/server/rendering->>Browser: return static loading page template + Note over core/server/rendering,Browser: includes inlined font-face styles and static loading page styles + critical (render blocking) + Browser->>Browser: define injection points + Browser->>Browser: load static loading page styles + Browser->>Browser: load font-face styles + Browser->>legacy: load startup.js special route + legacy->>legacy: build startup.js from template + Note over legacy: inject theme settings and font sources + legacy->>Browser: startup.js + critical startup.js + Browser->>Browser: get theme preferences from local storage + Browser->>Browser: set global theme tag + Browser->>Browser: inject theme-specific loading page styles + Browser->>Browser: inject theme-specific font css vars + end + end + Browser->>Browser: render loading/error page
(with loaders hidden) + Browser->>legacy: load bootstrap.js special route + legacy->>legacy: build bootstrap.js from template + legacy->>Browser: bootstrap.js + critical bootstrap.js + Browser->>Browser: toggle visibility of errors/loaders + Browser->>Browser: get theme preferences from local storage + Browser->>core/server/core_app: load js bundles + core/server/core_app->>Browser: (React application) + Browser->>core/server/core_app: load theme-specific stylesheets
(base, OUI, KUI, legacy) + core/server/core_app->>Browser: themed css + end + end +``` + +### Loading + +`src/legacy/ui/ui_render/ui_render_mixin.js` via `src/legacy/ui/ui_render/bootstrap/template.js.hbs` and `src/legacy/ui/ui_render/bootstrap/app_bootstrap.js`. Aliased in `src/legacy/ui/ui_mixin.js`, called by `src/legacy/server/osd_server.js`. Called by `src/core/server/legacy/legacy_service.ts` via `src/core/server/server.ts` + +### Injected style tags + +1. `src/core/server/rendering/views/styles.tsx` - depends on dark/light mode and injects style tag in head +2. `src/core/server/rendering/views/fonts.tsx` - depends on theme version and injects font style tag in head +3. Monaco editor styles +4. Ace styles +5. Ace TM overrides +6. Ace error styles +6. Component styles + +### Styleshsheets loaded + +Each of the following are loaded in the browser by the [bootstrap script](/src/legacy/ui/ui_render/bootstrap/template.js.hbs) in this order. Currently, these are never unloaded. + +1. Monaco editor styles (e.g. [/packages/osd-ui-shared-deps/target/osd-ui-shared-deps.css](/packages/osd-ui-shared-deps/target/osd-ui-shared-deps.css)), packaged by [/packages/osd-ui-shared-deps/webpack.config.js](/packages/osd-ui-shared-deps/webpack.config.js). In theory, this file could include styles from other shared dependencies, but currently `osd-monaco` is the only package that exports styles. Note that these are the default, un-themed styles; theming of monaco editors is handled by [/src/plugins/opensearch_dashboards_react/public/code_editor/editor_theme.ts](/src/plugins/opensearch_dashboards_react/public/code_editor/editor_theme.ts). +2. Theme and mode-specific OUI styles (e.g. [](), compiled by `packages/osd-ui-shared-deps/webpack.config.js`). +3. Theme and mode-specific KUI styles (e.g. `packages/osd-ui-framework/src/kui_next_dark.scss`, compiled by `packages/osd-ui-framework/Gruntfile.js`). Separate stylesheets for each theme version/dark mode combo (colors). +4. Mode-specific legacy styles (e.g. [/src/core/server/core_app/assets/legacy_dark_theme.css](/src/core/server/core_app/assets/legacy_dark_theme.css)) + +Component styles are not loaded as stylesheets. + +## Current theme usage + +### JSON/JS Vars + +1. Defined by `packages/osd-ui-shared-deps/theme.ts` + 1. Used by `src/plugins/charts/public/static/color_maps/color_maps.ts` to set vis colors + 2. Used by `src/plugins/discover/public/application/components/chart/histogram/histogram.tsx` to define Discover histogram Elastic Chart styling + 3. Used by `src/plugins/maps_legacy/public/map/opensearch_dashboards_map.js` and `src/plugins/region_map/public/choropleth_layer.js` for minor map UI styling (line color, empty shade) + 4. Used by `src/plugins/vis_type_vega/public/data_model/vega_parser.ts` for Vega/Vega-Lite theming +2. Used by `src/plugins/vis_type_vislib/public/vislib/components/tooltip/tooltip.js` for tooltip spacing +3. Used by `src/plugins/expressions/public/react_expression_renderer.tsx` to define padding options. +4. Used by `src/core/server/rendering/views/theme.ts` to inject values into `src/core/server/rendering/views/styles.tsx` +5. Used (incorrectly) to style a badge color in `src/plugins/index_pattern_management/public/components/create_button/create_button.tsx` +6. Used by `src/plugins/opensearch_dashboards_react/public/code_editor/editor_theme.ts` to create Monaco theme styles + +## Theme Management + +### Change default theme + +Update `DEFAULT_THEME_VERSION` in `src/core/server/ui_settings/ui_settings_config.ts` to point to the desired theme version. + +### Adding a new theme + +1. Add a [a new theme to OUI](https://github.com/opensearch-project/oui/blob/main/wiki/theming.md) and publish new OUI version +2. Update OSD to consume new OUI version +3. Make the following changes in OSD: + 1. Load your theme by creating sass files in `src/core/public/core_app/styles` + 2. Update [webpack config](packages/osd-ui-shared-deps/webpack.config.js) to create css files for your theme + 2. Add kui css files: + 1. Create kui sass files for your theme in `packages/osd-ui-framework/src/` + 2. Update `packages/osd-ui-framework/Gruntfile.js` to build these files + 3. Generate the files by running `npx grunt compileCss` from this package root + 3. Add fonts to OSD: + 1. Make sure your theme fonts are in [/src/core/server/core_app/assets/fonts](/src/core/server/core_app/assets/fonts/readme.md) + 2. Update `src/core/server/rendering/views/fonts.tsx` to reference those files + 3. Update src/core/server/core_app/assets/fonts/readme.md to reference the fonts + 4. Update `packages/osd-ui-shared-deps/theme_config.js`: + 1. Add version and label for version to `THEME_VERSION_LABEL_MAP` + 2. Update `kuiCssDistFilenames` map for new theme + 3. Update `ThemeTag` type in corresponding definition file (`theme_config.d.ts`) + 5. Load variables for new theme in `packages/osd-ui-shared-deps/theme.ts'` + 6. Update `src/legacy/ui/ui_render/ui_render_mixin.js': + 1. Load variables for your theme in `THEME_SOURCES` + 2. Define the text font for your theme in `fontText` + 3. Define the code font for your theme in `fontCode` diff --git a/packages/osd-optimizer/src/common/theme_tags.ts b/packages/osd-optimizer/src/common/theme_tags.ts index 8170c6bcab69..3078a9c6dc06 100644 --- a/packages/osd-optimizer/src/common/theme_tags.ts +++ b/packages/osd-optimizer/src/common/theme_tags.ts @@ -28,6 +28,8 @@ * under the License. */ +import { themeTags as THEME_TAGS } from '@osd/ui-shared-deps'; +import type { ThemeTag, ThemeTags } from '@osd/ui-shared-deps'; import { ascending } from './array_helpers'; const tags = (...themeTags: string[]) => @@ -37,10 +39,9 @@ const validTag = (tag: any): tag is ThemeTag => ALL_THEMES.includes(tag); const isArrayOfStrings = (input: unknown): input is string[] => Array.isArray(input) && input.every((v) => typeof v === 'string'); -export type ThemeTags = readonly ThemeTag[]; -export type ThemeTag = 'v7light' | 'v7dark' | 'v8light' | 'v8dark'; -export const DEFAULT_THEMES = tags('v7light', 'v7dark', 'v8light', 'v8dark'); -export const ALL_THEMES = tags('v7light', 'v7dark', 'v8light', 'v8dark'); +export type { ThemeTag, ThemeTags }; +export const DEFAULT_THEMES = tags(...THEME_TAGS); +export const ALL_THEMES = tags(...THEME_TAGS); export function parseThemeTags(input?: any): ThemeTags { if (!input) { diff --git a/packages/osd-ui-shared-deps/index.d.ts b/packages/osd-ui-shared-deps/index.d.ts index 49192a18d291..4e03db427bd4 100644 --- a/packages/osd-ui-shared-deps/index.d.ts +++ b/packages/osd-ui-shared-deps/index.d.ts @@ -43,6 +43,11 @@ export const jsFilename: string; */ export const jsDepFilenames: string[]; +/** + * Re-export all types from theme_config + */ +export * from './theme_config'; + /** * Filename of the unthemed css file in the distributable directory */ @@ -50,24 +55,27 @@ export const baseCssDistFilename: string; /** * Filename of the dark-theme css file in the distributable directory + * @deprecated */ export const darkCssDistFilename: string; /** * Filename of the dark-theme css file in the distributable directory + * @deprecated */ export const darkV8CssDistFilename: string; /** * Filename of the light-theme css file in the distributable directory + * @deprecated */ export const lightCssDistFilename: string; /** * Filename of the light-theme css file in the distributable directory + * @deprecated */ export const lightV8CssDistFilename: string; - /** * Externals mapping inteded to be used in a webpack config */ diff --git a/packages/osd-ui-shared-deps/index.js b/packages/osd-ui-shared-deps/index.js index 36218a28d4eb..fe3de2b1f003 100644 --- a/packages/osd-ui-shared-deps/index.js +++ b/packages/osd-ui-shared-deps/index.js @@ -30,13 +30,18 @@ const Path = require('path'); +Object.assign(exports, require('./theme_config')); exports.distDir = Path.resolve(__dirname, 'target'); exports.jsDepFilenames = ['osd-ui-shared-deps.@elastic.js']; exports.jsFilename = 'osd-ui-shared-deps.js'; exports.baseCssDistFilename = 'osd-ui-shared-deps.css'; +/** @deprecated */ exports.lightCssDistFilename = 'osd-ui-shared-deps.v7.light.css'; +/** @deprecated */ exports.lightV8CssDistFilename = 'osd-ui-shared-deps.v8.light.css'; +/** @deprecated */ exports.darkCssDistFilename = 'osd-ui-shared-deps.v7.dark.css'; +/** @deprecated */ exports.darkV8CssDistFilename = 'osd-ui-shared-deps.v8.dark.css'; exports.externals = { // stateful deps diff --git a/packages/osd-ui-shared-deps/theme.ts b/packages/osd-ui-shared-deps/theme.ts index c803a5e37ef7..082f468175e2 100644 --- a/packages/osd-ui-shared-deps/theme.ts +++ b/packages/osd-ui-shared-deps/theme.ts @@ -37,13 +37,14 @@ export type Theme = typeof LightTheme; // in the OpenSearch Dashboards app we can rely on this global being defined, but in // some cases (like jest) the global is undefined -export const tag: string = globals.__osdThemeTag__ || 'v8light'; -export const version = tag.startsWith('v7') ? 7 : 8; -export const darkMode = tag.endsWith('dark'); +export const tag: string = globals.__osdThemeTag__; +const themeVersion = tag?.replace(/(light|dark)$/, '') || 'v8'; +export const version = parseInt(themeVersion.replace(/[^\d]+/g, ''), 10) || 8; +export const darkMode = tag?.endsWith?.('dark'); export let euiLightVars: Theme; export let euiDarkVars: Theme; -if (version === 7) { +if (themeVersion === 'v7') { euiLightVars = require('@elastic/eui/dist/eui_theme_light.json'); euiDarkVars = require('@elastic/eui/dist/eui_theme_dark.json'); } else { diff --git a/packages/osd-ui-shared-deps/theme_config.d.ts b/packages/osd-ui-shared-deps/theme_config.d.ts new file mode 100644 index 000000000000..0ca80280434f --- /dev/null +++ b/packages/osd-ui-shared-deps/theme_config.d.ts @@ -0,0 +1,41 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Types for valid theme tags (themeVersion + themeMode) + * Note: used by @osd/optimizer + */ +export type ThemeTag = 'v7light' | 'v7dark' | 'v8light' | 'v8dark'; +export type ThemeTags = readonly ThemeTag[]; + +/** + * List of valid ThemeTags + * Note: used by @osd/optimizer + */ +export const themeTags: ThemeTags; + +/** + * Map of themeVersion values to labels + * Note: this is used for ui display + */ +export const themeVersionLabelMap: Record; + +/** + * Map of labels and versions to themeVersion values + * Note: this is used to correct incorrectly persisted ui settings + */ +export const themeVersionValueMap: Record; + +/** + * Theme CSS distributable filenames by themeVersion and themeMode + * Note: used by bootstrap template + */ +export const themeCssDistFilenames: Record>; + +/** + * KUI CSS distributable filenames by themeVersion and themeMode + * Note: used by bootstrap template + */ +export const kuiCssDistFilenames: Record>; diff --git a/packages/osd-ui-shared-deps/theme_config.js b/packages/osd-ui-shared-deps/theme_config.js new file mode 100644 index 000000000000..f128925bbbcb --- /dev/null +++ b/packages/osd-ui-shared-deps/theme_config.js @@ -0,0 +1,44 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * The purpose of this file is to centalize theme configuration so it can be used across server, + * client, and dev tooling. DO NOT add dependencies that wouldn't operate in all of these contexts. + * + * Default theme is specified in the uiSettings schema. + */ + +const THEME_MODES = ['light', 'dark']; +const THEME_VERSION_LABEL_MAP = { + v7: 'v7', + v8: 'Next (preview)', +}; +const THEME_VERSION_VALUE_MAP = { + // allow version lookup by label ... + ...Object.fromEntries(Object.entries(THEME_VERSION_LABEL_MAP).map((a) => a.reverse())), + // ... or by the version itself + ...Object.fromEntries(Object.keys(THEME_VERSION_LABEL_MAP).map((v) => [v, v])), +}; +const THEME_VERSIONS = Object.keys(THEME_VERSION_LABEL_MAP); +const THEME_TAGS = THEME_VERSIONS.flatMap((v) => THEME_MODES.map((m) => `${v}${m}`)); + +exports.themeVersionLabelMap = THEME_VERSION_LABEL_MAP; + +exports.themeVersionValueMap = THEME_VERSION_VALUE_MAP; + +exports.themeTags = THEME_TAGS; + +exports.themeCssDistFilenames = THEME_VERSIONS.reduce((map, v) => { + map[v] = THEME_MODES.reduce((acc, m) => { + acc[m] = `osd-ui-shared-deps.${v}.${m}.css`; + return acc; + }, {}); + return map; +}, {}); + +exports.kuiCssDistFilenames = { + v7: { dark: 'kui_dark.css', light: 'kui_light.css' }, + v8: { dark: 'kui_next_dark.css', light: 'kui_next_light.css' }, +}; diff --git a/src/core/public/ui_settings/__snapshots__/ui_settings_client.test.ts.snap b/src/core/public/ui_settings/__snapshots__/ui_settings_client.test.ts.snap index 68948ef0d4af..5d0b95e2938b 100644 --- a/src/core/public/ui_settings/__snapshots__/ui_settings_client.test.ts.snap +++ b/src/core/public/ui_settings/__snapshots__/ui_settings_client.test.ts.snap @@ -20,6 +20,19 @@ You can use \`IUiSettingsClient.get(\\"throwableProperty\\", defaultValue)\`, wh \`defaultValue\` when the key is unrecognized." `; +exports[`#getDefault converts json default values 1`] = ` +Object { + "a": 1, +} +`; + +exports[`#getDefault fetches correct uiSettings defaults 1`] = `"Browser"`; + +exports[`#getDefault throws on unknown properties that don't have a value yet. 1`] = ` +"Unexpected \`IUiSettingsClient.getDefaultValue(\\"unknownProperty\\")\` call on unrecognized configuration setting \\"unknownProperty\\". +Please check that the setting for \\"unknownProperty\\" exists." +`; + exports[`#getUpdate$ sends { key, newValue, oldValue } notifications when client changes 1`] = ` Array [ Array [ diff --git a/src/core/public/ui_settings/types.ts b/src/core/public/ui_settings/types.ts index 7eca4cc68d81..86f78443eee6 100644 --- a/src/core/public/ui_settings/types.ts +++ b/src/core/public/ui_settings/types.ts @@ -66,6 +66,13 @@ export interface IUiSettingsClient { */ getAll: () => Readonly>; + /** + * Gets the default value for a specific uiSetting. If the parameter is not defined and the key is + * not registered by any plugin then an error is thrown, otherwise reads the default value defined by + * a plugin. + */ + getDefault: (key: string) => T; + /** * Sets the value for a uiSetting. If the setting is not registered by any plugin * it will be stored as a custom setting. The new value will be synchronously available via diff --git a/src/core/public/ui_settings/ui_settings_client.test.ts b/src/core/public/ui_settings/ui_settings_client.test.ts index 9060b0d6db4e..9cf4985f440c 100644 --- a/src/core/public/ui_settings/ui_settings_client.test.ts +++ b/src/core/public/ui_settings/ui_settings_client.test.ts @@ -64,6 +64,24 @@ afterEach(() => { done$.complete(); }); +describe('#getDefault', () => { + it('fetches correct uiSettings defaults', () => { + const { client } = setup(); + expect(client.getDefault('dateFormat')).toMatchSnapshot(); + expect(client.getDefault('aLongNumeral')).toBe(BigInt(Number.MAX_SAFE_INTEGER) + 11n); + }); + + it('converts json default values', () => { + const { client } = setup({ defaults: { test: { value: '{"a": 1}', type: 'json' } } }); + expect(client.getDefault('test')).toMatchSnapshot(); + }); + + it("throws on unknown properties that don't have a value yet.", () => { + const { client } = setup(); + expect(() => client.getDefault('unknownProperty')).toThrowErrorMatchingSnapshot(); + }); +}); + describe('#get', () => { it('gives access to uiSettings values', () => { const { client } = setup(); diff --git a/src/core/public/ui_settings/ui_settings_client.ts b/src/core/public/ui_settings/ui_settings_client.ts index 8a5701de6b39..ac548278bf47 100644 --- a/src/core/public/ui_settings/ui_settings_client.ts +++ b/src/core/public/ui_settings/ui_settings_client.ts @@ -32,7 +32,7 @@ import { cloneDeep, defaultsDeep } from 'lodash'; import { Observable, Subject, concat, defer, of } from 'rxjs'; import { filter, map } from 'rxjs/operators'; -import { UserProvidedValues, PublicUiSettingsParams } from 'src/core/server/types'; +import { UserProvidedValues, PublicUiSettingsParams, UiSettingsType } from 'src/core/server/types'; import { IUiSettingsClient, UiSettingsState } from './types'; import { UiSettingsApi } from './ui_settings_api'; @@ -71,6 +71,18 @@ export class UiSettingsClient implements IUiSettingsClient { return cloneDeep(this.cache); } + getDefault(key: string): T { + const declared = this.isDeclared(key); + + if (!declared) { + throw new Error( + `Unexpected \`IUiSettingsClient.getDefaultValue("${key}")\` call on unrecognized configuration setting "${key}". +Please check that the setting for "${key}" exists.` + ); + } + return this.resolveValue(this.cache[key].value, this.cache[key].type); + } + get(key: string, defaultOverride?: T) { const declared = this.isDeclared(key); @@ -92,16 +104,7 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r const userValue = this.cache[key].userValue; const defaultValue = defaultOverride !== undefined ? defaultOverride : this.cache[key].value; const value = userValue == null ? defaultValue : userValue; - - if (type === 'json') { - return JSON.parse(value); - } - - return type === 'number' && typeof value !== 'bigint' - ? isFinite(value) && (value > Number.MAX_SAFE_INTEGER || value < Number.MIN_SAFE_INTEGER) - ? BigInt(value) - : parseFloat(value) - : value; + return this.resolveValue(value, type); } get$(key: string, defaultOverride?: T) { @@ -173,6 +176,18 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r return this.updateErrors$.asObservable(); } + private resolveValue(value: any, type: UiSettingsType | undefined) { + if (type === 'json') { + return JSON.parse(value); + } + + return type === 'number' && typeof value !== 'bigint' + ? isFinite(value) && (value > Number.MAX_SAFE_INTEGER || value < Number.MIN_SAFE_INTEGER) + ? BigInt(value) + : parseFloat(value) + : value; + } + private assertUpdateAllowed(key: string) { if (this.isOverridden(key)) { throw new Error( diff --git a/src/core/public/ui_settings/ui_settings_service.mock.ts b/src/core/public/ui_settings/ui_settings_service.mock.ts index de9477ed0e08..231627fa53bd 100644 --- a/src/core/public/ui_settings/ui_settings_service.mock.ts +++ b/src/core/public/ui_settings/ui_settings_service.mock.ts @@ -36,6 +36,7 @@ import { IUiSettingsClient } from './types'; const createSetupContractMock = () => { const setupContract: jest.Mocked = { getAll: jest.fn(), + getDefault: jest.fn(), get: jest.fn(), get$: jest.fn(), set: jest.fn(), diff --git a/src/core/server/rendering/rendering_service.tsx b/src/core/server/rendering/rendering_service.tsx index acaee7f42bc5..a94056372667 100644 --- a/src/core/server/rendering/rendering_service.tsx +++ b/src/core/server/rendering/rendering_service.tsx @@ -33,6 +33,7 @@ import { renderToStaticMarkup } from 'react-dom/server'; import { first, take } from 'rxjs/operators'; import { i18n } from '@osd/i18n'; import { Agent as HttpsAgent } from 'https'; +import { themeVersionValueMap } from '@osd/ui-shared-deps'; import Axios from 'axios'; // @ts-expect-error untyped internal module used to prevent axios from using xhr adapter in tests @@ -102,10 +103,14 @@ export class RenderingService { false; // At the very least, the schema should define a default theme; the '' will be unreachable - const themeVersion = + const configuredThemeVersion = (settings.user?.['theme:version']?.userValue ?? uiSettings.getOverrideOrDefault('theme:version')) || ''; + // Validate themeVersion is in valid format + const themeVersion = + themeVersionValueMap[configuredThemeVersion] || + (uiSettings.getDefault('theme:version') as string); const brandingAssignment = await this.assignBrandingConfig( darkMode, diff --git a/src/core/server/ui_settings/settings/theme.ts b/src/core/server/ui_settings/settings/theme.ts index 94dec047ff35..e157da9c514e 100644 --- a/src/core/server/ui_settings/settings/theme.ts +++ b/src/core/server/ui_settings/settings/theme.ts @@ -30,7 +30,10 @@ import { schema } from '@osd/config-schema'; import { i18n } from '@osd/i18n'; +import { themeVersionLabelMap } from '@osd/ui-shared-deps'; +import type { Type } from '@osd/config-schema'; import { UiSettingsParams } from '../../../types'; +import { DEFAULT_THEME_VERSION } from '../ui_settings_config'; export const getThemeSettings = (): Record => { return { @@ -50,9 +53,9 @@ export const getThemeSettings = (): Record => { name: i18n.translate('core.ui_settings.params.themeVersionTitle', { defaultMessage: 'Theme version', }), - value: 'Next (preview)', + value: themeVersionLabelMap[DEFAULT_THEME_VERSION], type: 'select', - options: ['v7', 'Next (preview)'], + options: Object.values(themeVersionLabelMap), description: i18n.translate('core.ui_settings.params.themeVersionText', { defaultMessage: `

Switch between the themes used for the current and next versions of OpenSearch Dashboards. A page refresh is required for the setting to be applied.

{linkText}

`, values: { @@ -62,7 +65,11 @@ export const getThemeSettings = (): Record => { }), requiresPageReload: true, category: ['appearance'], - schema: schema.oneOf([schema.literal('v7'), schema.literal('Next (preview)')]), + schema: schema.oneOf( + Object.keys(themeVersionLabelMap).map((v) => schema.literal(themeVersionLabelMap[v])) as [ + Type + ] + ), }, }; }; diff --git a/src/core/server/ui_settings/types.ts b/src/core/server/ui_settings/types.ts index 60510882755a..399349f33f9e 100644 --- a/src/core/server/ui_settings/types.ts +++ b/src/core/server/ui_settings/types.ts @@ -59,6 +59,10 @@ export interface IUiSettingsClient { * Returns the overridden uiSettings value if one exists, or the registered default if one exists {@link UiSettingsParams} */ getOverrideOrDefault: (key: string) => unknown; + /** + * Returns the registered default if one exists {@link UiSettingsParams} + */ + getDefault: (key: string) => unknown; /** * Retrieves uiSettings values set by the user with fallbacks to default values if not specified. */ diff --git a/src/core/server/ui_settings/ui_settings_client.test.ts b/src/core/server/ui_settings/ui_settings_client.test.ts index f78d77e61848..e30ff651e91b 100644 --- a/src/core/server/ui_settings/ui_settings_client.test.ts +++ b/src/core/server/ui_settings/ui_settings_client.test.ts @@ -601,6 +601,25 @@ describe('ui settings', () => { }); }); + describe('#getDefault()', () => { + it(`returns the promised value for a key`, async () => { + const opensearchDocSource = {}; + const defaults = { dateFormat: { value: chance.word() } }; + const { uiSettings } = setup({ opensearchDocSource, defaults }); + const result = uiSettings.getDefault('dateFormat'); + + expect(result).toBe(defaults.dateFormat.value); + }); + + it(`returns undefined for undefined defaults`, async () => { + const opensearchDocSource = { custom: 'value' }; + const { uiSettings } = setup({ opensearchDocSource }); + const result = uiSettings.getDefault('custom'); + + expect(result).toBe(undefined); + }); + }); + describe('#get()', () => { it('pulls user configuration from OpenSearch', async () => { const opensearchDocSource = {}; diff --git a/src/core/server/ui_settings/ui_settings_client.ts b/src/core/server/ui_settings/ui_settings_client.ts index 066efa34cafd..8744cb3b80da 100644 --- a/src/core/server/ui_settings/ui_settings_client.ts +++ b/src/core/server/ui_settings/ui_settings_client.ts @@ -95,6 +95,10 @@ export class UiSettingsClient implements IUiSettingsClient { return this.isOverridden(key) ? this.overrides[key].value : this.defaults[key]?.value; } + getDefault(key: string): unknown { + return this.defaults[key]?.value; + } + async get(key: string): Promise { const all = await this.getAll(); return all[key]; diff --git a/src/core/server/ui_settings/ui_settings_config.ts b/src/core/server/ui_settings/ui_settings_config.ts index 6924640ca1e5..c539e50cf908 100644 --- a/src/core/server/ui_settings/ui_settings_config.ts +++ b/src/core/server/ui_settings/ui_settings_config.ts @@ -37,6 +37,8 @@ const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) => renameFromRoot('server.defaultRoute', 'uiSettings.overrides.defaultRoute'), ]; +export const DEFAULT_THEME_VERSION = 'v8'; + /* There are 4 levels of uiSettings: * 1) defaults hardcoded in code * 2) defaults provided in the opensearch_dashboards.yml @@ -56,7 +58,7 @@ const configSchema = schema.object({ overrides: schema.object({}, { unknowns: 'allow' }), defaults: schema.object({ 'theme:darkMode': schema.maybe(schema.boolean({ defaultValue: false })), - 'theme:version': schema.maybe(schema.string({ defaultValue: 'v8' })), + 'theme:version': schema.maybe(schema.string({ defaultValue: DEFAULT_THEME_VERSION })), }), }); diff --git a/src/core/server/ui_settings/ui_settings_service.mock.ts b/src/core/server/ui_settings/ui_settings_service.mock.ts index bb6e9ec64bd5..df34cbe1bf16 100644 --- a/src/core/server/ui_settings/ui_settings_service.mock.ts +++ b/src/core/server/ui_settings/ui_settings_service.mock.ts @@ -40,6 +40,7 @@ const createClientMock = () => { const mocked: jest.Mocked = { getRegistered: jest.fn(), getOverrideOrDefault: jest.fn(), + getDefault: jest.fn(), get: jest.fn(), getAll: jest.fn(), getUserProvided: jest.fn(), diff --git a/src/legacy/ui/ui_render/bootstrap/startup.js.hbs b/src/legacy/ui/ui_render/bootstrap/startup.js.hbs index 2a7e6f28a90b..9534a808c09f 100644 --- a/src/legacy/ui/ui_render/bootstrap/startup.js.hbs +++ b/src/legacy/ui/ui_render/bootstrap/startup.js.hbs @@ -30,14 +30,16 @@ if ({{configEnableUserControl}}) { rawThemeVersion = '{{configThemeVersion}}'; } +var themeSources = {{THEME_SOURCES}}; +var themeVersionValueMap = {{THEME_VERSION_VALUE_MAP}}; + // TODO: source of truth for mapping should be elsewhere var darkMode = rawDarkMode ? 'dark' : 'light'; -var themeVersion = rawThemeVersion === 'v7' ? 'v7' : 'v8'; +var resolvedThemeVersion = themeVersionValueMap[rawThemeVersion]; +var themeVersion = themeSources[resolvedThemeVersion] ? resolvedThemeVersion : '{{defaultThemeVersion}}'; window.__osdThemeTag__ = themeVersion + darkMode; -var themeSources = {{THEME_SOURCES}}; - var themeDefinition = themeSources[themeVersion][darkMode]; var stylesheetTarget = document.querySelector('head meta[name="add-styles-here"]'); diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 476c465edb52..24837ba4a085 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -96,14 +96,19 @@ export function uiRenderMixin(osdServer, server, config) { !authEnabled || request.auth.isAuthenticated ? await uiSettings.get('theme:darkMode') : uiSettings.getOverrideOrDefault('theme:darkMode'); + const themeMode = darkMode ? 'dark' : 'light'; - const themeVersion = + const configuredThemeVersion = !authEnabled || request.auth.isAuthenticated ? await uiSettings.get('theme:version') : uiSettings.getOverrideOrDefault('theme:version'); + // Validate themeVersion is in valid format + const themeVersion = + UiSharedDeps.themeVersionValueMap[configuredThemeVersion] || + uiSettings.getDefault('theme:version'); // Next (preview) label is mapped to v8 here - const themeTag = `${themeVersion === 'v7' ? 'v7' : 'v8'}${darkMode ? 'dark' : 'light'}`; + const themeTag = `${themeVersion}${themeMode}`; const buildHash = server.newPlatform.env.packageInfo.buildNum; const basePath = config.get('server.basePath'); @@ -112,25 +117,9 @@ export function uiRenderMixin(osdServer, server, config) { const styleSheetPaths = [ `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`, - ...(darkMode - ? [ - themeVersion === 'v7' - ? `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.darkCssDistFilename}` - : `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.darkV8CssDistFilename}`, - themeVersion === 'v7' - ? `${basePath}/node_modules/@osd/ui-framework/dist/kui_dark.css` - : `${basePath}/node_modules/@osd/ui-framework/dist/kui_next_dark.css`, - `${basePath}/ui/legacy_dark_theme.css`, - ] - : [ - themeVersion === 'v7' - ? `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.lightCssDistFilename}` - : `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.lightV8CssDistFilename}`, - themeVersion === 'v7' - ? `${basePath}/node_modules/@osd/ui-framework/dist/kui_light.css` - : `${basePath}/node_modules/@osd/ui-framework/dist/kui_next_light.css`, - `${basePath}/ui/legacy_light_theme.css`, - ]), + `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.themeCssDistFilenames[themeVersion][themeMode]}`, + `${basePath}/node_modules/@osd/ui-framework/dist/${UiSharedDeps.kuiCssDistFilenames[themeVersion][themeMode]}`, + `${basePath}/ui/legacy_${themeMode}_theme.css`, ]; const kpUiPlugins = osdServer.newPlatform.__internals.uiPlugins; diff --git a/src/plugins/advanced_settings/public/header_user_theme_menu.tsx b/src/plugins/advanced_settings/public/header_user_theme_menu.tsx index fc297aaa74bc..b1d6f09dafff 100644 --- a/src/plugins/advanced_settings/public/header_user_theme_menu.tsx +++ b/src/plugins/advanced_settings/public/header_user_theme_menu.tsx @@ -21,23 +21,17 @@ import { EuiButtonIcon, } from '@elastic/eui'; import { CoreStart } from 'opensearch-dashboards/public'; +import { themeVersionLabelMap, themeVersionValueMap } from '@osd/ui-shared-deps/theme_config'; import { useOpenSearchDashboards, useUiSetting$ } from '../../opensearch_dashboards_react/public'; export const HeaderUserThemeMenu = () => { const { services: { uiSettings }, } = useOpenSearchDashboards(); - // TODO: move to central location? - const themeOptions = [ - { - value: 'v7', - text: 'v7', - }, - { - value: 'next', - text: 'Next (preview)', - }, - ]; + const themeOptions = Object.keys(themeVersionLabelMap).map((v) => ({ + value: v, + text: themeVersionLabelMap[v], + })); const screenModeOptions = [ { value: 'light', @@ -52,13 +46,18 @@ export const HeaderUserThemeMenu = () => { text: 'Use browser settings', }, ]; + const defaultTheme = uiSettings.getDefault('theme:version'); + const defaultScreenMode = uiSettings.getDefault('theme:darkMode'); const prefersAutomatic = (window.localStorage.getItem('useBrowserColorScheme') && window.matchMedia) || false; const [darkMode, setDarkMode] = useUiSetting$('theme:darkMode'); const [themeVersion, setThemeVersion] = useUiSetting$('theme:version'); const [isPopoverOpen, setPopover] = useState(false); // TODO: improve naming? - const [theme, setTheme] = useState(themeOptions.find((t) => t.text === themeVersion)?.value); + const [theme, setTheme] = useState( + themeOptions.find((t) => t.value === themeVersionValueMap[themeVersion])?.value || + themeVersionValueMap[defaultTheme] + ); const [screenMode, setScreenMode] = useState( prefersAutomatic ? screenModeOptions[2].value @@ -66,9 +65,6 @@ export const HeaderUserThemeMenu = () => { ? screenModeOptions[1].value : screenModeOptions[0].value ); - const allSettings = uiSettings.getAll(); - const defaultTheme = allSettings['theme:version'].value; - const defaultScreenMode = allSettings['theme:darkMode'].value; const legacyAppearance = !uiSettings.get('home:useNewHomePage'); @@ -85,7 +81,7 @@ export const HeaderUserThemeMenu = () => { }; const onAppearanceSubmit = async (e: SyntheticEvent) => { - const actions = [setThemeVersion(themeOptions.find((t) => theme === t.value)?.text ?? '')]; + const actions = [setThemeVersion(themeOptions.find((t) => theme === t.value)?.value ?? '')]; if (screenMode === 'automatic') { const browserMode = window.matchMedia('(prefers-color-scheme: dark)').matches; diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index 62bb2f9e90fc..6c8cbf7bd5aa 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -929,6 +929,7 @@ exports[`Dashboard top nav render in embed mode 1`] = ` }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -1985,6 +1986,7 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -3041,6 +3043,7 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -4097,6 +4100,7 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -5153,6 +5157,7 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -6209,6 +6214,7 @@ exports[`Dashboard top nav render with all components 1`] = ` }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], diff --git a/src/plugins/dashboard/public/application/embeddable/empty/__snapshots__/dashboard_empty_screen.test.tsx.snap b/src/plugins/dashboard/public/application/embeddable/empty/__snapshots__/dashboard_empty_screen.test.tsx.snap index 3e910f5dc4f9..9889462a0472 100644 --- a/src/plugins/dashboard/public/application/embeddable/empty/__snapshots__/dashboard_empty_screen.test.tsx.snap +++ b/src/plugins/dashboard/public/application/embeddable/empty/__snapshots__/dashboard_empty_screen.test.tsx.snap @@ -173,6 +173,7 @@ exports[`DashboardEmptyScreen renders correctly with readonly mode 1`] = ` }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -520,6 +521,7 @@ exports[`DashboardEmptyScreen renders correctly with visualize paragraph 1`] = ` }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -906,6 +908,7 @@ exports[`DashboardEmptyScreen renders correctly without visualize paragraph 1`] }, "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], diff --git a/src/plugins/data/public/ui/query_editor/__snapshots__/language_selector.test.tsx.snap b/src/plugins/data/public/ui/query_editor/__snapshots__/language_selector.test.tsx.snap index 00dd4ef65d32..41755d0462a8 100644 --- a/src/plugins/data/public/ui/query_editor/__snapshots__/language_selector.test.tsx.snap +++ b/src/plugins/data/public/ui/query_editor/__snapshots__/language_selector.test.tsx.snap @@ -465,6 +465,7 @@ exports[`LanguageSelector should select DQL if language is kuery 1`] = ` "get": [MockFunction], "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction], @@ -1032,6 +1033,7 @@ exports[`LanguageSelector should select lucene if language is lucene 1`] = ` "get": [MockFunction], "get$": [MockFunction], "getAll": [MockFunction], + "getDefault": [MockFunction], "getSaved$": [MockFunction], "getUpdate$": [MockFunction], "getUpdateErrors$": [MockFunction],