fix(charts): fix legend theming and hollow symbols in dark mode (#35123)

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts

@@ -25,6 +25,7 @@ import {
   FilterState,
   FormulaAnnotationLayer,
   IntervalAnnotationLayer,
+  isThemeDark,
   LegendState,
   SupersetTheme,
   TimeseriesAnnotationLayer,
@@ -137,6 +138,33 @@ export const getBaselineSeriesForStream = (
   };
 };
 
+export function transformNegativeLabelsPosition(
+  series: SeriesOption,
+  isHorizontal: boolean,
+): TimeseriesDataRecord[] {
+  /*
+   * Adjusts label position for negative values in bar series
+   * @param series - Array of series options
+   * @param isHorizontal - Whether chart is horizontal
+   * @returns data with adjusted label positions for negative values
+   */
+  const transformValue = (value: any) => {
+    const [xValue, yValue] = Array.isArray(value) ? value : [null, null];
+    const axisValue = isHorizontal ? xValue : yValue;
+
+    return axisValue < 0
+      ? {
+          value,
+          label: {
+            position: 'outside',
+          },
+        }
+      : value;
+  };
+
+  return (series.data as TimeseriesDataRecord[]).map(transformValue);
+}
+
 export function transformSeries(
   series: SeriesOption,
   colorScale: CategoricalColorScale,
@@ -246,6 +274,9 @@ export function transformSeries(
   } else {
     plotType = seriesType === 'bar' ? 'bar' : 'line';
   }
+
+  const isDarkMode = theme ? isThemeDark(theme) : false;
+
   /**
    * if timeShiftColor is enabled the colorScaleKey forces the color to be the
    * same as the original series, otherwise uses separate colors
@@ -289,6 +320,12 @@ export function transformSeries(
     isConfidenceBand || (stack === StackControlsValue.Stream && area)
       ? { ...opts.lineStyle, opacity: OpacityEnum.Transparent }
       : { ...opts.lineStyle, opacity };
+
+  // Use filled circles in dark mode to avoid the white fill issue with hollow circles
+  // Use emptyCircle explicitly in light mode
+  const symbol =
+    plotType === 'line' ? (isDarkMode ? 'circle' : 'emptyCircle') : undefined;
+
   return {
     ...series,
     ...(Array.isArray(data) && seriesType === 'bar' && !stack
@@ -321,6 +358,7 @@ export function transformSeries(
         : undefined,
     emphasis,
     showSymbol,
+    symbol,
     symbolSize: markerSize,
     label: {
       show: !!showValue,
@@ -637,30 +675,3 @@ export function getPadding(
     isHorizontal,
   );
 }
-
-export function transformNegativeLabelsPosition(
-  series: SeriesOption,
-  isHorizontal: boolean,
-): TimeseriesDataRecord[] {
-  /*
-   * Adjusts label position for negative values in bar series
-   * @param series - Array of series options
-   * @param isHorizontal - Whether chart is horizontal
-   * @returns data with adjusted label positions for negative values
-   */
-  const transformValue = (value: any) => {
-    const [xValue, yValue] = Array.isArray(value) ? value : [null, null];
-    const axisValue = isHorizontal ? xValue : yValue;
-
-    return axisValue < 0
-      ? {
-          value,
-          label: {
-            position: 'outside',
-          },
-        }
-      : value;
-  };
-
-  return (series.data as TimeseriesDataRecord[]).map(transformValue);
-}

superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx

@@ -204,6 +204,12 @@ function Echart(
           },
           legend: {
             textStyle: { color: antdTheme.colorTextSecondary },
+            pageTextStyle: {
+              color: antdTheme.colorTextSecondary,
+            },
+            pageIconColor: antdTheme.colorTextSecondary,
+            pageIconInactiveColor: antdTheme.colorTextDisabled,
+            inactiveColor: antdTheme.colorTextDisabled,
           },
           tooltip: {
             backgroundColor: antdTheme.colorBgContainer,