Skip to content

Commit

Permalink
[9.0] Make Amsterdam an opt-in theme (#212787) (#213093)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `9.0`:
- [Make Amsterdam an opt-in theme
(#212787)](#212787)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Tomasz
Kajtoch","email":"tomasz.kajtoch@elastic.co"},"sourceCommit":{"committedDate":"2025-03-04T12:40:06Z","message":"Make
Amsterdam an opt-in theme (#212787)\n\n## Summary\n\nThis PR updates
`DEFAULT_THEME_TAGS` used to determine what theme tags\nare bundled in
Kibana by default to only include the Borealis theme,\nspecifically
`borealislight` and `borealisdark` theme tags. This change\nis expected
to decrease bundle sizes significantly and get back to\nbundling a
single theme, not two (4 → 2 theme tags).\n\nNow that Serverless, `9.0`,
and `main` all run with Borealis, there's no\nrisk in removing Amsterdam
from the bundle and decreasing Kibana bundle\nsizes.\n\nWe need to keep
the feature flag in code for the time being to easily\ntest future
Borealis iterations.\n\nAmsterdam will still be available as an opt-in
theme and is meant to be\nused locally when testing changes to be
backported to 8.x versions that\nuse Amsterdam. To do so, Kibana needs
to be started/built with\n`KBN_OPTIMIZER_THEMES` environment variable
set and the feature flag\noverridden in `kibana.dev.yml`.\n\n```yml\n#
config/kibana.dev.yml\nfeature_flags.overrides.coreRendering.defaultThemeName:
amsterdam\n```\n\n```shell\n# Run dev server with both borealis and
Amsterdam theme
tags\nKBN_OPTIMIZER_THEMES=\"borealislight,borealisdark,v8light,v8dark\"
yarn start\n```\n\n### Checklist\n\nCheck the PR satisfies following
conditions. \n\nReviewers should verify this PR satisfies this list as
well.\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios\n\n---------\n\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"4dd8de807ae8a9d1b00a2a99c98510f7a21382db","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","v9.1.0"],"title":"Make
Amsterdam an opt-in
theme","number":212787,"url":"https://github.com/elastic/kibana/pull/212787","mergeCommit":{"message":"Make
Amsterdam an opt-in theme (#212787)\n\n## Summary\n\nThis PR updates
`DEFAULT_THEME_TAGS` used to determine what theme tags\nare bundled in
Kibana by default to only include the Borealis theme,\nspecifically
`borealislight` and `borealisdark` theme tags. This change\nis expected
to decrease bundle sizes significantly and get back to\nbundling a
single theme, not two (4 → 2 theme tags).\n\nNow that Serverless, `9.0`,
and `main` all run with Borealis, there's no\nrisk in removing Amsterdam
from the bundle and decreasing Kibana bundle\nsizes.\n\nWe need to keep
the feature flag in code for the time being to easily\ntest future
Borealis iterations.\n\nAmsterdam will still be available as an opt-in
theme and is meant to be\nused locally when testing changes to be
backported to 8.x versions that\nuse Amsterdam. To do so, Kibana needs
to be started/built with\n`KBN_OPTIMIZER_THEMES` environment variable
set and the feature flag\noverridden in `kibana.dev.yml`.\n\n```yml\n#
config/kibana.dev.yml\nfeature_flags.overrides.coreRendering.defaultThemeName:
amsterdam\n```\n\n```shell\n# Run dev server with both borealis and
Amsterdam theme
tags\nKBN_OPTIMIZER_THEMES=\"borealislight,borealisdark,v8light,v8dark\"
yarn start\n```\n\n### Checklist\n\nCheck the PR satisfies following
conditions. \n\nReviewers should verify this PR satisfies this list as
well.\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios\n\n---------\n\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"4dd8de807ae8a9d1b00a2a99c98510f7a21382db"}},"sourceBranch":"main","suggestedTargetBranches":["9.0"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212787","number":212787,"mergeCommit":{"message":"Make
Amsterdam an opt-in theme (#212787)\n\n## Summary\n\nThis PR updates
`DEFAULT_THEME_TAGS` used to determine what theme tags\nare bundled in
Kibana by default to only include the Borealis theme,\nspecifically
`borealislight` and `borealisdark` theme tags. This change\nis expected
to decrease bundle sizes significantly and get back to\nbundling a
single theme, not two (4 → 2 theme tags).\n\nNow that Serverless, `9.0`,
and `main` all run with Borealis, there's no\nrisk in removing Amsterdam
from the bundle and decreasing Kibana bundle\nsizes.\n\nWe need to keep
the feature flag in code for the time being to easily\ntest future
Borealis iterations.\n\nAmsterdam will still be available as an opt-in
theme and is meant to be\nused locally when testing changes to be
backported to 8.x versions that\nuse Amsterdam. To do so, Kibana needs
to be started/built with\n`KBN_OPTIMIZER_THEMES` environment variable
set and the feature flag\noverridden in `kibana.dev.yml`.\n\n```yml\n#
config/kibana.dev.yml\nfeature_flags.overrides.coreRendering.defaultThemeName:
amsterdam\n```\n\n```shell\n# Run dev server with both borealis and
Amsterdam theme
tags\nKBN_OPTIMIZER_THEMES=\"borealislight,borealisdark,v8light,v8dark\"
yarn start\n```\n\n### Checklist\n\nCheck the PR satisfies following
conditions. \n\nReviewers should verify this PR satisfies this list as
well.\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios\n\n---------\n\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"4dd8de807ae8a9d1b00a2a99c98510f7a21382db"}}]}]
BACKPORT-->

Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>
  • Loading branch information
kibanamachine and tkajtoch authored Mar 4, 2025
1 parent 0cd3fa7 commit 6452ad4
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 54 deletions.
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

0 comments on commit 6452ad4

Please sign in to comment.