Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Amsterdam an opt-in theme #212787

Merged
merged 5 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ describe('getOptimizerCacheKey()', () => {
"reactVersion": "17",
"repoRoot": <absolute path>,
"themeTags": Array [
"v8light",
"v8dark",
"borealislight",
"borealisdark",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jest.doMock('./render_template', () => ({
}));

export const getThemeTagMock = jest.fn();
jest.doMock('./get_theme_tag', () => ({
jest.doMock('../theme', () => ({
getThemeTag: getThemeTagMock,
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,7 @@ describe('bootstrapRenderer', () => {

expect(getThemeTagMock).toHaveBeenLastCalledWith(
expect.objectContaining({
// 'amsterdam' is mapped to 'v8' for backwards compatibility
name: 'v8',
name: 'amsterdam',
})
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import {
import type { IUiSettingsClient } from '@kbn/core-ui-settings-server';
import type { UiPlugins } from '@kbn/core-plugins-base-server-internal';
import { InternalUserSettingsServiceSetup } from '@kbn/core-user-settings-server-internal';
import { getThemeTag } from '../theme';
import { getPluginsBundlePaths } from './get_plugin_bundle_paths';
import { getJsDependencyPaths } from './get_js_dependency_paths';
import { getThemeTag } from './get_theme_tag';
import { renderTemplate } from './render_template';
import { getBundlesHref } from '../render_utils';

Expand Down Expand Up @@ -89,7 +89,7 @@ export const bootstrapRendererFactory: BootstrapRendererFactory = ({
}

const themeTag = getThemeTag({
name: !themeName || themeName === 'amsterdam' ? 'v8' : themeName,
name: themeName,
darkMode,
});
const bundlesHref = getBundlesHref(baseHref);
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,10 @@ jest.doMock('./get_apm_config', () => {
getApmConfig: getApmConfigMock,
};
});

export const getIsThemeBundledMock = jest.fn();
jest.doMock('./theme', () => {
return {
isThemeBundled: getIsThemeBundledMock,
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
getScriptPathsMock,
getBrowserLoggingConfigMock,
getApmConfigMock,
getIsThemeBundledMock,
} from './rendering_service.test.mocks';

import { load } from 'cheerio';
Expand Down Expand Up @@ -615,12 +616,41 @@ describe('RenderingService', () => {
globalClient: uiSettingsServiceMock.createClient(),
};

getIsThemeBundledMock.mockImplementation((name) => ['borealis', 'amsterdam'].includes(name));

let renderResult = await render(createKibanaRequest(), uiSettings);
expect(getIsThemeBundledMock).toHaveBeenCalledWith('borealis');
expect(renderResult).toContain(',&quot;name&quot;:&quot;borealis&quot;');

themeName$.next('amsterdam');
renderResult = await render(createKibanaRequest(), uiSettings);
expect(renderResult).toContain(',&quot;name&quot;:&quot;amsterdam&quot;');
});

it('falls back to the default theme if theme is not bundled', async () => {
// setup and render added to assert the current theme name
const { render } = await service.setup(mockRenderingSetupDeps);
const themeName$ = new BehaviorSubject<ThemeName>('unknown' as any);
const getStringValue$ = jest
.fn()
.mockImplementation((_, fallback) => themeName$.asObservable());
service.start({
...mockRenderingStartDeps,
featureFlags: {
...mockRenderingStartDeps.featureFlags,
getStringValue$,
},
});

const uiSettings = {
client: uiSettingsServiceMock.createClient(),
globalClient: uiSettingsServiceMock.createClient(),
};

getIsThemeBundledMock.mockReturnValue(false);

const renderResult = await render(createKibanaRequest(), uiSettings);
expect(renderResult).toContain(',&quot;name&quot;:&quot;borealis&quot;');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
import { filterUiPlugins } from './filter_ui_plugins';
import { getApmConfig } from './get_apm_config';
import type { InternalRenderingRequestHandlerContext } from './internal_types';
import { isThemeBundled } from './theme';

type RenderOptions =
| RenderingSetupDeps
Expand Down Expand Up @@ -134,7 +135,16 @@ export class RenderingService {
featureFlags
.getStringValue$<ThemeName>(DEFAULT_THEME_NAME_FEATURE_FLAG, DEFAULT_THEME_NAME)
// Parse the input feature flag value to ensure it's of type ThemeName
.pipe(map((value) => parseThemeNameValue(value)))
// and that it's bundled with this build of Kibana
.pipe(
map((themeName) => {
if (isThemeBundled(themeName)) {
return parseThemeNameValue(themeName);
}

return DEFAULT_THEME_NAME;
})
)
.subscribe(this.themeName$);
}

Expand Down
82 changes: 82 additions & 0 deletions src/core/packages/rendering/server-internal/src/theme.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { getThemeTag, isThemeBundled } from './theme';

describe('getThemeTag', () => {
it('returns the correct value for name:amsterdam and darkMode:false', () => {
expect(
getThemeTag({
name: 'v8',
darkMode: false,
})
).toEqual('v8light');
});

it('returns the correct value for name:amsterdam and darkMode:true', () => {
expect(
getThemeTag({
name: 'v8',
darkMode: true,
})
).toEqual('v8dark');
});

it('returns the correct value for other other theme names and darkMode:false', () => {
expect(
getThemeTag({
name: 'borealis',
darkMode: false,
})
).toEqual('borealislight');
});

it('returns the correct value for other other theme names and darkMode:true', () => {
expect(
getThemeTag({
name: 'borealis',
darkMode: true,
})
).toEqual('borealisdark');
});
});

describe('isThemeBundled', () => {
let originalKbnOptimizerThemes: any;

beforeAll(() => {
originalKbnOptimizerThemes = process.env.KBN_OPTIMIZER_THEMES;
});

afterAll(() => {
process.env.KBN_OPTIMIZER_THEMES = originalKbnOptimizerThemes;
});

it('returns true when both light and dark mode theme tags are included in KBN_OPTIMIZER_THEMES', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v8light,v8dark,borealislight,borealisdark';
expect(isThemeBundled('amsterdam')).toEqual(true);
expect(isThemeBundled('borealis')).toEqual(true);
});

it('returns false when either theme tag is missing in KBN_OPTIMIZER_THEMES for given theme name', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v8light,borealisdark,borealisdark';
expect(isThemeBundled('amsterdam')).toEqual(false);
expect(isThemeBundled('borealis')).toEqual(false);
});

it('uses default themes when KBN_OPTIMIZER_THEMES is not set', () => {
delete process.env.KBN_OPTIMIZER_THEMES;
expect(isThemeBundled('borealis')).toEqual(true);
expect(isThemeBundled('sometheme' as any)).toEqual(false);

process.env.KBN_OPTIMIZER_THEMES = '';
expect(isThemeBundled('borealis')).toEqual(true);
expect(isThemeBundled('sometheme' as any)).toEqual(false);
});
});
34 changes: 34 additions & 0 deletions src/core/packages/rendering/server-internal/src/theme.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { parseThemeTags, ThemeName, ThemeTag } from '@kbn/core-ui-settings-common';

export const getThemeTag = ({ name, darkMode }: { name: string; darkMode: boolean }) => {
// Amsterdam theme is called `v8` internally
// and should be kept this way for compatibility reasons.
if (name === 'amsterdam') {
name = 'v8';
}

return `${name}${darkMode ? 'dark' : 'light'}`;
};

/**
* Check whether the theme is bundled in the current kibana build.
* For a theme to be considered bundled both light and dark mode
* styles must be included.
*/
export const isThemeBundled = (name: ThemeName): boolean => {
const bundledThemeTags = parseThemeTags(process.env.KBN_OPTIMIZER_THEMES);

return (
bundledThemeTags.includes(getThemeTag({ name, darkMode: false }) as ThemeTag) &&
bundledThemeTags.includes(getThemeTag({ name, darkMode: true }) as ThemeTag)
);
};
2 changes: 1 addition & 1 deletion src/core/packages/ui-settings/common/src/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type ThemeTags = readonly ThemeTag[];
* An array of theme tags available in Kibana by default when not customized
* using KBN_OPTIMIZER_THEMES environment variable.
*/
export const DEFAULT_THEME_TAGS: ThemeTags = SUPPORTED_THEME_TAGS;
export const DEFAULT_THEME_TAGS: ThemeTags = ThemeBorealisTags;

export const FALLBACK_THEME_TAG: ThemeTag = 'borealislight';

Expand Down