ui: Tidy up chart theming (#4905)

- Centralize theming in EchartView to avoid duplication.
- Avoid depending on `pf-theme-provider` and remove fallback to
documentElement as this won't have the CSS variables set.
- Fixes reactivity in scroll position chart in chrome scroll plugin
- Use themes rather than options to specify colors, which avoids having
to fiddle around with injecting theme colors into options.

Author: stevegolton

ui/src/base/assert.ts

@@ -41,9 +41,11 @@ export function assertDefined<T>(value: T | undefined, optMsg?: string): T {
   return value;
 }
 
+// Asserts that the value is an instance of the given class. Returns the value
+// with a narrowed type if the assertion passes, otherwise throws an error.
 export function assertIsInstance<T>(
   value: unknown,
-  clazz: Function,
+  clazz: abstract new (...args: never[]) => T,
   optMsg?: string,
 ): T {
   assertTrue(

ui/src/components/widgets/charts/bar_chart.ts

@@ -22,7 +22,6 @@ import {
   buildBrushOption,
   buildTooltipOption,
 } from './chart_option_builder';
-import {getChartThemeColors} from './chart_theme';
 
 /**
  * A single bar in the bar chart.
@@ -168,8 +167,6 @@ function buildBarOption(
   } = attrs;
   const fmtDimension = formatDimension ?? String;
   const fmtMeasure = formatMeasure ?? formatNumber;
-
-  const theme = getChartThemeColors();
   const horizontal = orientation === 'horizontal';
   const labels = data.items.map((item) => fmtDimension(item.label));
 
@@ -209,7 +206,6 @@ function buildBarOption(
 
   const option: Record<string, unknown> = {
     animation: false,
-    color: [...theme.chartColors],
     grid: buildGridOption({
       bottom: dimensionLabel && !horizontal ? 45 : 25,
     }),
@@ -228,11 +224,10 @@ function buildBarOption(
         type: 'bar',
         data: data.items.map((item) => item.value),
         itemStyle: barColor !== undefined ? {color: barColor} : undefined,
-        emphasis: {
-          itemStyle: {
-            color: barHoverColor ?? theme.accentColor,
-          },
-        },
+        emphasis:
+          barHoverColor !== undefined
+            ? {itemStyle: {color: barHoverColor}}
+            : undefined,
       },
     ],
   };
@@ -247,7 +242,7 @@ function buildBarOption(
     option.toolbox = {show: false};
   }
 
-  return option as EChartsCoreOption;
+  return option;
 }
 
 function buildBarEventHandlers(

ui/src/components/widgets/charts/boxplot.ts

@@ -21,7 +21,6 @@ import {
   buildGridOption,
   buildTooltipOption,
 } from './chart_option_builder';
-import {getChartThemeColors} from './chart_theme';
 
 /**
  * A single box in a boxplot chart.
@@ -132,7 +131,6 @@ function buildBoxplotOption(
   } = attrs;
   const fmtVal = formatValue ?? formatNumber;
   const horizontal = orientation === 'horizontal';
-  const theme = getChartThemeColors();
 
   const showXAxisGrid = gridLines === 'vertical' || gridLines === 'both';
   const showYAxisGrid = gridLines === 'horizontal' || gridLines === 'both';
@@ -177,7 +175,6 @@ function buildBoxplotOption(
 
   const option: Record<string, unknown> = {
     animation: false,
-    color: [...theme.chartColors],
     grid: buildGridOption({
       bottom: horizontal ? 25 : categoryLabel ? 40 : 25,
     }),

ui/src/components/widgets/charts/chart_option_builder.ts

@@ -12,8 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+/**
+ * Chart option builder utilities.
+ *
+ * These functions build ECharts option objects with a consistent structure.
+ * Theme colors are NOT included here - EChartView applies theme colors
+ * automatically by reading CSS variables from the DOM element.
+ */
+
 import type {EChartsCoreOption} from 'echarts/core';
-import {getChartThemeColors} from './chart_theme';
 
 /**
  * Configuration for an axis in a chart.
@@ -48,35 +55,28 @@ export interface BrushConfig {
 
 /**
  * Build an axis option from config.
- * Explicitly includes theme colors because ECharts doesn't deep merge
- * option objects with theme objects - setting axisLabel overrides the theme's
- * axisLabel entirely, so we must include colors here.
+ * Theme colors are applied by EChartView, so we omit color settings here.
  */
 export function buildAxisOption(
   config: AxisConfig,
   isXAxis: boolean,
 ): Record<string, unknown> {
-  const theme = getChartThemeColors();
   const axis: Record<string, unknown> = {
     type: config.type,
     name: config.name,
     nameLocation: isXAxis ? ('middle' as const) : ('end' as const),
     nameGap: config.nameGap ?? (isXAxis ? 25 : 10),
-    nameTextStyle: {fontSize: 11, color: theme.textColor},
+    nameTextStyle: {fontSize: 11},
     axisLabel: {
       fontSize: 10,
-      color: theme.textColor,
       ...(config.formatter !== undefined && {formatter: config.formatter}),
       ...(config.labelOverflow !== undefined && {
         overflow: config.labelOverflow,
       }),
       ...(config.labelWidth !== undefined && {width: config.labelWidth}),
     },
-    axisTick: {lineStyle: {color: theme.borderColor}},
-    axisLine: {lineStyle: {color: theme.borderColor}},
     splitLine: {
       show: config.showSplitLine ?? false,
-      lineStyle: {color: theme.borderColor},
     },
   };
 
@@ -100,7 +100,7 @@ export function buildAxisOption(
 }
 
 /**
- * Build a themed grid option.
+ * Build a grid option.
  */
 export function buildGridOption(opts?: {
   top?: number;
@@ -119,28 +119,23 @@ export function buildGridOption(opts?: {
 }
 
 /**
- * Build a themed tooltip option.
- * Explicitly includes theme colors so tooltips adapt when the theme changes.
+ * Build a tooltip option.
+ * Theme colors are applied by EChartView.
  * Extra options are merged in (e.g. trigger, formatter).
  */
 export function buildTooltipOption(
   extra?: Record<string, unknown>,
 ): Record<string, unknown> {
-  const theme = getChartThemeColors();
   return {
-    backgroundColor: theme.backgroundColor,
-    borderColor: theme.borderColor,
-    textStyle: {color: theme.textColor},
     ...extra,
   };
 }
 
 /**
  * Build a brush configuration.
- * Uses accent color from theme (ECharts doesn't theme brush colors).
+ * Theme colors are applied by EChartView.
  */
 export function buildBrushOption(config: BrushConfig): Record<string, unknown> {
-  const theme = getChartThemeColors();
   return {
     ...(config.xAxisIndex !== undefined && {xAxisIndex: config.xAxisIndex}),
     ...(config.yAxisIndex !== undefined && {yAxisIndex: config.yAxisIndex}),
@@ -149,7 +144,6 @@ export function buildBrushOption(config: BrushConfig): Record<string, unknown> {
     brushStyle: {
       borderWidth: 1,
       color: 'rgba(0, 0, 0, 0.1)',
-      borderColor: theme.accentColor,
     },
     throttleType: 'debounce' as const,
     throttleDelay: 100,
@@ -158,13 +152,11 @@ export function buildBrushOption(config: BrushConfig): Record<string, unknown> {
 
 /**
  * Build a legend option.
- * Explicitly includes theme colors because ECharts doesn't deep merge
- * option objects with theme objects.
+ * Theme colors are applied by EChartView.
  */
 export function buildLegendOption(
   position: 'top' | 'right' = 'top',
 ): Record<string, unknown> {
-  const theme = getChartThemeColors();
   if (position === 'right') {
     return {
       show: true,
@@ -178,7 +170,6 @@ export function buildLegendOption(
         width: 120,
         overflow: 'truncate',
         ellipsis: '\u2026',
-        color: theme.textColor,
       },
       tooltip: {show: true},
       pageButtonPosition: 'end',
@@ -187,17 +178,16 @@ export function buildLegendOption(
   return {
     show: true,
     top: 0,
-    textStyle: {fontSize: 10, color: theme.textColor},
+    textStyle: {fontSize: 10},
   };
 }
 
 /**
  * Build a complete base chart option with grid, axes, and optional
  * tooltip/brush/legend. Charts add their own `series` on top.
  *
- * The `color` array (series colors) and tooltip theme colors are included
- * so that charts update automatically when the Mithril component re-renders
- * after a theme change — without needing to reinitialize the ECharts instance.
+ * Theme colors (series colors, text colors, etc.) are applied by EChartView
+ * which reads CSS variables from the DOM element.
  */
 export function buildChartOption(config: {
   readonly grid?: Parameters<typeof buildGridOption>[0];
@@ -208,11 +198,9 @@ export function buildChartOption(config: {
   readonly legend?: Record<string, unknown>;
 }): EChartsCoreOption {
   const {grid, xAxis, yAxis, tooltip, brush, legend} = config;
-  const theme = getChartThemeColors();
 
   const option: Record<string, unknown> = {
     animation: false,
-    color: [...theme.chartColors],
     grid: buildGridOption(grid),
     xAxis: buildAxisOption(xAxis, true),
     yAxis: buildAxisOption(yAxis, false),

ui/src/components/widgets/charts/chart_theme.ts

@@ -43,15 +43,11 @@ export interface ChartThemeColors {
 
 /**
  * Returns the current theme colors by reading CSS variables from the given
- * element. Falls back to .pf-theme-provider then document.documentElement so
- * charts work in embedded builds that omit the theme provider.
+ * element. The element must be within the DOM tree that has the theme CSS
+ * variables defined (typically a child of .pf-theme-provider).
  */
-export function getChartThemeColors(el?: Element): ChartThemeColors {
-  const elem =
-    el ??
-    document.querySelector('.pf-theme-provider') ??
-    document.documentElement;
-  const style = getComputedStyle(elem);
+export function getChartThemeColors(el: Element): ChartThemeColors {
+  const style = getComputedStyle(el);
 
   const chartColors: string[] = [];
   for (let i = 1; i <= 8; i++) {