feat: Theming - Charts (#1608)

Added ChartThemeProvider, added support for resolving css variables in
chart theme, and updated chart theme to use css variables.

resolves #1572

BREAKING CHANGE: 
- ChartThemeProvider is now required to provide ChartTheme
- ChartModelFactory and ChartUtils now require chartTheme args

Author: bmingles

package-lock.json

@@ -1,4 +1,5 @@
 import { useContext, useMemo } from 'react';
+import { ChartThemeProvider } from '@deephaven/chart';
 import { ThemeProvider } from '@deephaven/components';
 import { PluginsContext } from '@deephaven/plugin';
 import { getThemeDataFromPlugins } from '../plugins';
@@ -19,7 +20,11 @@ export function ThemeBootstrap({ children }: ThemeBootstrapProps): JSX.Element {
     [pluginModules]
   );
 
-  return <ThemeProvider themes={themes}>{children}</ThemeProvider>;
+  return (
+    <ThemeProvider themes={themes}>
+      <ChartThemeProvider>{children}</ChartThemeProvider>
+    </ThemeProvider>
+  );
 }
 
 export default ThemeBootstrap;

packages/app-utils/src/components/ThemeBootstrap.tsx

@@ -27,10 +27,12 @@
     "build:sass": "sass --embed-sources --load-path=../../node_modules ./src:./dist"
   },
   "dependencies": {
+    "@deephaven/components": "file:../components",
     "@deephaven/icons": "file:../icons",
     "@deephaven/jsapi-types": "file:../jsapi-types",
     "@deephaven/jsapi-utils": "file:../jsapi-utils",
     "@deephaven/log": "file:../log",
+    "@deephaven/react-hooks": "file:../react-hooks",
     "@deephaven/utils": "file:../utils",
     "deep-equal": "^2.0.5",
     "lodash.debounce": "^4.0.8",

packages/chart/package.json

@@ -28,7 +28,7 @@ import {
   ModeBarButtonAny,
 } from 'plotly.js';
 import type { PlotParams } from 'react-plotly.js';
-import createPlotlyComponent from 'react-plotly.js/factory.js';
+import createPlotlyComponent from './plotly/createPlotlyComponent';
 import Plotly from './plotly/Plotly';
 import ChartModel from './ChartModel';
 import ChartUtils, { ChartModelSettings } from './ChartUtils';

packages/chart/src/Chart.tsx

@@ -1,17 +1,23 @@
 import dh from '@deephaven/jsapi-shim';
+import { TestUtils } from '@deephaven/utils';
 import ChartModelFactory from './ChartModelFactory';
+import type { ChartTheme } from './ChartTheme';
 import FigureChartModel from './FigureChartModel';
 
+const { createMockProxy } = TestUtils;
+
 describe('creating model from metadata', () => {
   it('handles loading a FigureChartModel from table settings', async () => {
     const columns = [{ name: 'A' }, { name: 'B' }, { name: 'C' }];
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const table = new (dh as any).Table({ columns });
     const settings = { series: ['C'], xAxis: 'name', type: 'PIE' as const };
+    const chartTheme = createMockProxy<ChartTheme>();
     const model = await ChartModelFactory.makeModelFromSettings(
       dh,
       settings,
-      table
+      table,
+      chartTheme
     );
 
     expect(model).toBeInstanceOf(FigureChartModel);

packages/chart/src/ChartModelFactory.test.ts

@@ -1,7 +1,7 @@
 import type { dh as DhType, Figure, Table } from '@deephaven/jsapi-types';
 import ChartUtils, { ChartModelSettings } from './ChartUtils';
 import FigureChartModel from './FigureChartModel';
-import ChartTheme from './ChartTheme';
+import { ChartTheme } from './ChartTheme';
 import ChartModel from './ChartModel';
 
 class ChartModelFactory {
@@ -16,7 +16,7 @@ class ChartModelFactory {
    * @param settings.xAxis The column name to use for the x-axis
    * @param [settings.hiddenSeries] Array of hidden series names
    * @param table The table to build the model for
-   * @param theme The theme for the figure. Defaults to ChartTheme
+   * @param theme The theme for the figure
    * @returns The ChartModel Promise representing the figure
    * CRA sets tsconfig to type check JS based on jsdoc comments. It isn't able to figure out FigureChartModel extends ChartModel
    * This causes TS issues in 1 or 2 spots. Once this is TS it can be returned to just FigureChartModel
@@ -25,14 +25,14 @@ class ChartModelFactory {
     dh: DhType,
     settings: ChartModelSettings,
     table: Table,
-    theme = ChartTheme
+    theme: ChartTheme
   ): Promise<ChartModel> {
     const figure = await ChartModelFactory.makeFigureFromSettings(
       dh,
       settings,
       table
     );
-    return new FigureChartModel(dh, figure, settings, theme);
+    return new FigureChartModel(dh, figure, theme, settings);
   }
 
   /**
@@ -78,7 +78,7 @@ class ChartModelFactory {
    * @param settings.xAxis The column name to use for the x-axis
    * @param [settings.hiddenSeries] Array of hidden series names
    * @param figure The figure to build the model for
-   * @param theme The theme for the figure. Defaults to ChartTheme
+   * @param theme The theme for the figure
    * @returns The FigureChartModel representing the figure
    * CRA sets tsconfig to type check JS based on jsdoc comments. It isn't able to figure out FigureChartModel extends ChartModel
    * This causes TS issues in 1 or 2 spots. Once this is TS it can be returned to just FigureChartModel
@@ -87,9 +87,9 @@ class ChartModelFactory {
     dh: DhType,
     settings: ChartModelSettings | undefined,
     figure: Figure,
-    theme = ChartTheme
+    theme: ChartTheme
   ): Promise<ChartModel> {
-    return new FigureChartModel(dh, figure, settings, theme);
+    return new FigureChartModel(dh, figure, theme, settings);
   }
 }
 

packages/chart/src/ChartModelFactory.ts

@@ -2,20 +2,20 @@
 @import '@deephaven/components/scss/custom.scss';
 
 :export {
-  paper-bgcolor: $content-bg;
-  plot-bgcolor: $gray-850;
-  title-color: $white;
-  colorway: $blue $green $yellow $purple $orange $red $white;
-  gridcolor: $gray-700;
-  linecolor: $gray-500;
-  zerolinecolor: $gray-300;
-  activecolor: $primary;
-  rangebgcolor: rgba($gray-500, 0.7);
-  area-color: $blue;
-  trend-color: lighten($green, 20%);
-  line-color: $green;
-  error-band-line-color: lighten($green, 40%);
-  error-band-fill-color: rgba(lighten($green, 20%), 0.1);
-  ohlc-increasing: $green;
-  ohlc-decreasing: $red;
+  paper-bgcolor: var(--dh-color-chart-bg);
+  plot-bgcolor: var(--dh-color-chart-plot-bg);
+  title-color: var(--dh-color-chart-title);
+  colorway: var(--dh-color-chart-colorway);
+  gridcolor: var(--dh-color-chart-grid);
+  linecolor: var(--dh-color-chart-axis-line);
+  zerolinecolor: var(--dh-color-chart-axis-line-zero);
+  activecolor: var(--dh-color-chart-active);
+  rangebgcolor: var(--dh-color-chart-range-bg);
+  area-color: var(--dh-color-chart-area);
+  trend-color: var(--dh-color-chart-trend);
+  line-color: var(--dh-color-chart-line-deprecated);
+  error-band-line-color: var(--dh-color-chart-error-band-line);
+  error-band-fill-color: var(--dh-color-chart-error-band-fill);
+  ohlc-increasing: var(--dh-color-chart-ohlc-increase);
+  ohlc-decreasing: var(--dh-color-chart-ohlc-decrease);
 }

packages/chart/src/ChartTheme.module.scss

@@ -0,0 +1,36 @@
+/// <reference types="./declaration" />
+
+import { TestUtils } from '@deephaven/utils';
+import { resolveCssVariablesInRecord } from '@deephaven/components';
+import { defaultChartTheme } from './ChartTheme';
+import chartThemeRaw from './ChartTheme.module.scss';
+
+jest.mock('@deephaven/components', () => ({
+  ...jest.requireActual('@deephaven/components'),
+  resolveCssVariablesInRecord: jest.fn(),
+}));
+
+const { asMock } = TestUtils;
+
+const mockChartTheme = new Proxy(
+  {},
+  { get: (_target, name) => `chartTheme['${String(name)}']` }
+);
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  expect.hasAssertions();
+
+  asMock(resolveCssVariablesInRecord)
+    .mockName('resolveCssVariablesInRecord')
+    .mockReturnValue(mockChartTheme);
+});
+
+describe('defaultChartTheme', () => {
+  it('should create the default chart theme', () => {
+    const actual = defaultChartTheme();
+
+    expect(resolveCssVariablesInRecord).toHaveBeenCalledWith(chartThemeRaw);
+    expect(actual).toMatchSnapshot();
+  });
+});

packages/chart/src/ChartTheme.test.ts

@@ -1,20 +1,67 @@
-import ChartTheme from './ChartTheme.module.scss';
+import {
+  getExpressionRanges,
+  resolveCssVariablesInRecord,
+} from '@deephaven/components';
+import Log from '@deephaven/log';
+import { ColorUtils } from '@deephaven/utils';
+import chartThemeRaw from './ChartTheme.module.scss';
 
-export default Object.freeze({
-  paper_bgcolor: ChartTheme['paper-bgcolor'],
-  plot_bgcolor: ChartTheme['plot-bgcolor'],
-  title_color: ChartTheme['title-color'],
-  colorway: ChartTheme.colorway,
-  gridcolor: ChartTheme.gridcolor,
-  linecolor: ChartTheme.linecolor,
-  zerolinecolor: ChartTheme.zerolinecolor,
-  activecolor: ChartTheme.activecolor,
-  rangebgcolor: ChartTheme.rangebgcolor,
-  area_color: ChartTheme['area-color'],
-  trend_color: ChartTheme['trend-color'],
-  line_color: ChartTheme['line-color'],
-  error_band_line_color: ChartTheme['error-band-line-color'],
-  error_band_fill_color: ChartTheme['error-band-fill-color'],
-  ohlc_increasing: ChartTheme['ohlc-increasing'],
-  ohlc_decreasing: ChartTheme['ohlc-decreasing'],
-});
+const log = Log.module('ChartTheme');
+
+export interface ChartTheme {
+  paper_bgcolor: string;
+  plot_bgcolor: string;
+  title_color: string;
+  colorway: string;
+  gridcolor: string;
+  linecolor: string;
+  zerolinecolor: string;
+  activecolor: string;
+  rangebgcolor: string;
+  area_color: string;
+  trend_color: string;
+  line_color: string;
+  error_band_line_color: string;
+  error_band_fill_color: string;
+  ohlc_increasing: string;
+  ohlc_decreasing: string;
+}
+
+export function defaultChartTheme(): Readonly<ChartTheme> {
+  const chartTheme = resolveCssVariablesInRecord(chartThemeRaw);
+
+  // The color normalization in `resolveCssVariablesInRecord` won't work for
+  // colorway since it is an array of colors. We need to explicitly normalize
+  // each color expression
+  chartTheme.colorway = getExpressionRanges(chartTheme.colorway ?? '')
+    .map(([start, end]) =>
+      ColorUtils.normalizeCssColor(
+        chartTheme.colorway.substring(start, end + 1)
+      )
+    )
+    .join(' ');
+
+  log.debug2('Chart theme:', chartThemeRaw);
+  log.debug2('Chart theme derived:', chartTheme);
+
+  return Object.freeze({
+    paper_bgcolor: chartTheme['paper-bgcolor'],
+    plot_bgcolor: chartTheme['plot-bgcolor'],
+    title_color: chartTheme['title-color'],
+    colorway: chartTheme.colorway,
+    gridcolor: chartTheme.gridcolor,
+    linecolor: chartTheme.linecolor,
+    zerolinecolor: chartTheme.zerolinecolor,
+    activecolor: chartTheme.activecolor,
+    rangebgcolor: chartTheme.rangebgcolor,
+    area_color: chartTheme['area-color'],
+    trend_color: chartTheme['trend-color'],
+    line_color: chartTheme['line-color'],
+    error_band_line_color: chartTheme['error-band-line-color'],
+    error_band_fill_color: chartTheme['error-band-fill-color'],
+    ohlc_increasing: chartTheme['ohlc-increasing'],
+    ohlc_decreasing: chartTheme['ohlc-decreasing'],
+  });
+}
+
+export default defaultChartTheme;

packages/chart/src/ChartTheme.ts

@@ -0,0 +1,43 @@
+import { createContext, ReactNode, useEffect, useState } from 'react';
+import { useTheme } from '@deephaven/components';
+import defaultChartTheme, { ChartTheme } from './ChartTheme';
+
+export type ChartThemeContextValue = ChartTheme;
+
+export const ChartThemeContext = createContext<ChartThemeContextValue | null>(
+  null
+);
+
+export interface ChartThemeProviderProps {
+  children: ReactNode;
+}
+
+/*
+ * Provides a chart theme based on the active themes from the ThemeProvider.
+ */
+export function ChartThemeProvider({
+  children,
+}: ChartThemeProviderProps): JSX.Element {
+  const { activeThemes } = useTheme();
+
+  const [chartTheme, setChartTheme] = useState<ChartTheme | null>(null);
+
+  // The `ThemeProvider` that supplies `activeThemes` also provides the corresponding
+  // CSS theme variables to the DOM by dynamically rendering <style> tags whenever
+  // the `activeThemes` change. Painting the latest CSS variables to the DOM may
+  // not happen until after `ChartThemeProvider` is rendered, but they should be
+  // available by the time the effect runs. Therefore, it is important to derive
+  // the chart theme in an effect instead of deriving in a `useMemo` to ensure
+  // we have the latest CSS variables.
+  useEffect(() => {
+    if (activeThemes != null) {
+      setChartTheme(defaultChartTheme());
+    }
+  }, [activeThemes]);
+
+  return (
+    <ChartThemeContext.Provider value={chartTheme}>
+      {children}
+    </ChartThemeContext.Provider>
+  );
+}