Replace Borealis isServerless conditional with YML config (#206690)

Resolves https://github.com/elastic/eui-private/issues/171
Resolves https://github.com/elastic/eui-private/issues/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](https://github.com/elastic/kibana/pull/199748#discussion_r1840402343).
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 <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Tomasz Kajtoch 2025-01-20 16:38:50 +01:00 committed by GitHub
parent 354385213f
commit 8d2a43a0ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 102 additions and 99 deletions

View file

@ -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"

View file

@ -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 => {

View file

@ -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[];

View file

@ -111,7 +111,7 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -423,14 +423,14 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -812,21 +812,21 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -1126,21 +1126,21 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -1445,21 +1445,21 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -1753,21 +1753,21 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
Object {
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -2002,7 +2002,7 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -2142,7 +2142,7 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -2287,7 +2287,7 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],
@ -2421,7 +2421,7 @@ Array [
"type": "return",
"value": Object {
"darkMode": false,
"name": "amsterdam",
"name": "borealis",
},
},
],

View file

@ -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,7 +325,7 @@ 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({
@ -329,7 +333,7 @@ describe('bootstrapRenderer', () => {
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,
});
});

View file

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

View file

@ -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({

View file

@ -7,8 +7,8 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/
export const DEFAULT_THEME_NAME = 'amsterdam';
export const SUPPORTED_THEME_NAMES = ['amsterdam', 'borealis'];
export const DEFAULT_THEME_NAME = 'borealis';
export const SUPPORTED_THEME_NAMES = ['amsterdam', 'borealis'] as const;
export type ThemeName = (typeof SUPPORTED_THEME_NAMES)[number];
@ -37,7 +37,7 @@ export type ThemeTags = readonly ThemeTag[];
*/
export const DEFAULT_THEME_TAGS: ThemeTags = SUPPORTED_THEME_TAGS;
export const FALLBACK_THEME_TAG: ThemeTag = 'v8light';
export const FALLBACK_THEME_TAG: ThemeTag = 'borealislight';
const isValidTag = (tag: unknown) =>
SUPPORTED_THEME_TAGS.includes(tag as (typeof SUPPORTED_THEME_TAGS)[number]);

View file

@ -17,7 +17,6 @@ import { getStateSettings } from './state';
import { getAnnouncementsSettings } from './announcements';
const defaultOptions: GetCoreSettingsOptions = {
isServerless: false,
isDist: true,
isThemeSwitcherEnabled: undefined,
};

View file

@ -7,7 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/
import type { UiSettingsParams } from '@kbn/core-ui-settings-common';
import type { ThemeName, UiSettingsParams } from '@kbn/core-ui-settings-common';
import { getAccessibilitySettings } from './accessibility';
import { getDateFormatSettings } from './date_formats';
import { getMiscUiSettings } from './misc';
@ -17,9 +17,9 @@ import { getStateSettings } from './state';
import { getAnnouncementsSettings } from './announcements';
export interface GetCoreSettingsOptions {
isServerless: boolean;
isDist: boolean;
isThemeSwitcherEnabled: boolean | undefined;
defaultTheme?: ThemeName;
}
export const getCoreSettings = (

View file

@ -11,7 +11,6 @@ import type { UiSettingsParams } from '@kbn/core-ui-settings-common';
import { getThemeSettings, type GetThemeSettingsOptions } from './theme';
const defaultOptions: GetThemeSettingsOptions = {
isServerless: false,
isDist: true,
isThemeSwitcherEnabled: undefined,
};
@ -42,6 +41,8 @@ describe('theme settings', () => {
const validate = getValidationFn(themeSettings['theme:name']);
it('should only accept expected values', () => {
// TODO: Remove amsterdam theme
// https://github.com/elastic/eui-private/issues/170
expect(() => validate('amsterdam')).not.toThrow();
expect(() => validate('borealis')).not.toThrow();
@ -50,25 +51,24 @@ describe('theme settings', () => {
});
describe('readonly', () => {
it('should be readonly when `isServerless = true`', () => {
it('should not be editable when `isThemeSwitcherEnabled` is falsy', () => {
expect(getThemeSettings(defaultOptions)['theme:name'].readonly).toBe(true);
expect(
getThemeSettings({ ...defaultOptions, isServerless: true })['theme:name'].readonly
getThemeSettings({
...defaultOptions,
isThemeSwitcherEnabled: false,
})['theme:name'].readonly
).toBe(true);
expect(
getThemeSettings({ ...defaultOptions, isServerless: false })['theme:name'].readonly
).toBe(false);
});
it('should be editable when `isThemeSwitcherEnabled = true`', () => {
expect(
getThemeSettings({ ...defaultOptions, isServerless: true, isThemeSwitcherEnabled: true })[
'theme:name'
].readonly
getThemeSettings({ ...defaultOptions, isThemeSwitcherEnabled: true })['theme:name']
.readonly
).toBe(false);
expect(
getThemeSettings({
...defaultOptions,
isServerless: false,
isThemeSwitcherEnabled: true,
})['theme:name'].readonly
).toBe(false);
@ -76,16 +76,17 @@ describe('theme settings', () => {
});
describe('value', () => {
it('should default to `amsterdam` when `isServerless = true`', () => {
expect(
getThemeSettings({ ...defaultOptions, isServerless: true })['theme:name'].value
).toBe('amsterdam');
it('should default to `borealis`', () => {
expect(getThemeSettings(defaultOptions)['theme:name'].value).toBe('borealis');
});
it('should default to `borealis` when `isServerless = false`', () => {
it('should use the `defaultTheme` value when defined', () => {
expect(
getThemeSettings({ ...defaultOptions, isServerless: false })['theme:name'].value
).toBe('borealis');
getThemeSettings({
...defaultOptions,
defaultTheme: 'amsterdam',
})['theme:name'].value
).toBe('amsterdam');
});
});
});

View file

@ -9,6 +9,7 @@
import { schema } from '@kbn/config-schema';
import { i18n } from '@kbn/i18n';
import type { Writable } from '@kbn/utility-types';
import type { ThemeVersion } from '@kbn/ui-shared-deps-npm';
import {
type UiSettingsParams,
@ -17,18 +18,17 @@ import {
SUPPORTED_THEME_NAMES,
DEFAULT_THEME_NAME,
} from '@kbn/core-ui-settings-common';
import { defaultThemeSchema } from '../ui_settings_config';
interface ThemeInfo {
defaultDarkMode: boolean;
defaultThemeName: ThemeName;
}
const getThemeInfo = ({ isDist, isServerless }: GetThemeSettingsOptions): ThemeInfo => {
const getThemeInfo = ({ isDist }: GetThemeSettingsOptions): ThemeInfo => {
const themeTags = parseThemeTags(process.env.KBN_OPTIMIZER_THEMES);
const themeInfo: ThemeInfo = {
defaultDarkMode: false,
defaultThemeName: DEFAULT_THEME_NAME,
};
if (!isDist) {
@ -36,30 +36,20 @@ const getThemeInfo = ({ isDist, isServerless }: GetThemeSettingsOptions): ThemeI
themeInfo.defaultDarkMode = themeTags[0]?.endsWith('dark') || false;
}
if (!isServerless) {
// Default to Borealis theme in non-serverless
themeInfo.defaultThemeName = 'borealis';
}
return themeInfo;
};
export interface GetThemeSettingsOptions {
isServerless: boolean;
isDist: boolean;
isThemeSwitcherEnabled: boolean | undefined;
defaultTheme?: ThemeName;
}
export const getThemeSettings = (
options: GetThemeSettingsOptions
): Record<string, UiSettingsParams> => {
const { defaultDarkMode, defaultThemeName } = getThemeInfo(options);
// Make `theme:name` readonly in serverless unless the theme switcher is enabled
let isThemeNameReadonly = options.isServerless;
if (options.isThemeSwitcherEnabled !== undefined) {
isThemeNameReadonly = !options.isThemeSwitcherEnabled;
}
const { defaultDarkMode } = getThemeInfo(options);
const defaultTheme = options.defaultTheme ?? DEFAULT_THEME_NAME;
return {
'theme:darkMode': {
@ -122,7 +112,8 @@ export const getThemeSettings = (
defaultMessage: 'Theme',
}),
type: 'select',
options: SUPPORTED_THEME_NAMES,
// Cast to a mutable array to satisfy the `UiSettingsParams.options` type
options: SUPPORTED_THEME_NAMES as Writable<typeof SUPPORTED_THEME_NAMES>,
optionLabels: {
amsterdam: i18n.translate('core.ui_settings.params.themeName.options.amsterdam', {
defaultMessage: 'Amsterdam',
@ -131,15 +122,10 @@ export const getThemeSettings = (
defaultMessage: 'Borealis',
}),
},
value: defaultThemeName,
readonly: isThemeNameReadonly,
value: defaultTheme,
readonly: !options.isThemeSwitcherEnabled,
requiresPageReload: true,
schema: schema.oneOf([
schema.literal('amsterdam'),
schema.literal('borealis'),
// Allow experimental themes
schema.string(),
]),
schema: defaultThemeSchema,
},
};
};

View file

@ -9,6 +9,7 @@
import { schema, TypeOf, offeringBasedSchema } from '@kbn/config-schema';
import type { ServiceConfigDescriptor } from '@kbn/core-base-server-internal';
import { DEFAULT_THEME_NAME } from '@kbn/core-ui-settings-common';
import { ConfigDeprecationProvider } from '@kbn/config';
const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) => [
@ -16,12 +17,22 @@ const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) =>
renameFromRoot('server.defaultRoute', 'uiSettings.overrides.defaultRoute', { level: 'warning' }),
];
export const defaultThemeSchema = schema.oneOf([
// TODO: Remove amsterdam theme
// https://github.com/elastic/eui-private/issues/170
schema.literal('amsterdam'),
schema.literal('borealis'),
// Allow experimental themes
schema.string(),
]);
const configSchema = schema.object({
overrides: schema.object({}, { unknowns: 'allow' }),
publicApiEnabled: offeringBasedSchema({ serverless: schema.boolean({ defaultValue: false }) }),
experimental: schema.maybe(
schema.object({
themeSwitcherEnabled: schema.maybe(schema.boolean({ defaultValue: false })),
defaultTheme: schema.maybe(schema.string({ defaultValue: DEFAULT_THEME_NAME })),
})
),
});

View file

@ -18,6 +18,7 @@ import type { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-ser
import type { InternalSavedObjectsServiceSetup } from '@kbn/core-saved-objects-server-internal';
import type {
ReadonlyModeType,
ThemeName,
UiSettingsParams,
UiSettingsScope,
} from '@kbn/core-ui-settings-common';
@ -53,7 +54,6 @@ export class UiSettingsService
private readonly config$: Observable<UiSettingsConfigType>;
private readonly isDist: boolean;
private readonly isDev: boolean;
private readonly isServerless: boolean;
private readonly uiSettingsDefaults = new Map<string, UiSettingsParams>();
private readonly uiSettingsGlobalDefaults = new Map<string, UiSettingsParams>();
private overrides: Record<string, any> = {};
@ -64,7 +64,6 @@ export class UiSettingsService
this.isDist = coreContext.env.packageInfo.dist;
this.config$ = coreContext.configService.atPath<UiSettingsConfigType>(uiConfigDefinition.path);
this.isDev = coreContext.env.mode.dev;
this.isServerless = coreContext.env.packageInfo.buildFlavor === 'serverless';
}
public async preboot(): Promise<InternalUiSettingsServicePreboot> {
@ -76,8 +75,8 @@ export class UiSettingsService
this.register(
getCoreSettings({
isDist: this.isDist,
isServerless: this.isServerless,
isThemeSwitcherEnabled: experimental?.themeSwitcherEnabled,
defaultTheme: experimental?.defaultTheme as ThemeName,
})
);

View file

@ -32,6 +32,7 @@
"@kbn/core-logging-server-mocks",
"@kbn/core-saved-objects-api-server-internal",
"@kbn/core-saved-objects-common",
"@kbn/utility-types",
],
"exclude": [
"target/**/*",

View file

@ -18,7 +18,7 @@ import { ThemeService } from './theme';
import { coreMock } from '@kbn/core/public/mocks';
const createTheme$Mock = (mode: boolean) => {
return from([{ darkMode: mode, name: 'amsterdam' }]);
return from([{ darkMode: mode, name: 'borealis' }]);
};
const { theme: setUpMockTheme } = coreMock.createSetup();
@ -36,7 +36,7 @@ describe('ThemeService', () => {
expect(await themeService.darkModeEnabled$.pipe(take(1)).toPromise()).toStrictEqual({
darkMode: false,
name: 'amsterdam',
name: 'borealis',
});
});
@ -47,7 +47,7 @@ describe('ThemeService', () => {
expect(await themeService.darkModeEnabled$.pipe(take(1)).toPromise()).toStrictEqual({
darkMode: true,
name: 'amsterdam',
name: 'borealis',
});
});
});