fix(charts): Hide Values greater than Max Y Axis Bound on Mixed Time …

…Series with Bar series (#21015) * Mixed TimeSeries: - When Bar chart is used as serie type, we need to hide values that are greater than the max Y Axis Bound. * Mixed Time Series: - Simplify logic for getOverMaxHiddenFormatter * Mixed Time Series: - Add tests for new getOverMaxHiddenFormatter util func
apache · Aug 22, 2022 · bdcc0a9 · bdcc0a9
1 parent d44202f
commit bdcc0a9
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 46 deletions.
diff --git a/superset-frontend/packages/superset-ui-core/src/number-format/NumberFormats.ts b/superset-frontend/packages/superset-ui-core/src/number-format/NumberFormats.ts
@@ -52,6 +52,7 @@ const SI = SI_3_DIGIT;
 
 const SMART_NUMBER = 'SMART_NUMBER';
 const SMART_NUMBER_SIGNED = 'SMART_NUMBER_SIGNED';
+const OVER_MAX_HIDDEN = 'OVER_MAX_HIDDEN';
 
 const NumberFormats = {
   DOLLAR,
@@ -82,6 +83,7 @@ const NumberFormats = {
   SI_3_DIGIT,
   SMART_NUMBER,
   SMART_NUMBER_SIGNED,
+  OVER_MAX_HIDDEN,
 };
 
 export default NumberFormats;
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -38,9 +38,10 @@ import {
   EchartsMixedTimeseriesChartTransformedProps,
   EchartsMixedTimeseriesProps,
 } from './types';
-import { ForecastSeriesEnum } from '../types';
+import { EchartsTimeseriesSeriesType, ForecastSeriesEnum } from '../types';
 import { parseYAxisBound } from '../utils/controls';
 import {
+  getOverMaxHiddenFormatter,
   currentSeries,
   dedupSeries,
   extractSeries,
@@ -210,51 +211,6 @@ export default function transformProps(
     percentageThreshold,
     xAxisCol,
   });
-  rawSeriesA.forEach(entry => {
-    const transformedSeries = transformSeries(entry, colorScale, {
-      area,
-      markerEnabled,
-      markerSize,
-      areaOpacity: opacity,
-      seriesType,
-      showValue,
-      stack: Boolean(stack),
-      yAxisIndex,
-      filterState,
-      seriesKey: entry.name,
-      sliceId,
-      queryIndex: 0,
-      formatter,
-      showValueIndexes: showValueIndexesA,
-      totalStackedValues,
-      thresholdValues,
-    });
-    if (transformedSeries) series.push(transformedSeries);
-  });
-
-  rawSeriesB.forEach(entry => {
-    const transformedSeries = transformSeries(entry, colorScale, {
-      area: areaB,
-      markerEnabled: markerEnabledB,
-      markerSize: markerSizeB,
-      areaOpacity: opacityB,
-      seriesType: seriesTypeB,
-      showValue: showValueB,
-      stack: Boolean(stackB),
-      yAxisIndex: yAxisIndexB,
-      filterState,
-      seriesKey: primarySeries.has(entry.name as string)
-        ? `${entry.name} (1)`
-        : entry.name,
-      sliceId,
-      queryIndex: 1,
-      formatter: formatterSecondary,
-      showValueIndexes: showValueIndexesB,
-      totalStackedValues: totalStackedValuesB,
-      thresholdValues: thresholdValuesB,
-    });
-    if (transformedSeries) series.push(transformedSeries);
-  });
 
   annotationLayers
     .filter((layer: AnnotationLayer) => layer.show)
@@ -309,6 +265,64 @@ export default function transformProps(
   // yAxisBounds need to be parsed to replace incompatible values with undefined
   let [min, max] = (yAxisBounds || []).map(parseYAxisBound);
 
+  const maxLabelFormatter = getOverMaxHiddenFormatter({ max, formatter });
+  const maxLabelFormatterSecondary = getOverMaxHiddenFormatter({
+    max,
+    formatter: formatterSecondary,
+  });
+
+  rawSeriesA.forEach(entry => {
+    const transformedSeries = transformSeries(entry, colorScale, {
+      area,
+      markerEnabled,
+      markerSize,
+      areaOpacity: opacity,
+      seriesType,
+      showValue,
+      stack: Boolean(stack),
+      yAxisIndex,
+      filterState,
+      seriesKey: entry.name,
+      sliceId,
+      queryIndex: 0,
+      formatter:
+        seriesType === EchartsTimeseriesSeriesType.Bar
+          ? maxLabelFormatter
+          : formatter,
+      showValueIndexes: showValueIndexesA,
+      totalStackedValues,
+      thresholdValues,
+    });
+    if (transformedSeries) series.push(transformedSeries);
+  });
+
+  rawSeriesB.forEach(entry => {
+    const transformedSeries = transformSeries(entry, colorScale, {
+      area: areaB,
+      markerEnabled: markerEnabledB,
+      markerSize: markerSizeB,
+      areaOpacity: opacityB,
+      seriesType: seriesTypeB,
+      showValue: showValueB,
+      stack: Boolean(stackB),
+      yAxisIndex: yAxisIndexB,
+      filterState,
+      seriesKey: primarySeries.has(entry.name as string)
+        ? `${entry.name} (1)`
+        : entry.name,
+      sliceId,
+      queryIndex: 1,
+      formatter:
+        seriesTypeB === EchartsTimeseriesSeriesType.Bar
+          ? maxLabelFormatterSecondary
+          : formatterSecondary,
+      showValueIndexes: showValueIndexesB,
+      totalStackedValues: totalStackedValuesB,
+      thresholdValues: thresholdValuesB,
+    });
+    if (transformedSeries) series.push(transformedSeries);
+  });
+
   // default to 0-100% range when doing row-level contribution chart
   if (contributionMode === 'row' && stack) {
     if (min === undefined) min = 0;

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -24,6 +24,7 @@ import {
   DTTM_ALIAS,
   ensureIsArray,
   GenericDataType,
+  NumberFormats,
   NumberFormatter,
   TimeFormatter,
 } from '@superset-ui/core';
@@ -326,3 +327,24 @@ export function getAxisType(dataType?: GenericDataType): AxisType {
   }
   return 'category';
 }
+
+export function getOverMaxHiddenFormatter(
+  config: {
+    max?: number;
+    formatter?: NumberFormatter;
+  } = {},
+) {
+  const { max, formatter } = config;
+  // Only apply this logic if there's a MAX set in the controls
+  const shouldHideIfOverMax = !!max || max === 0;
+
+  return new NumberFormatter({
+    formatFunc: value =>
+      `${
+        shouldHideIfOverMax && value > max
+          ? ''
+          : formatter?.format(value) || value
+      }`,
+    id: NumberFormats.OVER_MAX_HIDDEN,
+  });
+}
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
@@ -26,6 +26,7 @@ import {
   getLegendProps,
   sanitizeHtml,
   extractShowValueIndexes,
+  getOverMaxHiddenFormatter,
 } from '../../src/utils/series';
 import { LegendOrientation, LegendType } from '../../src/types';
 import { defaultLegendPadding } from '../../src/defaults';
@@ -536,4 +537,16 @@ describe('formatSeriesName', () => {
       expect(sanitizeHtml(NULL_STRING)).toEqual('&lt;NULL&gt;');
     });
   });
+
+  describe('getOverMaxHiddenFormatter', () => {
+    it('should hide value if greater than max', () => {
+      const formatter = getOverMaxHiddenFormatter({ max: 81000 });
+      expect(formatter.format(84500)).toEqual('');
+    });
+    it('should show value if less or equal than max', () => {
+      const formatter = getOverMaxHiddenFormatter({ max: 81000 });
+      expect(formatter.format(81000)).toEqual('81000');
+      expect(formatter.format(50000)).toEqual('50000');
+    });
+  });
 });