Skip to content

Commit

Permalink
Renames v7 theme, removes beta theme
Browse files Browse the repository at this point in the history
* Forces the v1 (the old v7) theme for all users, keeps the advanced
setting ui hidden.
* Keeps the theme switching logic for when the OUI cascadia theme is
ready.

Resolves opensearch-project#494

Signed-off-by: Tommy Markley <5437176+tmarkley@users.noreply.github.com>
  • Loading branch information
tmarkley committed Dec 12, 2022
1 parent 8732b1c commit aba71ca
Show file tree
Hide file tree
Showing 26 changed files with 74 additions and 120 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### 💥 Breaking Changes

- Renames v7 theme, removes beta theme

### Deprecations

### 🛡 Security
Expand Down
2 changes: 1 addition & 1 deletion DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ Do not use setters, they cause more problems than they can solve.
When writing a new component, create a sibling SASS file of the same name and import directly into the **top** of the JS/TS component file. Doing so ensures the styles are never separated or lost on import and allows for better modularization (smaller individual plugin asset footprint).
All SASS (.scss) files will automatically build with the [EUI](https://elastic.github.io/eui/#/guidelines/sass) & OpenSearch Dashboards invisibles (SASS variables, mixins, functions) from the [`globals_[theme].scss` file](src/core/public/core_app/styles/_globals_v7light.scss).
All SASS (.scss) files will automatically build with the OpenSearch Dashboards invisibles (SASS variables, mixins, functions) from the [`globals_[theme].scss` file](src/core/public/core_app/styles/_globals_v1light.scss).
While the styles for this component will only be loaded if the component exists on the page,
the styles **will** be global and so it is recommended to use a three letter prefix on your
Expand Down
10 changes: 5 additions & 5 deletions packages/osd-optimizer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ Any import in a bundle which resolves into another bundles "context" directory,

## Themes

SASS imports in bundles are automatically converted to CSS for one or more themes. In development we build the `v7light` and `v7dark` themes by default to improve build performance. When producing distributable bundles the default shifts to `*` so that the distributable bundles will include all themes, preventing the bundles from needing to be rebuilt when users change the active theme in OpenSearch Dashboards's advanced settings.
SASS imports in bundles are automatically converted to CSS for one or more themes. In development we build the `v1light` and `v1dark` themes by default to improve build performance. When producing distributable bundles the default shifts to `*` so that the distributable bundles will include all themes, preventing the bundles from needing to be rebuilt when users change the active theme in OpenSearch Dashboards's advanced settings.

To customize the themes that are built for development you can specify the `OSD_OPTIMIZER_THEMES` environment variable to one or more theme tags, or use `*` to build styles for all themes. Unfortunately building more than one theme significantly impacts build performance, so try to be strategic about which themes you build.

Currently supported theme tags: `v7light`, `v7dark`, `v8light`, `v8dark`
Currently supported theme tags: `v1light`, `v1dark`

Examples:
```sh
# start OpenSearch Dashboards with only a single theme
OSD_OPTIMIZER_THEMES=v7light yarn start
OSD_OPTIMIZER_THEMES=v1light yarn start

# start OpenSearch Dashboards with dark themes for version 7 and 8
OSD_OPTIMIZER_THEMES=v7dark,v8dark yarn start
# start OpenSearch Dashboards with dark themes for version 1
OSD_OPTIMIZER_THEMES=v1dark yarn start

# start OpenSearch Dashboards with all the themes
OSD_OPTIMIZER_THEMES=* yarn start
Expand Down

This file was deleted.

This file was deleted.

39 changes: 18 additions & 21 deletions packages/osd-optimizer/src/common/theme_tags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,70 +33,67 @@ import { parseThemeTags } from './theme_tags';
it('returns default tags when passed undefined', () => {
expect(parseThemeTags()).toMatchInlineSnapshot(`
Array [
"v7dark",
"v7light",
"v1dark",
"v1light",
]
`);
});

it('returns all tags when passed *', () => {
expect(parseThemeTags('*')).toMatchInlineSnapshot(`
Array [
"v7dark",
"v7light",
"v8dark",
"v8light",
"v1dark",
"v1light",
]
`);
});

it('returns specific tag when passed a single value', () => {
expect(parseThemeTags('v8light')).toMatchInlineSnapshot(`
expect(parseThemeTags('v1light')).toMatchInlineSnapshot(`
Array [
"v8light",
"v1light",
]
`);
});

