fix: Small tweaks for Line and Area chart migrations (ECharts) (apach…

…e#28334) Co-authored-by: John Bodley <4567245+john-bodley@users.noreply.github.com>
EnxDev · May 31, 2024 · 71f8e5a · 71f8e5a
1 parent 5adb498
commit 71f8e5a
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 9 deletions.
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx
@@ -25,6 +25,7 @@ import {
   getColumnLabel,
   getNumberFormatter,
   LegendState,
+  ensureIsArray,
 } from '@superset-ui/core';
 import { ViewRootGroup } from 'echarts/types/src/util/types';
 import GlobalModel from 'echarts/types/src/model/Global';
@@ -173,6 +174,7 @@ export default function EchartsTimeseries({
           ...(eventParams.name ? [eventParams.name] : []),
           ...(labelMap[seriesName] ?? []),
         ];
+        const groupBy = ensureIsArray(formData.groupby);
         if (data && xAxis.type === AxisType.Time) {
           drillToDetailFilters.push({
             col:
@@ -188,7 +190,7 @@ export default function EchartsTimeseries({
         }
         [
           ...(xAxis.type === AxisType.Category && data ? [xAxis.label] : []),
-          ...formData.groupby,
+          ...groupBy,
         ].forEach((dimension, i) =>
           drillToDetailFilters.push({
             col: dimension,
@@ -197,7 +199,7 @@ export default function EchartsTimeseries({
             formattedVal: String(values[i]),
           }),
         );
-        formData.groupby.forEach((dimension, i) => {
+        groupBy.forEach((dimension, i) => {
           const val = labelMap[seriesName][i];
           drillByFilters.push({
             col: dimension,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -184,10 +184,10 @@ export default function transformProps(
     zoomable,
   }: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
   const refs: Refs = {};
-
+  const groupBy = ensureIsArray(groupby);
   const labelMap = Object.entries(label_map).reduce((acc, entry) => {
     if (
-      entry[1].length > groupby.length &&
+      entry[1].length > groupBy.length &&
       Array.isArray(timeCompare) &&
       timeCompare.includes(entry[1][0])
     ) {
@@ -219,7 +219,7 @@ export default function transformProps(
     getMetricLabel,
   );
 
-  const isMultiSeries = groupby?.length || metrics?.length > 1;
+  const isMultiSeries = groupBy.length || metrics?.length > 1;
 
   const [rawSeries, sortedTotalValues, minPositiveValue] = extractSeries(
     rebasedData,
@@ -538,7 +538,7 @@ export default function transformProps(
           // if there are no dimensions, key is a verbose name of a metric,
           // otherwise it is a comma separated string where the first part is metric name
           const formatterKey =
-            groupby.length === 0 ? inverted[key] : labelMap[key]?.[0];
+            groupBy.length === 0 ? inverted[key] : labelMap[key]?.[0];
           const content = formatForecastTooltipSeries({
             ...value,
             seriesName: key,
@@ -576,6 +576,7 @@ export default function transformProps(
       right: TIMESERIES_CONSTANTS.toolboxRight,
       feature: {
         dataZoom: {
+          ...(stack ? { yAxisIndex: false } : {}), // disable y-axis zoom for stacked charts
           title: {
             zoom: t('zoom area'),
             back: t('restore zoom'),
@@ -604,7 +605,7 @@ export default function transformProps(
     echartOptions,
     emitCrossFilters,
     formData,
-    groupby,
+    groupby: groupBy,
     height,
     labelMap,
     selectedValues,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts
@@ -21,7 +21,6 @@ import {
   AnnotationLayer,
   AxisType,
   ContributionType,
-  QueryFormColumn,
   QueryFormData,
   QueryFormMetric,
   TimeFormatter,
@@ -87,7 +86,6 @@ export type EchartsTimeseriesFormData = QueryFormData & {
   zoomable: boolean;
   richTooltip: boolean;
   xAxisLabelRotation: number;
-  groupby: QueryFormColumn[];
   showValue: boolean;
   onlyTotal: boolean;
   showExtraControls: boolean;

diff --git a/superset/migrations/shared/migrate_viz/processors.py b/superset/migrations/shared/migrate_viz/processors.py
@@ -128,6 +128,10 @@ def _pre_action(self) -> None:
         ):
             self.data["bottom_margin"] = 30
 
+        left_margin = self.data.get("left_margin")
+        if self.data.get("y_axis_label") and (not left_margin or left_margin == "auto"):
+            self.data["left_margin"] = 30
+
         if (rolling_type := self.data.get("rolling_type")) and rolling_type != "None":
             self.data["rolling_type"] = rolling_type