You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/12/15 16:18:37 UTC

(superset) 04/05: fix(plugin-chart-echarts): use scale for truncating x-axis (#26269)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 91e970537b57a9a6155612c4574bcdb16d4b8a9b
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu Dec 14 10:13:39 2023 -0800

    fix(plugin-chart-echarts): use scale for truncating x-axis (#26269)
---
 .../src/MixedTimeseries/controlPanel.tsx           |   9 +-
 .../src/MixedTimeseries/transformProps.ts          |  52 +++++++----
 .../src/MixedTimeseries/types.ts                   |   1 +
 .../src/Timeseries/transformProps.ts               |   8 +-
 .../plugin-chart-echarts/src/utils/controls.ts     |   1 -
 .../plugin-chart-echarts/src/utils/series.ts       |  40 ++++++--
 .../plugin-chart-echarts/test/utils/series.test.ts | 101 +++++++++++++++++++--
 7 files changed, 174 insertions(+), 38 deletions(-)

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx
index ec2443bb60..8161b32167 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx
@@ -32,7 +32,12 @@ import {
 
 import { DEFAULT_FORM_DATA } from './types';
 import { EchartsTimeseriesSeriesType } from '../Timeseries/types';
-import { legendSection, richTooltipSection } from '../controls';
+import {
+  legendSection,
+  richTooltipSection,
+  truncateXAxis,
+  xAxisBounds,
+} from '../controls';
 
 const {
   area,
@@ -350,6 +355,8 @@ const config: ControlPanelConfig = {
             },
           },
         ],
+        [truncateXAxis],
+        [xAxisBounds],
         [
           {
             name: 'truncateYAxis',
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
index 8e5a230fcf..763bbd1f95 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -20,32 +20,32 @@
 import { invert } from 'lodash';
 import {
   AnnotationLayer,
+  buildCustomFormatters,
   CategoricalColorNamespace,
+  CurrencyFormatter,
+  ensureIsArray,
   GenericDataType,
+  getCustomFormatter,
   getNumberFormatter,
+  getXAxisLabel,
+  isDefined,
   isEventAnnotationLayer,
   isFormulaAnnotationLayer,
   isIntervalAnnotationLayer,
+  isPhysicalColumn,
   isTimeseriesAnnotationLayer,
   QueryFormData,
+  QueryFormMetric,
   TimeseriesChartDataResponseResult,
   TimeseriesDataRecord,
-  getXAxisLabel,
-  isPhysicalColumn,
-  isDefined,
-  ensureIsArray,
-  buildCustomFormatters,
   ValueFormatter,
-  QueryFormMetric,
-  getCustomFormatter,
-  CurrencyFormatter,
 } from '@superset-ui/core';
 import { getOriginalSeries } from '@superset-ui/chart-controls';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
 import {
   DEFAULT_FORM_DATA,
-  EchartsMixedTimeseriesFormData,
   EchartsMixedTimeseriesChartTransformedProps,
+  EchartsMixedTimeseriesFormData,
   EchartsMixedTimeseriesProps,
 } from './types';
 import {
@@ -55,14 +55,15 @@ import {
 } from '../types';
 import { parseAxisBound } from '../utils/controls';
 import {
-  getOverMaxHiddenFormatter,
   dedupSeries,
+  extractDataTotalValues,
   extractSeries,
+  extractShowValueIndexes,
   getAxisType,
   getColtypesMapping,
   getLegendProps,
-  extractDataTotalValues,
-  extractShowValueIndexes,
+  getMinAndMaxFromBounds,
+  getOverMaxHiddenFormatter,
 } from '../utils/series';
 import {
   extractAnnotationLabels,
@@ -86,7 +87,7 @@ import {
   transformSeries,
   transformTimeseriesAnnotation,
 } from '../Timeseries/transformers';
-import { TIMESERIES_CONSTANTS, TIMEGRAIN_TO_TIMESTAMP } from '../constants';
+import { TIMEGRAIN_TO_TIMESTAMP, TIMESERIES_CONSTANTS } from '../constants';
 import { getDefaultTooltip } from '../utils/tooltip';
 import { getYAxisFormatter } from '../utils/getYAxisFormatter';
 
@@ -164,6 +165,7 @@ export default function transformProps(
     showValueB,
     stack,
     stackB,
+    truncateXAxis,
     truncateYAxis,
     tooltipTimeFormat,
     yAxisFormat,
@@ -179,6 +181,7 @@ export default function transformProps(
     zoomable,
     richTooltip,
     tooltipSortByMetric,
+    xAxisBounds,
     xAxisLabelRotation,
     groupby,
     groupbyB,
@@ -343,7 +346,8 @@ export default function transformProps(
     });
 
   // yAxisBounds need to be parsed to replace incompatible values with undefined
-  let [min, max] = (yAxisBounds || []).map(parseAxisBound);
+  const [xAxisMin, xAxisMax] = (xAxisBounds || []).map(parseAxisBound);
+  let [yAxisMin, yAxisMax] = (yAxisBounds || []).map(parseAxisBound);
   let [minSecondary, maxSecondary] = (yAxisBoundsSecondary || []).map(
     parseAxisBound,
   );
@@ -384,7 +388,7 @@ export default function transformProps(
         formatter:
           seriesType === EchartsTimeseriesSeriesType.Bar
             ? getOverMaxHiddenFormatter({
-                max,
+                max: yAxisMax,
                 formatter: seriesFormatter,
               })
             : seriesFormatter,
@@ -445,8 +449,8 @@ export default function transformProps(
 
   // default to 0-100% range when doing row-level contribution chart
   if (contributionMode === 'row' && stack) {
-    if (min === undefined) min = 0;
-    if (max === undefined) max = 1;
+    if (yAxisMin === undefined) yAxisMin = 0;
+    if (yAxisMax === undefined) yAxisMax = 1;
     if (minSecondary === undefined) minSecondary = 0;
     if (maxSecondary === undefined) maxSecondary = 1;
   }
@@ -497,13 +501,23 @@ export default function transformProps(
         xAxisType === 'time' && timeGrainSqla
           ? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
           : 0,
+      ...getMinAndMaxFromBounds(
+        xAxisType,
+        truncateXAxis,
+        xAxisMin,
+        xAxisMax,
+        seriesType === EchartsTimeseriesSeriesType.Bar ||
+          seriesTypeB === EchartsTimeseriesSeriesType.Bar
+          ? EchartsTimeseriesSeriesType.Bar
+          : undefined,
+      ),
     },
     yAxis: [
       {
         ...defaultYAxis,
         type: logAxis ? 'log' : 'value',
-        min,
-        max,
+        min: yAxisMin,
+        max: yAxisMax,
         minorTick: { show: true },
         minorSplitLine: { show: minorSplitLine },
         axisLabel: {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/types.ts
index 30969ae367..2e9ba641aa 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/types.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/types.ts
@@ -104,6 +104,7 @@ export const DEFAULT_FORM_DATA: EchartsMixedTimeseriesFormData = {
   yAxisFormatSecondary: TIMESERIES_DEFAULTS.yAxisFormat,
   yAxisTitleSecondary: DEFAULT_TITLE_FORM_DATA.yAxisTitle,
   tooltipTimeFormat: TIMESERIES_DEFAULTS.tooltipTimeFormat,
+  xAxisBounds: TIMESERIES_DEFAULTS.xAxisBounds,
   xAxisTimeFormat: TIMESERIES_DEFAULTS.xAxisTimeFormat,
   area: TIMESERIES_DEFAULTS.area,
   areaB: TIMESERIES_DEFAULTS.area,
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index 02abd23c1a..cc89ff30c7 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -458,7 +458,13 @@ export default function transformProps(
       xAxisType === AxisType.time && timeGrainSqla
         ? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
         : 0,
-    ...getMinAndMaxFromBounds(xAxisType, truncateXAxis, xAxisMin, xAxisMax),
+    ...getMinAndMaxFromBounds(
+      xAxisType,
+      truncateXAxis,
+      xAxisMin,
+      xAxisMax,
+      seriesType,
+    ),
   };
 
   let yAxis: any = {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/controls.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/controls.ts
index e5ba4c95d1..8ec2b3d13f 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/controls.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/controls.ts
@@ -19,7 +19,6 @@
 
 import { validateNumber } from '@superset-ui/core';
 
-// eslint-disable-next-line import/prefer-default-export
 export function parseAxisBound(
   bound?: string | number | null,
 ): number | undefined {
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index 69c0ccbe1b..a294fa44c3 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -41,7 +41,12 @@ import {
   StackControlsValue,
   TIMESERIES_CONSTANTS,
 } from '../constants';
-import { LegendOrientation, LegendType, StackType } from '../types';
+import {
+  EchartsTimeseriesSeriesType,
+  LegendOrientation,
+  LegendType,
+  StackType,
+} from '../types';
 import { defaultLegendPadding } from '../defaults';
 
 function isDefined<T>(value: T | undefined | null): boolean {
@@ -547,16 +552,35 @@ export function calculateLowerLogTick(minPositiveValue: number) {
   return Math.pow(10, logBase10);
 }
 
+type BoundsType = {
+  min?: number | 'dataMin';
+  max?: number | 'dataMax';
+  scale?: true;
+};
+
 export function getMinAndMaxFromBounds(
   axisType: AxisType,
   truncateAxis: boolean,
   min?: number,
   max?: number,
-): { min: number | 'dataMin'; max: number | 'dataMax' } | {} {
-  return truncateAxis && axisType === AxisType.value
-    ? {
-        min: min === undefined ? 'dataMin' : min,
-        max: max === undefined ? 'dataMax' : max,
-      }
-    : {};
+  seriesType?: EchartsTimeseriesSeriesType,
+): BoundsType | {} {
+  if (axisType === AxisType.value && truncateAxis) {
+    const ret: BoundsType = {};
+    if (seriesType === EchartsTimeseriesSeriesType.Bar) {
+      ret.scale = true;
+    }
+    if (min !== undefined) {
+      ret.min = min;
+    } else if (seriesType !== EchartsTimeseriesSeriesType.Bar) {
+      ret.min = 'dataMin';
+    }
+    if (max !== undefined) {
+      ret.max = max;
+    } else if (seriesType !== EchartsTimeseriesSeriesType.Bar) {
+      ret.max = 'dataMax';
+    }
+    return ret;
+  }
+  return {};
 }
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index b309bf6f3c..8e55b125b8 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -41,7 +41,11 @@ import {
   sortAndFilterSeries,
   sortRows,
 } from '../../src/utils/series';
-import { LegendOrientation, LegendType } from '../../src/types';
+import {
+  EchartsTimeseriesSeriesType,
+  LegendOrientation,
+  LegendType,
+} from '../../src/types';
 import { defaultLegendPadding } from '../../src/defaults';
 import { NULL_STRING } from '../../src/constants';
 
@@ -885,28 +889,109 @@ test('getAxisType', () => {
 });
 
 test('getMinAndMaxFromBounds returns empty object when not truncating', () => {
-  expect(getMinAndMaxFromBounds(AxisType.value, false, 10, 100)).toEqual({});
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.value,
+      false,
+      10,
+      100,
+      EchartsTimeseriesSeriesType.Bar,
+    ),
+  ).toEqual({});
+});
+
+test('getMinAndMaxFromBounds returns empty object for categorical axis', () => {
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.category,
+      false,
+      10,
+      100,
+      EchartsTimeseriesSeriesType.Bar,
+    ),
+  ).toEqual({});
+});
+
+test('getMinAndMaxFromBounds returns empty object for time axis', () => {
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.time,
+      false,
+      10,
+      100,
+      EchartsTimeseriesSeriesType.Bar,
+    ),
+  ).toEqual({});
 });
 
-test('getMinAndMaxFromBounds returns automatic bounds when truncating', () => {
+test('getMinAndMaxFromBounds returns dataMin/dataMax for non-bar charts', () => {
   expect(
-    getMinAndMaxFromBounds(AxisType.value, true, undefined, undefined),
+    getMinAndMaxFromBounds(
+      AxisType.value,
+      true,
+      undefined,
+      undefined,
+      EchartsTimeseriesSeriesType.Line,
+    ),
   ).toEqual({
     min: 'dataMin',
     max: 'dataMax',
   });
 });
 
-test('getMinAndMaxFromBounds returns automatic upper bound when truncating', () => {
-  expect(getMinAndMaxFromBounds(AxisType.value, true, 10, undefined)).toEqual({
+test('getMinAndMaxFromBounds returns bound without scale for non-bar charts', () => {
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.value,
+      true,
+      10,
+      undefined,
+      EchartsTimeseriesSeriesType.Line,
+    ),
+  ).toEqual({
     min: 10,
     max: 'dataMax',
   });
 });
 
+test('getMinAndMaxFromBounds returns scale when truncating without bounds', () => {
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.value,
+      true,
+      undefined,
+      undefined,
+      EchartsTimeseriesSeriesType.Bar,
+    ),
+  ).toEqual({ scale: true });
+});
+
+test('getMinAndMaxFromBounds returns automatic upper bound when truncating', () => {
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.value,
+      true,
+      10,
+      undefined,
+      EchartsTimeseriesSeriesType.Bar,
+    ),
+  ).toEqual({
+    min: 10,
+    scale: true,
+  });
+});
+
 test('getMinAndMaxFromBounds returns automatic lower bound when truncating', () => {
-  expect(getMinAndMaxFromBounds(AxisType.value, true, undefined, 100)).toEqual({
-    min: 'dataMin',
+  expect(
+    getMinAndMaxFromBounds(
+      AxisType.value,
+      true,
+      undefined,
+      100,
+      EchartsTimeseriesSeriesType.Bar,
+    ),
+  ).toEqual({
     max: 100,
+    scale: true,
   });
 });