diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f7d089ba687..33f8dab98a9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,7 +66,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) - [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) -- [Theme] Make theme and dark mode settings user/device specific (in local storage), with opt-out ([#5652](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5652)) ### 🐛 Bug Fixes diff --git a/docs/theme.md b/docs/theme.md deleted file mode 100644 index 293c65661c04..000000000000 --- a/docs/theme.md +++ /dev/null @@ -1,131 +0,0 @@ -# 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.darkCssDistFilename](/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 diff --git a/src/core/public/osd_bootstrap.test.ts b/src/core/public/osd_bootstrap.test.ts index e4209b460f86..806841287bee 100644 --- a/src/core/public/osd_bootstrap.test.ts +++ b/src/core/public/osd_bootstrap.test.ts @@ -34,7 +34,6 @@ import { __osdBootstrap__ } from './'; describe('osd_bootstrap', () => { beforeAll(() => { const metadata = { - branding: { darkMode: 'true' }, i18n: { translationsUrl: 'http://localhost' }, vars: { apmConfig: null }, }; diff --git a/src/core/public/osd_bootstrap.ts b/src/core/public/osd_bootstrap.ts index ed64ed0bc2b5..f5571292b83a 100644 --- a/src/core/public/osd_bootstrap.ts +++ b/src/core/public/osd_bootstrap.ts @@ -38,11 +38,6 @@ export async function __osdBootstrap__() { document.querySelector('osd-injected-metadata')!.getAttribute('data')! ); - const globals: any = typeof window === 'undefined' ? {} : window; - const themeTag: string = globals.__osdThemeTag__ || ''; - - injectedMetadata.branding.darkMode = themeTag.endsWith('dark'); - let i18nError: Error | undefined; const apmSystem = new ApmSystem(injectedMetadata.vars.apmConfig, injectedMetadata.basePath); diff --git a/src/core/public/ui_settings/ui_settings_client.ts b/src/core/public/ui_settings/ui_settings_client.ts index 19637debf948..8a5701de6b39 100644 --- a/src/core/public/ui_settings/ui_settings_client.ts +++ b/src/core/public/ui_settings/ui_settings_client.ts @@ -58,13 +58,6 @@ export class UiSettingsClient implements IUiSettingsClient { this.defaults = cloneDeep(params.defaults); this.cache = defaultsDeep({}, this.defaults, cloneDeep(params.initialSettings)); - if ( - this.cache['theme:enableUserControl']?.userValue ?? - this.cache['theme:enableUserControl']?.value - ) { - this.cache = defaultsDeep(this.cache, this.getBrowserStoredSettings()); - } - params.done$.subscribe({ complete: () => { this.update$.complete(); @@ -180,28 +173,6 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r return this.updateErrors$.asObservable(); } - private getBrowserStoredSettings() { - const uiSettingsJSON = window.localStorage.getItem('uiSettings') || '{}'; - try { - return JSON.parse(uiSettingsJSON); - } catch (error) { - this.updateErrors$.next(error); - } - return {}; - } - - private setBrowserStoredSettings(key: string, newVal: any) { - const oldSettings = this.getBrowserStoredSettings(); - const newSettings = cloneDeep(oldSettings); - if (newVal === null) { - delete newSettings[key]; - } else { - newSettings[key] = { userValue: newVal }; - } - window.localStorage.setItem(`uiSettings`, JSON.stringify(newSettings)); - return { settings: newSettings }; - } - private assertUpdateAllowed(key: string) { if (this.isOverridden(key)) { throw new Error( @@ -227,18 +198,8 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r this.setLocally(key, newVal); try { - if ( - this.cache['theme:enableUserControl']?.userValue ?? - this.cache['theme:enableUserControl']?.value - ) { - const { settings } = this.cache[key]?.preferBrowserSetting - ? this.setBrowserStoredSettings(key, newVal) - : (await this.api.batchSet(key, newVal)) || {}; - this.cache = defaultsDeep({}, defaults, this.getBrowserStoredSettings(), settings); - } else { - const { settings } = (await this.api.batchSet(key, newVal)) || {}; - this.cache = defaultsDeep({}, defaults, settings); - } + const { settings } = await this.api.batchSet(key, newVal); + this.cache = defaultsDeep({}, defaults, settings); this.saved$.next({ key, newValue: newVal, oldValue: initialVal }); return true; } catch (error) { diff --git a/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap b/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap index 01c238783ce5..ad92d759a832 100644 --- a/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap +++ b/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap @@ -8,6 +8,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": false, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, @@ -60,6 +61,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": false, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, @@ -112,6 +114,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": true, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, @@ -164,6 +167,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": true, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, @@ -220,6 +224,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/ui/default_branding", + "darkMode": false, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, @@ -272,6 +277,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": false, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, @@ -324,6 +330,7 @@ Object { "branding": Object { "applicationTitle": "OpenSearch Dashboards", "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": false, "loadingLogo": Object {}, "logo": Object {}, "mark": Object {}, diff --git a/src/core/server/rendering/rendering_service.tsx b/src/core/server/rendering/rendering_service.tsx index 6030db2fd873..acaee7f42bc5 100644 --- a/src/core/server/rendering/rendering_service.tsx +++ b/src/core/server/rendering/rendering_service.tsx @@ -95,8 +95,20 @@ export class RenderingService { defaults: uiSettings.getRegistered(), user: includeUserSettings ? await uiSettings.getUserProvided() : {}, }; + // Cannot use `uiSettings.get()` since a user might not be authenticated + const darkMode = + (settings.user?.['theme:darkMode']?.userValue ?? + uiSettings.getOverrideOrDefault('theme:darkMode')) || + false; + + // At the very least, the schema should define a default theme; the '' will be unreachable + const themeVersion = + (settings.user?.['theme:version']?.userValue ?? + uiSettings.getOverrideOrDefault('theme:version')) || + ''; const brandingAssignment = await this.assignBrandingConfig( + darkMode, opensearchDashboardsConfig as OpenSearchDashboardsConfigType ); @@ -104,9 +116,10 @@ export class RenderingService { strictCsp: http.csp.strict, uiPublicUrl, bootstrapScriptUrl: `${basePath}/bootstrap.js`, - startupScriptUrl: `${basePath}/startup.js`, i18n: i18n.translate, locale: i18n.getLocale(), + darkMode, + themeVersion, injectedMetadata: { version: env.packageInfo.version, buildNumber: env.packageInfo.buildNum, @@ -131,6 +144,7 @@ export class RenderingService { uiSettings: settings, }, branding: { + darkMode, assetFolderUrl: `${uiPublicUrl}/default_branding`, logo: { defaultUrl: brandingAssignment.logoDefault, @@ -184,16 +198,20 @@ export class RenderingService { /** * Assign values for branding related configurations based on branding validation - * by calling checkBrandingValid(). If URL is valid, pass in + * by calling checkBrandingValid(). For dark mode URLs, add additional validation + * to see if there is a valid default mode URL exist first. If URL is valid, pass in * the actual URL; if not, pass in undefined. * + * @param {boolean} darkMode * @param {Readonly} opensearchDashboardsConfig * @returns {BrandingAssignment} valid URLs or undefined assigned for each branding configs */ private assignBrandingConfig = async ( + darkMode: boolean, opensearchDashboardsConfig: Readonly ): Promise => { const brandingValidation: BrandingValidation = await this.checkBrandingValid( + darkMode, opensearchDashboardsConfig ); const branding = opensearchDashboardsConfig.branding; @@ -212,18 +230,47 @@ export class RenderingService { : undefined; // assign dark mode URLs based on brandingValidation function result - const logoDarkmode = brandingValidation.isLogoDarkmodeValid + let logoDarkmode = brandingValidation.isLogoDarkmodeValid ? branding.logo.darkModeUrl : undefined; - const markDarkmode = brandingValidation.isMarkDarkmodeValid + let markDarkmode = brandingValidation.isMarkDarkmodeValid ? branding.mark.darkModeUrl : undefined; - const loadingLogoDarkmode = brandingValidation.isLoadingLogoDarkmodeValid + let loadingLogoDarkmode = brandingValidation.isLoadingLogoDarkmodeValid ? branding.loadingLogo.darkModeUrl : undefined; + /** + * For dark mode URLs, we added another validation: + * user can only provide a dark mode URL after providing a valid default mode URL, + * If user provides a valid dark mode URL but fails to provide a valid default mode URL, + * return undefined for the dark mode URL + */ + if (logoDarkmode && !logoDefault) { + this.logger + .get('branding') + .error('Must provide a valid logo default mode URL before providing a logo dark mode URL'); + logoDarkmode = undefined; + } + + if (markDarkmode && !markDefault) { + this.logger + .get('branding') + .error('Must provide a valid mark default mode URL before providing a mark dark mode URL'); + markDarkmode = undefined; + } + + if (loadingLogoDarkmode && !loadingLogoDefault) { + this.logger + .get('branding') + .error( + 'Must provide a valid loading logo default mode URL before providing a loading logo dark mode URL' + ); + loadingLogoDarkmode = undefined; + } + // assign favicon based on brandingValidation function result const favicon = brandingValidation.isFaviconValid ? branding.faviconUrl : undefined; @@ -255,30 +302,35 @@ export class RenderingService { * user inputs valid or invalid URLs by calling isUrlValid() function. Also * check if title is valid by calling isTitleValid() function. * + * @param {boolean} darkMode * @param {Readonly} opensearchDashboardsConfig * @returns {BrandingValidation} indicate valid/invalid URL for each branding config */ private checkBrandingValid = async ( + darkMode: boolean, opensearchDashboardsConfig: Readonly ): Promise => { const branding = opensearchDashboardsConfig.branding; const isLogoDefaultValid = await this.isUrlValid(branding.logo.defaultUrl, 'logo default'); - const isLogoDarkmodeValid = await this.isUrlValid(branding.logo.darkModeUrl, 'logo darkMode'); + const isLogoDarkmodeValid = darkMode + ? await this.isUrlValid(branding.logo.darkModeUrl, 'logo darkMode') + : false; const isMarkDefaultValid = await this.isUrlValid(branding.mark.defaultUrl, 'mark default'); - const isMarkDarkmodeValid = await this.isUrlValid(branding.mark.darkModeUrl, 'mark darkMode'); + const isMarkDarkmodeValid = darkMode + ? await this.isUrlValid(branding.mark.darkModeUrl, 'mark darkMode') + : false; const isLoadingLogoDefaultValid = await this.isUrlValid( branding.loadingLogo.defaultUrl, 'loadingLogo default' ); - const isLoadingLogoDarkmodeValid = await this.isUrlValid( - branding.loadingLogo.darkModeUrl, - 'loadingLogo darkMode' - ); + const isLoadingLogoDarkmodeValid = darkMode + ? await this.isUrlValid(branding.loadingLogo.darkModeUrl, 'loadingLogo darkMode') + : false; const isFaviconValid = await this.isUrlValid(branding.faviconUrl, 'favicon'); diff --git a/src/core/server/rendering/types.ts b/src/core/server/rendering/types.ts index ecf3e2a9674e..45821c2b8228 100644 --- a/src/core/server/rendering/types.ts +++ b/src/core/server/rendering/types.ts @@ -43,9 +43,10 @@ export interface RenderingMetadata { strictCsp: ICspConfig['strict']; uiPublicUrl: string; bootstrapScriptUrl: string; - startupScriptUrl: string; i18n: typeof i18n.translate; locale: string; + darkMode: boolean; + themeVersion: string; injectedMetadata: { version: string; buildNumber: number; diff --git a/src/core/server/rendering/views/__snapshots__/template.test.tsx.snap b/src/core/server/rendering/views/__snapshots__/template.test.tsx.snap index 1204752a6469..36d073992ec8 100644 --- a/src/core/server/rendering/views/__snapshots__/template.test.tsx.snap +++ b/src/core/server/rendering/views/__snapshots__/template.test.tsx.snap @@ -50,6 +50,10 @@ Array [ content="[object Object]/ui/favicons/browserconfig.xml" name="msapplication-config" />, + , null, , null, - , -