Skip to content

Commit

Permalink
Fix discover, tsvb and Lens chart theming issues (#69695)
Browse files Browse the repository at this point in the history
  • Loading branch information
nickofthyme authored Jul 2, 2020
1 parent c686719 commit f8ba824
Show file tree
Hide file tree
Showing 34 changed files with 439 additions and 197 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
"@babel/plugin-transform-modules-commonjs": "^7.10.1",
"@babel/register": "^7.10.1",
"@elastic/apm-rum": "^5.2.0",
"@elastic/charts": "19.5.2",
"@elastic/charts": "19.6.3",
"@elastic/datemath": "5.0.3",
"@elastic/ems-client": "7.9.3",
"@elastic/eui": "24.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-ui-shared-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"kbn:watch": "node scripts/build --dev --watch"
},
"dependencies": {
"@elastic/charts": "19.5.2",
"@elastic/charts": "19.6.3",
"@elastic/eui": "24.1.0",
"@elastic/numeral": "^2.5.0",
"@kbn/i18n": "1.0.0",
Expand Down
72 changes: 2 additions & 70 deletions src/plugins/charts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,80 +18,12 @@ Color mappings in `value`/`text` form

### `getHeatmapColors`

Funciton to retrive heatmap related colors based on `value` and `colorSchemaName`
Function to retrieve heatmap related colors based on `value` and `colorSchemaName`

### `truncatedColorSchemas`

Truncated color mappings in `value`/`text` form

## Theme

the `theme` service offers utilities to interact with theme of kibana. EUI provides a light and dark theme object to work with Elastic-Charts. However, every instance of a Chart would need to pass down this the correctly EUI theme depending on Kibana's light or dark mode. There are several ways you can use the `theme` service to get the correct theme.

> The current theme (light or dark) of Kibana is typically taken into account for the functions below.
### `useChartsTheme`

The simple fetching of the correct EUI theme; a **React hook**.

```js
import { npStart } from 'ui/new_platform';
import { Chart, Settings } from '@elastic/charts';

export const YourComponent = () => (
<Chart>
<Settings theme={npStart.plugins.charts.theme.useChartsTheme()} />
</Chart>
);
```

### `chartsTheme$`

An **observable** of the current charts theme. Use this implementation for more flexible updates to the chart theme without full page refreshes.

```tsx
import { npStart } from 'ui/new_platform';
import { EuiChartThemeType } from '@elastic/eui/src/themes/charts/themes';
import { Subscription } from 'rxjs';
import { Chart, Settings } from '@elastic/charts';

interface YourComponentProps {};

interface YourComponentState {
chartsTheme: EuiChartThemeType['theme'];
}

export class YourComponent extends Component<YourComponentProps, YourComponentState> {
private subscription?: Subscription;
public state = {
chartsTheme: npStart.plugins.charts.theme.chartsDefaultTheme,
};

componentDidMount() {
this.subscription = npStart.plugins.charts.theme
.chartsTheme$
.subscribe(chartsTheme => this.setState({ chartsTheme }));
}

componentWillUnmount() {
if (this.subscription) {
this.subscription.unsubscribe();
this.subscription = undefined;
}
}

public render() {
const { chartsTheme } = this.state;

return (
<Chart>
<Settings theme={chartsTheme} />
</Chart>
);
}
}
```

### `chartsDefaultTheme`

Returns default charts theme (i.e. light).
See Theme service [docs](public/services/theme/README.md)
92 changes: 92 additions & 0 deletions src/plugins/charts/public/services/theme/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Theme Service

The `theme` service offers utilities to interact with the kibana theme. EUI provides a light and dark theme object to supplement the Elastic-Charts `baseTheme`. However, every instance of a Chart would need to pass down the correct EUI theme depending on Kibana's light or dark mode. There are several ways you can use the `theme` service to get the correct shared `theme` and `baseTheme`.

> The current theme (light or dark) of Kibana is typically taken into account for the functions below.
## `chartsDefaultBaseTheme`

Default `baseTheme` from `@elastic/charts` (i.e. light).

## `chartsDefaultTheme`

Default `theme` from `@elastic/eui` (i.e. light).

## `useChartsTheme` and `useChartsBaseTheme`

A **React hook** for simple fetching of the correct EUI `theme` and `baseTheme`.

```js
import { npStart } from 'ui/new_platform';
import { Chart, Settings } from '@elastic/charts';

export const YourComponent = () => (
<Chart>
<Settings
theme={npStart.plugins.charts.theme.useChartsTheme()}
baseTheme={npStart.plugins.charts.theme.useChartsBaseTheme()}
/>
{/* ... */}
</Chart>
);
```

## `chartsTheme$` and `chartsBaseTheme$`

An **`Observable`** of the current charts `theme` and `baseTheme`. Use this implementation for more flexible updates to the chart theme without full page refreshes.

```tsx
import { npStart } from 'ui/new_platform';
import { EuiChartThemeType } from '@elastic/eui/src/themes/charts/themes';
import { Subscription, combineLatest } from 'rxjs';
import { Chart, Settings, Theme } from '@elastic/charts';

interface YourComponentProps {};

interface YourComponentState {
chartsTheme: EuiChartThemeType['theme'];
chartsBaseTheme: Theme;
}

export class YourComponent extends Component<YourComponentProps, YourComponentState> {
private subscriptions: Subscription[] = [];

public state = {
chartsTheme: npStart.plugins.charts.theme.chartsDefaultTheme,
chartsBaseTheme: npStart.plugins.charts.theme.chartsDefaultBaseTheme,
};

componentDidMount() {
this.subscription = combineLatest(
npStart.plugins.charts.theme.chartsTheme$,
npStart.plugins.charts.theme.chartsBaseTheme$
).subscribe(([chartsTheme, chartsBaseTheme]) =>
this.setState({ chartsTheme, chartsBaseTheme })
);
}

componentWillUnmount() {
if (this.subscription) {
this.subscription.unsubscribe();
}
}

public render() {
const { chartsBaseTheme, chartsTheme } = this.state;

return (
<Chart>
<Settings
theme={chartsTheme}
baseTheme={chartsBaseTheme}
/>
{/* ... */}
</Chart>
);
}
}
```

## Why have `theme` and `baseTheme`?

The `theme` prop is a recursive partial `Theme` that overrides properties from the `baseTheme`. This allows changes to the `Theme` TS type in `@elastic/charts` without having to update the `@elastic/eui` themes for every `<Chart />`.
14 changes: 11 additions & 3 deletions src/plugins/charts/public/services/theme/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,17 @@ import { EUI_CHARTS_THEME_LIGHT } from '@elastic/eui/dist/eui_charts_theme';
import { ThemeService } from './theme';

export const themeServiceMock: ThemeService = {
chartsDefaultTheme: EUI_CHARTS_THEME_LIGHT.theme,
chartsTheme$: jest.fn(() => ({
subsribe: jest.fn(),
subscribe: jest.fn(),
})),
chartsDefaultTheme: EUI_CHARTS_THEME_LIGHT.theme,
useChartsTheme: jest.fn(),
chartsBaseTheme$: jest.fn(() => ({
subscribe: jest.fn(),
})),
darkModeEnabled$: jest.fn(() => ({
subscribe: jest.fn(),
})),
useDarkMode: jest.fn().mockReturnValue(false),
useChartsTheme: jest.fn().mockReturnValue({}),
useChartsBaseTheme: jest.fn().mockReturnValue({}),
} as any;
64 changes: 62 additions & 2 deletions src/plugins/charts/public/services/theme/theme.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,35 @@ import { EUI_CHARTS_THEME_DARK, EUI_CHARTS_THEME_LIGHT } from '@elastic/eui/dist

import { ThemeService } from './theme';
import { coreMock } from '../../../../../core/public/mocks';
import { LIGHT_THEME, DARK_THEME } from '@elastic/charts';

const { uiSettings: setupMockUiSettings } = coreMock.createSetup();

describe('ThemeService', () => {
describe('chartsTheme$', () => {
describe('darkModeEnabled$', () => {
it('should throw error if service has not been initialized', () => {
const themeService = new ThemeService();
expect(() => themeService.chartsTheme$).toThrowError();
expect(() => themeService.darkModeEnabled$).toThrowError();
});

it('returns the false when not in dark mode', async () => {
setupMockUiSettings.get$.mockReturnValue(new BehaviorSubject(false));
const themeService = new ThemeService();
themeService.init(setupMockUiSettings);

expect(await themeService.darkModeEnabled$.pipe(take(1)).toPromise()).toBe(false);
});

it('returns the true when in dark mode', async () => {
setupMockUiSettings.get$.mockReturnValue(new BehaviorSubject(true));
const themeService = new ThemeService();
themeService.init(setupMockUiSettings);

expect(await themeService.darkModeEnabled$.pipe(take(1)).toPromise()).toBe(true);
});
});

describe('chartsTheme$', () => {
it('returns the light theme when not in dark mode', async () => {
setupMockUiSettings.get$.mockReturnValue(new BehaviorSubject(false));
const themeService = new ThemeService();
Expand All @@ -58,6 +78,28 @@ describe('ThemeService', () => {
});
});

describe('chartsBaseTheme$', () => {
it('returns the light theme when not in dark mode', async () => {
setupMockUiSettings.get$.mockReturnValue(new BehaviorSubject(false));
const themeService = new ThemeService();
themeService.init(setupMockUiSettings);

expect(await themeService.chartsBaseTheme$.pipe(take(1)).toPromise()).toEqual(LIGHT_THEME);
});

describe('in dark mode', () => {
it(`returns the dark theme`, async () => {
// Fake dark theme turned returning true
setupMockUiSettings.get$.mockReturnValue(new BehaviorSubject(true));
const themeService = new ThemeService();
themeService.init(setupMockUiSettings);
const result = await themeService.chartsBaseTheme$.pipe(take(1)).toPromise();

expect(result).toEqual(DARK_THEME);
});
});
});

describe('useChartsTheme', () => {
it('updates when the uiSettings change', () => {
const darkMode$ = new BehaviorSubject(false);
Expand All @@ -75,4 +117,22 @@ describe('ThemeService', () => {
expect(result.current).toBe(EUI_CHARTS_THEME_LIGHT.theme);
});
});

describe('useBaseChartTheme', () => {
it('updates when the uiSettings change', () => {
const darkMode$ = new BehaviorSubject(false);
setupMockUiSettings.get$.mockReturnValue(darkMode$);
const themeService = new ThemeService();
themeService.init(setupMockUiSettings);
const { useChartsBaseTheme } = themeService;

const { result } = renderHook(() => useChartsBaseTheme());
expect(result.current).toBe(LIGHT_THEME);

act(() => darkMode$.next(true));
expect(result.current).toBe(DARK_THEME);
act(() => darkMode$.next(false));
expect(result.current).toBe(LIGHT_THEME);
});
});
});
Loading

0 comments on commit f8ba824

Please sign in to comment.