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

[uiSettings/theme] restrict theme options based on KBN_OPTIMIZER_THEMES #94834

Merged
merged 8 commits into from
Mar 22, 2021
10 changes: 8 additions & 2 deletions src/core/server/ui_settings/settings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,20 @@ import { getNotificationsSettings } from './notifications';
import { getThemeSettings } from './theme';
import { getStateSettings } from './state';

export const getCoreSettings = (): Record<string, UiSettingsParams> => {
interface GetCoreSettingsOptions {
isDist?: boolean;
}
Comment on lines +18 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: why optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly because this is a temporary thing that I plan to remove once we not longer need to switch back to the v7 theme, so I wanted to avoid updating all the tests which use the getCoreSettings() fn.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if you're mostly curious about the property specifically, I didn't really want to push people to define the property if they somehow end up calling getCoreSettings() as I'm not really sure where it's called but the places where it matters the right options are passed.


export const getCoreSettings = (
options?: GetCoreSettingsOptions
): Record<string, UiSettingsParams> => {
return {
...getAccessibilitySettings(),
...getDateFormatSettings(),
...getMiscUiSettings(),
...getNavigationSettings(),
...getNotificationsSettings(),
...getThemeSettings(),
...getThemeSettings(options),
...getStateSettings(),
};
};
54 changes: 54 additions & 0 deletions src/core/server/ui_settings/settings/theme.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,57 @@ describe('theme settings', () => {
});
});
});

describe('process.env.KBN_OPTIMIZER_THEMES handling', () => {
it('provides valid options based on tags', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v7light,v8dark';
let settings = getThemeSettings({ isDist: false });
expect(settings['theme:version'].options).toEqual(['v7', 'v8']);

process.env.KBN_OPTIMIZER_THEMES = 'v8dark,v7light';
settings = getThemeSettings({ isDist: false });
expect(settings['theme:version'].options).toEqual(['v7', 'v8']);

process.env.KBN_OPTIMIZER_THEMES = 'v8dark,v7light,v7dark,v8light';
settings = getThemeSettings({ isDist: false });
expect(settings['theme:version'].options).toEqual(['v7', 'v8']);

process.env.KBN_OPTIMIZER_THEMES = '*';
settings = getThemeSettings({ isDist: false });
expect(settings['theme:version'].options).toEqual(['v7', 'v8']);

process.env.KBN_OPTIMIZER_THEMES = 'v7light';
settings = getThemeSettings({ isDist: false });
expect(settings['theme:version'].options).toEqual(['v7']);

process.env.KBN_OPTIMIZER_THEMES = 'v8light';
settings = getThemeSettings({ isDist: false });
expect(settings['theme:version'].options).toEqual(['v8']);
});

it('defaults to properties of first tag', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v8dark,v7light';
let settings = getThemeSettings({ isDist: false });
expect(settings['theme:darkMode'].value).toBe(true);
expect(settings['theme:version'].value).toBe('v8');

process.env.KBN_OPTIMIZER_THEMES = 'v7light,v8dark';
settings = getThemeSettings({ isDist: false });
expect(settings['theme:darkMode'].value).toBe(false);
expect(settings['theme:version'].value).toBe('v7');
});

it('ignores the value when isDist is undefined', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v7light';
const settings = getThemeSettings({ isDist: undefined });
expect(settings['theme:darkMode'].value).toBe(false);
expect(settings['theme:version'].options).toEqual(['v7', 'v8']);
});

it('ignores the value when isDist is true', () => {
process.env.KBN_OPTIMIZER_THEMES = 'v7light';
const settings = getThemeSettings({ isDist: true });
expect(settings['theme:darkMode'].value).toBe(false);
expect(settings['theme:version'].options).toEqual(['v7', 'v8']);
});
});
49 changes: 43 additions & 6 deletions src/core/server/ui_settings/settings/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,54 @@
* Side Public License, v 1.
*/

import { schema } from '@kbn/config-schema';
import { schema, Type } from '@kbn/config-schema';
import { i18n } from '@kbn/i18n';
import { UiSettingsParams } from '../../../types';

export const getThemeSettings = (): Record<string, UiSettingsParams> => {
function parseThemeTags() {
if (!process.env.KBN_OPTIMIZER_THEMES) {
return ['v8light', 'v8dark'];
}

if (process.env.KBN_OPTIMIZER_THEMES === '*') {
return ['v8light', 'v8dark', 'v7light', 'v7dark'];
}

return process.env.KBN_OPTIMIZER_THEMES.split(',').map((t) => t.trim());
}

function getThemeInfo(options: GetThemeSettingsOptions) {
if (options?.isDist ?? true) {
return {
defaultDarkMode: false,
defaultVersion: 'v8',
availableVersions: ['v7', 'v8'],
};
}

const themeTags = parseThemeTags();
return {
defaultDarkMode: themeTags[0].endsWith('dark'),
defaultVersion: themeTags[0].slice(0, 2),
availableVersions: ['v7', 'v8'].filter((v) => themeTags.some((t) => t.startsWith(v))),
};
}

interface GetThemeSettingsOptions {
isDist?: boolean;
}

export const getThemeSettings = (
options: GetThemeSettingsOptions = {}
): Record<string, UiSettingsParams> => {
const { availableVersions, defaultDarkMode, defaultVersion } = getThemeInfo(options);

return {
'theme:darkMode': {
name: i18n.translate('core.ui_settings.params.darkModeTitle', {
defaultMessage: 'Dark mode',
}),
value: false,
value: defaultDarkMode,
description: i18n.translate('core.ui_settings.params.darkModeText', {
defaultMessage: `Enable a dark mode for the Kibana UI. A page refresh is required for the setting to be applied.`,
}),
Expand All @@ -27,14 +64,14 @@ export const getThemeSettings = (): Record<string, UiSettingsParams> => {
name: i18n.translate('core.ui_settings.params.themeVersionTitle', {
defaultMessage: 'Theme version',
}),
value: 'v8',
value: defaultVersion,
type: 'select',
options: ['v7', 'v8'],
options: availableVersions,
description: i18n.translate('core.ui_settings.params.themeVersionText', {
defaultMessage: `Switch between the theme used for the current and next version of Kibana. A page refresh is required for the setting to be applied.`,
}),
requiresPageReload: true,
schema: schema.oneOf([schema.literal('v7'), schema.literal('v8')]),
schema: schema.oneOf(availableVersions.map((v) => schema.literal(v)) as [Type<string>]),
},
};
};
8 changes: 7 additions & 1 deletion src/core/server/ui_settings/ui_settings_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@ export class UiSettingsService
implements CoreService<InternalUiSettingsServiceSetup, InternalUiSettingsServiceStart> {
private readonly log: Logger;
private readonly config$: Observable<UiSettingsConfigType>;
private readonly isDist: boolean;
private readonly uiSettingsDefaults = new Map<string, UiSettingsParams>();
private overrides: Record<string, any> = {};

constructor(private readonly coreContext: CoreContext) {
this.log = coreContext.logger.get('ui-settings-service');
this.isDist = coreContext.env.packageInfo.dist;
this.config$ = coreContext.configService.atPath<UiSettingsConfigType>(uiConfigDefinition.path);
}

Expand All @@ -50,7 +52,11 @@ export class UiSettingsService

savedObjects.registerType(uiSettingsType);
registerRoutes(http.createRouter(''));
this.register(getCoreSettings());
this.register(
getCoreSettings({
isDist: this.isDist,
})
);

const config = await this.config$.pipe(first()).toPromise();
this.overrides = config.overrides;
Expand Down