it('returns specific tags when passed a comma separated list', () => {
expect(parseThemeTags('v8light, v7dark,v7light')).toMatchInlineSnapshot(`
expect(parseThemeTags('v1dark, v1light')).toMatchInlineSnapshot(`
Array [
"v7dark",
"v7light",
"v8light",
"v1dark",
"v1light",
]
`);
});

it('returns specific tags when passed an array', () => {
expect(parseThemeTags(['v8light', 'v7light'])).toMatchInlineSnapshot(`
expect(parseThemeTags(['v1dark', 'v1light'])).toMatchInlineSnapshot(`
Array [
"v7light",
"v8light",
"v1dark",
"v1light",
]
`);
});

it('throws when an invalid tag is in the array', () => {
expect(() => parseThemeTags(['v8light', 'v7light', 'bar'])).toThrowErrorMatchingInlineSnapshot(
`"Invalid theme tags [bar], options: [v7dark, v7light, v8dark, v8light]"`
expect(() => parseThemeTags(['v1dark', 'v1light', 'bar'])).toThrowErrorMatchingInlineSnapshot(
`"Invalid theme tags [bar], options: [v1dark, v1light]"`
);
});

it('throws when an invalid tags in comma separated list', () => {
expect(() => parseThemeTags('v8light ,v7light,bar,box ')).toThrowErrorMatchingInlineSnapshot(
`"Invalid theme tags [bar, box], options: [v7dark, v7light, v8dark, v8light]"`
expect(() => parseThemeTags('v1dark ,v1light, bar, box')).toThrowErrorMatchingInlineSnapshot(
`"Invalid theme tags [bar, box], options: [v1dark, v1light]"`
);
});

it('returns tags in alphabetical order', () => {
const tags = parseThemeTags(['v7light', 'v8light']);
const tags = parseThemeTags(['v1dark', 'v1light']);
expect(tags).toEqual(tags.slice().sort((a, b) => a.localeCompare(b)));
});

it('returns an immutable array', () => {
expect(() => {
const tags = parseThemeTags('v8light');
const tags = parseThemeTags('v1light');
// @ts-expect-error
tags.push('foo');
}).toThrowErrorMatchingInlineSnapshot(`"Cannot add property 1, object is not extensible"`);
Expand Down
6 changes: 3 additions & 3 deletions packages/osd-optimizer/src/common/theme_tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ const isArrayOfStrings = (input: unknown): input is string[] =>
Array.isArray(input) && input.every((v) => typeof v === 'string');

export type ThemeTags = readonly ThemeTag[];
export type ThemeTag = 'v7light' | 'v7dark' | 'v8light' | 'v8dark';
export const DEFAULT_THEMES = tags('v7light', 'v7dark');
export const ALL_THEMES = tags('v7light', 'v7dark', 'v8light', 'v8dark');
export type ThemeTag = 'v1light' | 'v1dark';
export const DEFAULT_THEMES = tags('v1light', 'v1dark');
export const ALL_THEMES = tags('v1light', 'v1dark');

export function parseThemeTags(input?: any): ThemeTags {
if (!input) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ it('builds expected bundles, saves bundle counts to metadata', async () => {
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/legacy/_other_styles.scss,
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/legacy/styles.scss,
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/plugins/bar/public/lib.ts,
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/src/core/public/core_app/styles/_globals_v7dark.scss,
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/src/core/public/core_app/styles/_globals_v7light.scss,
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/src/core/public/core_app/styles/_globals_v1dark.scss,
<absolute path>/packages/osd-optimizer/src/__fixtures__/__tmp__/mock_repo/src/core/public/core_app/styles/_globals_v1light.scss,
<absolute path>/packages/osd-optimizer/target/worker/entry_point_creator.js,
<absolute path>/packages/osd-ui-shared-deps/public_path_module_creator.js,
]
Expand Down
4 changes: 2 additions & 2 deletions packages/osd-optimizer/src/optimizer/cache_keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ describe('getOptimizerCacheKey()', () => {
"optimizerCacheKey": "♻",
"repoRoot": <absolute path>,
"themeTags": Array [
"v7dark",
"v7light",
"v1dark",
"v1light",
],
},
}
Expand Down
4 changes: 1 addition & 3 deletions packages/osd-optimizer/src/worker/theme_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ import { stringifyRequest, getOptions } from 'loader-utils';
import webpack from 'webpack';
import { parseThemeTags, ALL_THEMES, ThemeTag } from '../common';

const getVersion = (tag: ThemeTag) => (tag.includes('v7') ? 7 : 8);
const getIsDark = (tag: ThemeTag) => tag.includes('dark');
const compare = (a: ThemeTag, b: ThemeTag) =>
(getVersion(a) === getVersion(b) ? 1 : 0) + (getIsDark(a) === getIsDark(b) ? 1 : 0);
const compare = (a: ThemeTag, b: ThemeTag) => (getIsDark(a) === getIsDark(b) ? 1 : 0);

// eslint-disable-next-line import/no-default-export
export default function (this: webpack.loader.LoaderContext) {
Expand Down
10 changes: 0 additions & 10 deletions packages/osd-ui-shared-deps/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,11 @@ export const baseCssDistFilename: string;
*/
export const darkCssDistFilename: string;

/**
* Filename of the dark-theme css file in the distributable directory
*/
export const darkV8CssDistFilename: string;

/**
* Filename of the light-theme css file in the distributable directory
*/
export const lightCssDistFilename: string;

/**
* Filename of the light-theme css file in the distributable directory
*/
export const lightV8CssDistFilename: string;

/**
* Externals mapping inteded to be used in a webpack config
*/
Expand Down
6 changes: 2 additions & 4 deletions packages/osd-ui-shared-deps/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,8 @@ exports.distDir = Path.resolve(__dirname, 'target');
exports.jsDepFilenames = ['osd-ui-shared-deps.@elastic.js'];
exports.jsFilename = 'osd-ui-shared-deps.js';
exports.baseCssDistFilename = 'osd-ui-shared-deps.css';
exports.lightCssDistFilename = 'osd-ui-shared-deps.v7.light.css';
exports.lightV8CssDistFilename = 'osd-ui-shared-deps.v8.light.css';
exports.darkCssDistFilename = 'osd-ui-shared-deps.v7.dark.css';
exports.darkV8CssDistFilename = 'osd-ui-shared-deps.v8.dark.css';
exports.lightCssDistFilename = 'osd-ui-shared-deps.v1.light.css';
exports.darkCssDistFilename = 'osd-ui-shared-deps.v1.dark.css';
exports.externals = {
// stateful deps
angular: '__osdSharedDeps__.Angular',
Expand Down
15 changes: 4 additions & 11 deletions packages/osd-ui-shared-deps/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,19 @@
*/

import LightTheme from '@elastic/eui/dist/eui_theme_light.json';
import DarkTheme from '@elastic/eui/dist/eui_theme_dark.json';

const globals: any = typeof window === 'undefined' ? {} : window;

export type Theme = typeof LightTheme;

// in the OpenSearch Dashboards app we can rely on this global being defined, but in
// some cases (like jest) the global is undefined
export const tag: string = globals.__osdThemeTag__ || 'v7light';
export const version = tag.startsWith('v7') ? 7 : 8;
export const tag: string = globals.__osdThemeTag__ || 'v1light';
export const darkMode = tag.endsWith('dark');

export let euiLightVars: Theme;
export let euiDarkVars: Theme;
if (version === 7) {
euiLightVars = require('@elastic/eui/dist/eui_theme_light.json');
euiDarkVars = require('@elastic/eui/dist/eui_theme_dark.json');
} else {
euiLightVars = require('@elastic/eui/dist/eui_theme_amsterdam_light.json');
euiDarkVars = require('@elastic/eui/dist/eui_theme_amsterdam_dark.json');
}
export const euiLightVars: Theme = LightTheme;
export const euiDarkVars: Theme = DarkTheme;

/**
* EUI Theme vars that automatically adjust to light/dark theme
Expand Down
6 changes: 2 additions & 4 deletions packages/osd-ui-shared-deps/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ exports.getWebpackConfig = ({ dev = false } = {}) => ({
mode: dev ? 'development' : 'production',
entry: {
'osd-ui-shared-deps': './entry.js',
'osd-ui-shared-deps.v7.dark': ['@elastic/eui/dist/eui_theme_dark.css'],
'osd-ui-shared-deps.v7.light': ['@elastic/eui/dist/eui_theme_light.css'],
'osd-ui-shared-deps.v8.dark': ['@elastic/eui/dist/eui_theme_amsterdam_dark.css'],
'osd-ui-shared-deps.v8.light': ['@elastic/eui/dist/eui_theme_amsterdam_light.css'],
'osd-ui-shared-deps.v1.dark': ['@elastic/eui/dist/eui_theme_dark.css'],
'osd-ui-shared-deps.v1.light': ['@elastic/eui/dist/eui_theme_light.css'],
},
context: __dirname,
devtool: dev ? '#cheap-source-map' : false,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// v7dark global scope
// v1dark global scope
// ---
// prepended to all .scss imports (from JS, when v7dark theme selected)
// prepended to all .scss imports (from JS, when v1dark theme selected)

@import "@elastic/eui/src/themes/eui/eui_colors_dark";
@import "@elastic/eui/src/themes/eui/eui_globals";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// v7light global scope
// v1light global scope
// ---
// prepended to all .scss imports (from JS, when v7light theme selected)
// prepended to all .scss imports (from JS, when v1light theme selected)

@import "@elastic/eui/src/themes/eui/eui_colors_light";
@import "@elastic/eui/src/themes/eui/eui_globals";
Expand Down
7 changes: 0 additions & 7 deletions src/core/public/core_app/styles/_globals_v8dark.scss

This file was deleted.

7 changes: 0 additions & 7 deletions src/core/public/core_app/styles/_globals_v8light.scss

This file was deleted.

1 change: 0 additions & 1 deletion src/core/public/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export class UiSettingsClient implements IUiSettingsClient {
this.api = params.api;
this.defaults = cloneDeep(params.defaults);
this.cache = defaultsDeep({}, this.defaults, cloneDeep(params.initialSettings));

params.done$.subscribe({
complete: () => {
this.update$.complete();
Expand Down
24 changes: 16 additions & 8 deletions src/core/server/ui_settings/settings/theme.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,22 @@ describe('theme settings', () => {
describe('theme:version', () => {
const validate = getValidationFn(themeSettings['theme:version']);

it('should only accept valid values', () => {
expect(() => validate('v7')).not.toThrow();
expect(() => validate('v8 (beta)')).not.toThrow();
expect(() => validate('v12')).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value to equal [v7]
- [1]: expected value to equal [v8 (beta)]"
`);
it('should only accept v1', () => {
expect(() => validate('v1')).not.toThrow();
});

it('should not accept v7', () => {
expect(() => validate('v7')).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value to equal [v1]"
`);
});

it('should not accept v8 (beta)', () => {
expect(() => validate('v7')).toThrowErrorMatchingInlineSnapshot(`
"types that failed validation:
- [0]: expected value to equal [v1]"
`);
});
});
});
13 changes: 2 additions & 11 deletions src/core/server/ui_settings/settings/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,8 @@ export const getThemeSettings = (): Record<string, UiSettingsParams> => {
schema: schema.boolean(),
},
'theme:version': {
name: i18n.translate('core.ui_settings.params.themeVersionTitle', {
defaultMessage: 'Theme version',
}),
value: 'v7',
type: 'select',
options: ['v7', 'v8 (beta)'],
description: i18n.translate('core.ui_settings.params.themeVersionText', {
defaultMessage: `Switch between the theme used for the current and next version of OpenSearch Dashboards, A page refresh is required for the setting to be applied.`,
}),
requiresPageReload: true,
schema: schema.oneOf([schema.literal('v7'), schema.literal('v8 (beta)')]),
value: 'v1',
schema: schema.oneOf([schema.literal('v1')]),
readonly: true,
},
};
Expand Down
5 changes: 4 additions & 1 deletion src/core/server/ui_settings/ui_settings_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) =>
];

const configSchema = schema.object({
overrides: schema.object({}, { unknowns: 'allow' }),
overrides: schema.object(
{ 'theme:version': schema.string({ defaultValue: 'v1' }) },
{ unknowns: 'allow' }
),
});

export type UiSettingsConfigType = TypeOf<typeof configSchema>;
Expand Down
13 changes: 3 additions & 10 deletions src/legacy/ui/ui_render/ui_render_mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,7 @@ export function uiRenderMixin(osdServer, server, config) {
? await uiSettings.get('theme:darkMode')
: false;

const themeVersion =
!authEnabled || request.auth.isAuthenticated ? await uiSettings.get('theme:version') : 'v7';

const themeTag = `${themeVersion === 'v7' ? 'v7' : 'v8'}${darkMode ? 'dark' : 'light'}`;
const themeTag = `v1${darkMode ? 'dark' : 'light'}`;

const buildHash = server.newPlatform.env.packageInfo.buildNum;
const basePath = config.get('server.basePath');
Expand All @@ -111,16 +108,12 @@ export function uiRenderMixin(osdServer, server, config) {
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`,
...(darkMode
? [
themeVersion === 'v7'
? `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.darkCssDistFilename}`
: `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.darkV8CssDistFilename}`,
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.darkCssDistFilename}`,
`${basePath}/node_modules/@osd/ui-framework/dist/kui_dark.css`,
`${basePath}/ui/legacy_dark_theme.css`,
]
: [
themeVersion === 'v7'
? `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.lightCssDistFilename}`
: `${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.lightV8CssDistFilename}`,
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.lightCssDistFilename}`,
`${basePath}/node_modules/@osd/ui-framework/dist/kui_light.css`,
`${basePath}/ui/legacy_light_theme.css`,
]),
Expand Down

0 comments on commit aba71ca

Please sign in to comment.