Replace Borealis isServerless conditional with YML config (elastic#…

…206690) Resolves elastic/eui-private#171 Resolves elastic/eui-private#177 ## Summary This PR addresses a prior PR review [comment](https://github.com/elastic/kibana/pull/203840/files#diff-bb850523655bac7adb30995553acabae9705435fa51e5b8bf13c483152db694a) by removing `isServerless` from the logic determining what theme should be used at runtime with a simple YML configuration setting instead. I added a non-public `uiSettings.experimental.defaultTheme` config property that defaults to `borealis` and is set to `amsterdam` in `serverless.yml`. Since the default theme is now (and should be) set to Borealis, I also updated `DEFAULT_THEME_NAME` and `FALLBACK_THEME_NAME` to reflect that. This doesn't have any impact on Serverless; it will keep using Amsterdam. Additionally, while making these changes, I wanted to simultaneously improve types and address earlier PR [comment](elastic#199748 (comment)). Now `SUPPORTED_THEME_NAMES` array is declared as `const` making the `ThemeName` type strict instead of resolving a generic `string` type. Usages were updated to use `ThemeName` instead of `string`, too. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
jcger · Jan 20, 2025 · 8d2a43a · 8d2a43a
1 parent 3543852
commit 8d2a43a
Show file tree

Hide file tree

Showing 16 changed files with 102 additions and 99 deletions.
diff --git a/config/serverless.yml b/config/serverless.yml
@@ -234,3 +234,6 @@ discover.enableUiSettingsValidations: true
 xpack.dataUsage.enabled: true
 # This feature is disabled in Serverless until fully tested within a Serverless environment
 xpack.dataUsage.enableExperimental: ['dataUsageDisabled']
+
+# Ensure Serverless is using the Amsterdam theme
+uiSettings.experimental.defaultTheme: "amsterdam"
diff --git a/packages/core/theme/core-theme-browser-mocks/src/theme_service.mock.ts b/packages/core/theme/core-theme-browser-mocks/src/theme_service.mock.ts
@@ -14,7 +14,7 @@ import type { ThemeService } from '@kbn/core-theme-browser-internal';
 
 const mockTheme: CoreTheme = {
   darkMode: false,
-  name: 'amsterdam',
+  name: 'borealis',
 };
 
 const createThemeMock = (): CoreTheme => {

diff --git a/src/core/packages/injected-metadata/common-internal/src/types.ts b/src/core/packages/injected-metadata/common-internal/src/types.ts
@@ -11,7 +11,7 @@ import type { PluginName, DiscoveredPlugin } from '@kbn/core-base-common';
 import type { ThemeVersion } from '@kbn/ui-shared-deps-npm';
 import type { EnvironmentMode, PackageInfo } from '@kbn/config';
 import type { CustomBranding } from '@kbn/core-custom-branding-common';
-import type { DarkModeValue } from '@kbn/core-ui-settings-common';
+import type { DarkModeValue, ThemeName } from '@kbn/core-ui-settings-common';
 import type { BrowserLoggingConfig } from '@kbn/core-logging-common-internal';
 
 /** @internal */
@@ -41,7 +41,7 @@ export interface InjectedMetadataExternalUrlPolicy {
 /** @internal */
 export interface InjectedMetadataTheme {
   darkMode: DarkModeValue;
-  name: string;
+  name: ThemeName;
   version: ThemeVersion;
   stylesheetPaths: {
     default: string[];

diff --git a/...re/packages/overlays/browser-internal/src/modal/__snapshots__/modal_service.test.tsx.snap b/...re/packages/overlays/browser-internal/src/modal/__snapshots__/modal_service.test.tsx.snap
diff --git a/src/core/packages/rendering/server-internal/src/bootstrap/bootstrap_renderer.test.ts b/src/core/packages/rendering/server-internal/src/bootstrap/bootstrap_renderer.test.ts
@@ -35,13 +35,13 @@ const createPackageInfo = (parts: Partial<PackageInfo> = {}): PackageInfo => ({
 });
 
 const getClientGetMockImplementation =
-  ({ darkMode, name }: { darkMode?: boolean; name?: string } = {}) =>
+  ({ darkMode, name }: { darkMode?: boolean | string; name?: string } = {}) =>
   (key: string) => {
     switch (key) {
       case 'theme:darkMode':
         return Promise.resolve(darkMode ?? false);
       case 'theme:name':
-        return Promise.resolve(name ?? 'amsterdam');
+        return Promise.resolve(name ?? 'borealis');
     }
     return Promise.resolve();
   };
@@ -67,7 +67,7 @@ describe('bootstrapRenderer', () => {
     packageInfo = createPackageInfo();
     userSettingsService = userSettingsServiceMock.createSetupContract();
 
-    getThemeTagMock.mockReturnValue('v8light');
+    getThemeTagMock.mockReturnValue('borealislight');
     getPluginsBundlePathsMock.mockReturnValue(new Map());
     renderTemplateMock.mockReturnValue('__rendered__');
     getJsDependencyPathsMock.mockReturnValue([]);
@@ -124,7 +124,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: true,
       });
     });
@@ -141,7 +141,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: false,
       });
     });
