fix: single y axis bounds (#148)

apache · Nov 26, 2021 · e5d1e7c · e5d1e7c
1 parent fa11638
commit e5d1e7c
Show file tree

Hide file tree

Showing 5 changed files with 199 additions and 14 deletions.
diff --git a/.../plugins/superset-ui-plugins/packages/superset-ui-legacy-preset-chart-nvd3/src/NVD3Vis.js b/.../plugins/superset-ui-plugins/packages/superset-ui-legacy-preset-chart-nvd3/src/NVD3Vis.js
@@ -36,6 +36,8 @@ import isTruthy from './utils/isTruthy';
 import {
   cleanColorInput,
   computeBarChartWidth,
+  computeYDomain,
+  computeStackedYDomain,
   drawBarValues,
   generateBubbleTooltipContent,
   generateMultiLineTooltipContent,
@@ -440,7 +442,7 @@ function nvd3Vis(element, props) {
 
     if (showBarValue) {
       drawBarValues(svg, data, isBarStacked, yAxisFormat);
-      chart.dispatch.on('stateChange', () => {
+      chart.dispatch.on('stateChange.drawBarValues', () => {
         drawBarValues(svg, data, isBarStacked, yAxisFormat);
       });
     }
@@ -607,16 +609,52 @@ function nvd3Vis(element, props) {
       xTicks.selectAll('text').attr('dx', -6.5);
     }
 
-    // Apply y-axis bounds
-    if (chart.yDomain && Array.isArray(yAxisBounds) && yAxisBounds.length === 2) {
-      const [min, max] = yAxisBounds;
-      const hasCustomMin = isDefined(min) && !Number.isNaN(min);
-      const hasCustomMax = isDefined(max) && !Number.isNaN(max);
-      if (hasCustomMin && hasCustomMax) {
-        chart.yDomain([min, max]);
-        chart.clipEdge(true);
+    const applyYAxisBounds = () => {
+      if (chart.yDomain && Array.isArray(yAxisBounds) && yAxisBounds.length === 2) {
+        const [customMin, customMax] = yAxisBounds;
+        const hasCustomMin = isDefined(customMin) && !Number.isNaN(customMin);
+        const hasCustomMax = isDefined(customMax) && !Number.isNaN(customMax);
+
+        if ((hasCustomMin || hasCustomMax) && vizType === 'area' && chart.style() === 'expand') {
+          // Because there are custom bounds, we need to override them back to 0%-100% since this
+          // is an expanded area chart
+          chart.yDomain([0, 1]);
+        } else if (
+          (hasCustomMin || hasCustomMax) &&
+          vizType === 'area' &&
+          chart.style() === 'stream'
+        ) {
+          // Because there are custom bounds, we need to override them back to the domain of the
+          // data since this is a stream area chart
+          chart.yDomain(computeStackedYDomain(data));
+        } else if (hasCustomMin && hasCustomMax) {
+          // Override the y domain if there's both a custom min and max
+          chart.yDomain([customMin, customMax]);
+          chart.clipEdge(true);
+        } else if (hasCustomMin || hasCustomMax) {
+          // Only one of the bounds has been set, so we need to manually calculate the other one
+          let [trueMin, trueMax] = [0, 1];
+
+          // These viz types can be stacked
+          // They correspond to the nvd3 stackedAreaChart and multiBarChart
+          if (vizType === 'area' || (isVizTypes(['bar', 'dist_bar']) && chart.stacked())) {
+            // This is a stacked area chart or a stacked bar chart
+            [trueMin, trueMax] = computeStackedYDomain(data);
+          } else {
+            [trueMin, trueMax] = computeYDomain(data);
+          }
+
+          const min = hasCustomMin ? customMin : trueMin;
+          const max = hasCustomMax ? customMax : trueMax;
+          chart.yDomain([min, max]);
+          chart.clipEdge(true);
+        }
       }
-    }
+    };
+    applyYAxisBounds();
+
+    // Also reapply on each state change to account for enabled/disabled series
+    chart.dispatch.on('stateChange.applyYAxisBounds', applyYAxisBounds);
 
     // align yAxis1 and yAxis2 ticks
     if (isVizTypes(['dual_line', 'line_multi'])) {
@@ -659,7 +697,7 @@ function nvd3Vis(element, props) {
 
       // redo on legend toggle; nvd3 calls the callback *before* the line is
       // drawn, so we need to add a small delay here
-      chart.dispatch.on('stateChange', () => {
+      chart.dispatch.on('stateChange.showMarkers', () => {
         setTimeout(() => {
           svg
             .selectAll('.nv-point')

diff --git a/...ui/plugins/superset-ui-plugins/packages/superset-ui-legacy-preset-chart-nvd3/src/utils.js b/...ui/plugins/superset-ui-plugins/packages/superset-ui-legacy-preset-chart-nvd3/src/utils.js
@@ -362,7 +362,7 @@ export function setAxisShowMaxMin(axis, showminmax) {
 
 export function computeYDomain(data) {
   if (Array.isArray(data) && data.length > 0 && Array.isArray(data[0].values)) {
-    const extents = data.map(row => d3.extent(row.values, v => v.y));
+    const extents = data.filter(d => !d.disabled).map(row => d3.extent(row.values, v => v.y));
     const minOfMin = d3.min(extents, ([min]) => min);
     const maxOfMax = d3.max(extents, ([, max]) => max);
 
@@ -371,3 +371,14 @@ export function computeYDomain(data) {
 
   return [0, 1];
 }
+
+export function computeStackedYDomain(data) {
+  if (Array.isArray(data) && data.length > 0 && Array.isArray(data[0].values)) {
+    const series = data.filter(d => !d.disabled).map(d => d.values.map(v => v.y));
+    const stackedValues = series[0].map((_, i) => series.reduce((acc, cur) => acc + cur[i], 0));
+
+    return [Math.min(0, ...stackedValues), Math.max(0, ...stackedValues)];
+  }
+
+  return [0, 1];
+}
diff --git a/...gins/superset-ui-plugins/packages/superset-ui-legacy-preset-chart-nvd3/test/utils.test.js b/...gins/superset-ui-plugins/packages/superset-ui-legacy-preset-chart-nvd3/test/utils.test.js
@@ -16,7 +16,99 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { getTimeOrNumberFormatter, formatLabel, tryNumify } from '../src/utils';
+import {
+  computeStackedYDomain,
+  computeYDomain,
+  getTimeOrNumberFormatter,
+  formatLabel,
+  tryNumify,
+} from '../src/utils';
+
+const DATA = [
+  {
+    key: ['East Asia & Pacific'],
+    values: [
+      {
+        x: -315619200000.0,
+        y: 1031863394.0,
+      },
+      {
+        x: -283996800000.0,
+        y: 1034767718.0,
+      },
+    ],
+  },
+  {
+    key: ['South Asia'],
+    values: [
+      {
+        x: -315619200000.0,
+        y: 572036107.0,
+      },
+      {
+        x: -283996800000.0,
+        y: 584143236.0,
+      },
+    ],
+  },
+  {
+    key: ['Europe & Central Asia'],
+    values: [
+      {
+        x: -315619200000.0,
+        y: 660881033.0,
+      },
+      {
+        x: -283996800000.0,
+        y: 668526708.0,
+      },
+    ],
+  },
+];
+
+const DATA_WITH_DISABLED_SERIES = [
+  {
+    disabled: true,
+    key: ['East Asia & Pacific'],
+    values: [
+      {
+        x: -315619200000.0,
+        y: 1031863394.0,
+      },
+      {
+        x: -283996800000.0,
+        y: 1034767718.0,
+      },
+    ],
+  },
+  {
+    disabled: true,
+    key: ['South Asia'],
+    values: [
+      {
+        x: -315619200000.0,
+        y: 572036107.0,
+      },
+      {
+        x: -283996800000.0,
+        y: 584143236.0,
+      },
+    ],
+  },
+  {
+    key: ['Europe & Central Asia'],
+    values: [
+      {
+        x: -315619200000.0,
+        y: 660881033.0,
+      },
+      {
+        x: -283996800000.0,
+        y: 668526708.0,
+      },
+    ],
+  },
+];
 
 describe('nvd3/utils', () => {
   describe('getTimeOrNumberFormatter(format)', () => {
@@ -66,4 +158,32 @@ describe('nvd3/utils', () => {
       expect(tryNumify('a string')).toBe('a string');
     });
   });
+
+  describe('computeYDomain()', () => {
+    it('works with invalid data', () => {
+      expect(computeYDomain('foo')).toEqual([0, 1]);
+    });
+
+    it('works with all series enabled', () => {
+      expect(computeYDomain(DATA)).toEqual([572036107.0, 1034767718.0]);
+    });
+
+    it('works with some series disabled', () => {
+      expect(computeYDomain(DATA_WITH_DISABLED_SERIES)).toEqual([660881033.0, 668526708.0]);
+    });
+  });
+
+  describe('computeStackedYDomain()', () => {
+    it('works with invalid data', () => {
+      expect(computeStackedYDomain('foo')).toEqual([0, 1]);
+    });
+
+    it('works with all series enabled', () => {
+      expect(computeStackedYDomain(DATA)).toEqual([0, 2287437662.0]);
+    });
+
+    it('works with some series disabled', () => {
+      expect(computeStackedYDomain(DATA_WITH_DISABLED_SERIES)).toEqual([0, 668526708.0]);
+    });
+  });
 });
diff --git a/...ages/superset-ui-plugins-demo/storybook/stories/legacy-preset-chart-nvd3/Area/Stories.jsx b/...ages/superset-ui-plugins-demo/storybook/stories/legacy-preset-chart-nvd3/Area/Stories.jsx
@@ -97,7 +97,7 @@ export default [
             metrics: ['sum__SP_POP_TOTL'],
             richTooltip: true,
             showBrush: 'auto',
-            showControls: false,
+            showControls: true,
             showLegend: true,
             stackedStyle: 'stack',
             vizType: 'area',

diff --git a/...superset-ui-plugins-demo/storybook/stories/legacy-preset-chart-nvd3/Line/YAxisStories.jsx b/...superset-ui-plugins-demo/storybook/stories/legacy-preset-chart-nvd3/Line/YAxisStories.jsx
@@ -71,6 +71,22 @@ export default [
             width: 400,
           }}
         />
+        <pre>yAxisBounds=[40000, null] with Legend</pre>
+        <SuperChart
+          chartType="line"
+          chartProps={{
+            datasource: { verboseMap: {} },
+            formData: {
+              richTooltip: true,
+              showLegend: true,
+              vizType: 'line',
+              yAxisBounds: [40000, null],
+            },
+            height: 200,
+            payload: { data },
+            width: 400,
+          }}
+        />
       </div>
     ),
     storyName: 'yAxisBounds',