@@ -167,7 +167,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: true,
       });
     });
@@ -192,7 +192,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: false,
       });
     });
@@ -217,7 +217,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: false,
       });
     });
@@ -247,7 +247,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: true,
       });
     });
@@ -290,13 +290,17 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: true,
       });
     });
 
     it('calls getThemeTag with the correct parameters when darkMode is `system`', async () => {
-      uiSettingsClient.get.mockResolvedValue('system');
+      uiSettingsClient.get.mockImplementation(
+        getClientGetMockImplementation({
+          darkMode: 'system',
+        })
+      );
 
       const request = httpServerMock.createKibanaRequest();
 
@@ -307,7 +311,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'system',
+        name: 'borealis',
         darkMode: false,
       });
     });
@@ -321,15 +325,15 @@ describe('bootstrapRenderer', () => {
       });
     });
 
-    it('does not call uiSettingsClient.get', async () => {
+    it('does not call uiSettingsClient.get with `theme:darkMode`', async () => {
       const request = httpServerMock.createKibanaRequest();
 
       await renderer({
         request,
         uiSettingsClient,
       });
 
-      expect(uiSettingsClient.get).not.toHaveBeenCalled();
+      expect(uiSettingsClient.get).not.toHaveBeenCalledWith('theme:darkMode');
     });
 
     it('calls getThemeTag with the default parameters', async () => {
@@ -367,7 +371,7 @@ describe('bootstrapRenderer', () => {
 
       expect(getThemeTagMock).toHaveBeenCalledTimes(1);
       expect(getThemeTagMock).toHaveBeenCalledWith({
-        name: 'v8',
+        name: 'borealis',
         darkMode: false,
       });
     });

diff --git a/src/core/packages/rendering/server-internal/src/bootstrap/bootstrap_renderer.ts b/src/core/packages/rendering/server-internal/src/bootstrap/bootstrap_renderer.ts
@@ -12,8 +12,10 @@ import { PackageInfo } from '@kbn/config';
 import type { KibanaRequest, HttpAuth } from '@kbn/core-http-server';
 import {
   type DarkModeValue,
+  type ThemeName,
   DEFAULT_THEME_NAME,
   parseDarkModeValue,
+  parseThemeNameValue,
 } from '@kbn/core-ui-settings-common';
 import type { IUiSettingsClient } from '@kbn/core-ui-settings-server';
 import type { UiPlugins } from '@kbn/core-plugins-base-server-internal';
@@ -62,13 +64,11 @@ export const bootstrapRendererFactory: BootstrapRendererFactory = ({
 
   return async function bootstrapRenderer({ uiSettingsClient, request, isAnonymousPage = false }) {
     let darkMode: DarkModeValue = false;
-    let themeName: string = DEFAULT_THEME_NAME;
-
-    if (packageInfo.buildFlavor !== 'serverless') {
-      themeName = 'borealis';
-    }
+    let themeName: ThemeName = DEFAULT_THEME_NAME;
 
     try {
+      themeName = parseThemeNameValue(await uiSettingsClient.get('theme:name'));
+
       const authenticated = isAuthenticated(request);
 
       if (authenticated) {
@@ -79,8 +79,6 @@ export const bootstrapRendererFactory: BootstrapRendererFactory = ({
         } else {
           darkMode = parseDarkModeValue(await uiSettingsClient.get('theme:darkMode'));
         }
-
-        themeName = await uiSettingsClient.get('theme:name');
       }
     } catch (e) {
       // just use the default values in case of connectivity issues with ES

diff --git a/src/core/packages/rendering/server-internal/src/rendering_service.tsx b/src/core/packages/rendering/server-internal/src/rendering_service.tsx
@@ -21,6 +21,7 @@ import type { UiPlugins } from '@kbn/core-plugins-base-server-internal';
 import type { CustomBranding } from '@kbn/core-custom-branding-common';
 import {
   type DarkModeValue,
+  type ThemeName,
   parseDarkModeValue,
   parseThemeNameValue,
   type UiSettingsParams,
@@ -212,7 +213,7 @@ export class RenderingService {
       darkMode = getSettingValue<DarkModeValue>('theme:darkMode', settings, parseDarkModeValue);
     }
 
-    const themeName = getSettingValue<string>('theme:name', settings, parseThemeNameValue);
+    const themeName = getSettingValue<ThemeName>('theme:name', settings, parseThemeNameValue);
 
     const themeStylesheetPaths = (mode: boolean) =>
       getThemeStylesheetPaths({