fix(xy): stacked polarity (elastic#1502)

Resolves issue with stacking a mix of negative and positive bars and areas for any `StackMode`. Key changes include: - Disallowing stacked band charts (i.e. using `stackAccessors` with `y0Accessors`). Now shows `console.warn` when used and ignores `y0Accessor` for rendering but still adds to `initialY0` value. - Blocks showing banded legend and tooltip values for banded stacks, see case above. - Prevents percentage domain constraining when no `stackAccessors` are specified. fix elastic#1280 # Conflicts: # packages/charts/src/chart_types/xy_chart/rendering/area.ts
nickofthyme · Dec 9, 2021 · 4af9c8a · 4af9c8a
1 parent e77c2dc
commit 4af9c8a
Show file tree

Hide file tree

Showing 76 changed files with 586 additions and 301 deletions.
diff --git a/.gitignore b/.gitignore
@@ -19,5 +19,6 @@ coverage/
 npm-debug.log
 yarn-error.log
 playground/playground.tsx
+.jest-image-snapshot-touched-files
 
 **/__diff_output__/
diff --git a/NOTICE.txt b/NOTICE.txt
@@ -43,7 +43,7 @@ THIS SOFTWARE.
 
 ---
 
-This product includes code that is adapted from d3-array@3.0.4 and d3-scale@4.0.2,
+This product includes code that is adapted from d3-shape@3.0.1, d3-array@3.0.4 and d3-scale@4.0.2,
 which are both available under a "ISC" license.
 
 ISC License

diff --git a/integration/page_objects/common.ts b/integration/page_objects/common.ts
@@ -362,9 +362,12 @@ class CommonPage {
   async expectChartWithMouseAtUrlToMatchScreenshot(
     url: string,
     mousePosition: MousePosition,
-    options?: Omit<ScreenshotElementAtUrlOptions, 'action'>,
+    options?: ScreenshotElementAtUrlOptions,
   ) {
-    const action = async () => await this.moveMouseRelativeToDOMElement(mousePosition, this.chartSelector);
+    const action = async () => {
+      await options?.action?.();
+      await this.moveMouseRelativeToDOMElement(mousePosition, this.chartSelector);
+    };
     await this.expectChartAtUrlToMatchScreenshot(url, {
       ...options,
       action,

diff --git a/...tests-for-all-stories-area-chart-stacked-band-visually-looks-correct-1-snap.png b/...tests-for-all-stories-area-chart-stacked-band-visually-looks-correct-1-snap.png
diff --git a/...or-all-stories-mixed-charts-polarized-stacked-visually-looks-correct-1-snap.png b/...or-all-stories-mixed-charts-polarized-stacked-visually-looks-correct-1-snap.png
diff --git a/...le-to-extents-domain-fit-is-false-should-show-correct-extents-banded-1-snap.png b/...le-to-extents-domain-fit-is-false-should-show-correct-extents-banded-1-snap.png
diff --git a/...ale-to-extents-domain-fit-is-true-should-show-correct-extents-banded-1-snap.png b/...ale-to-extents-domain-fit-is-true-should-show-correct-extents-banded-1-snap.png
diff --git a/...xis-stories-test-ts-axis-stories-should-switch-to-a-30-minute-raster-1-snap.png b/...xis-stories-test-ts-axis-stories-should-switch-to-a-30-minute-raster-1-snap.png
diff --git a/...ercentage-polarity-mixed-area-series-should-display-correct-stacking-1-snap.png b/...ercentage-polarity-mixed-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...percentage-polarity-mixed-bar-series-should-display-correct-stacking-1-snap.png b/...percentage-polarity-mixed-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...entage-polarity-negative-area-series-should-display-correct-stacking-1-snap.png b/...entage-polarity-negative-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...centage-polarity-negative-bar-series-should-display-correct-stacking-1-snap.png b/...centage-polarity-negative-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...entage-polarity-positive-area-series-should-display-correct-stacking-1-snap.png b/...entage-polarity-positive-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...centage-polarity-positive-bar-series-should-display-correct-stacking-1-snap.png b/...centage-polarity-positive-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...ilhouette-polarity-mixed-area-series-should-display-correct-stacking-1-snap.png b/...ilhouette-polarity-mixed-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...silhouette-polarity-mixed-bar-series-should-display-correct-stacking-1-snap.png b/...silhouette-polarity-mixed-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...ouette-polarity-negative-area-series-should-display-correct-stacking-1-snap.png b/...ouette-polarity-negative-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...houette-polarity-negative-bar-series-should-display-correct-stacking-1-snap.png b/...houette-polarity-negative-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...ouette-polarity-positive-area-series-should-display-correct-stacking-1-snap.png b/...ouette-polarity-positive-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...houette-polarity-positive-bar-series-should-display-correct-stacking-1-snap.png b/...houette-polarity-positive-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...de-wiggle-polarity-mixed-area-series-should-display-correct-stacking-1-snap.png b/...de-wiggle-polarity-mixed-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...ode-wiggle-polarity-mixed-bar-series-should-display-correct-stacking-1-snap.png b/...ode-wiggle-polarity-mixed-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...wiggle-polarity-negative-area-series-should-display-correct-stacking-1-snap.png b/...wiggle-polarity-negative-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...-wiggle-polarity-negative-bar-series-should-display-correct-stacking-1-snap.png b/...-wiggle-polarity-negative-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...wiggle-polarity-positive-area-series-should-display-correct-stacking-1-snap.png b/...wiggle-polarity-positive-area-series-should-display-correct-stacking-1-snap.png
diff --git a/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...rea-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...-wiggle-polarity-positive-bar-series-should-display-correct-stacking-1-snap.png b/...-wiggle-polarity-positive-bar-series-should-display-correct-stacking-1-snap.png
diff --git a/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png b/...bar-series-should-show-area-chart-with-toggled-series-and-mouse-over-1-snap.png
diff --git a/...tation-should-render-marker-with-annotations-with-0-degree-rotations-1-snap.png b/...tation-should-render-marker-with-annotations-with-0-degree-rotations-1-snap.png
diff --git a/...tion-should-render-marker-with-annotations-with-180-degree-rotations-1-snap.png b/...tion-should-render-marker-with-annotations-with-180-degree-rotations-1-snap.png
diff --git a/...ation-should-render-marker-with-annotations-with-90-degree-rotations-1-snap.png b/...ation-should-render-marker-with-annotations-with-90-degree-rotations-1-snap.png
diff --git a/...uld-render-marker-with-annotations-with-negative-90-degree-rotations-1-snap.png b/...uld-render-marker-with-annotations-with-negative-90-degree-rotations-1-snap.png
diff --git a/...ould-not-display-tooltip-over-point-outside-of-domain-even-more-left-1-snap.png b/...ould-not-display-tooltip-over-point-outside-of-domain-even-more-left-1-snap.png
diff --git a/...ain-should-render-area-chart-with-points-outside-of-domain-correclty-1-snap.png b/...ain-should-render-area-chart-with-points-outside-of-domain-correclty-1-snap.png
diff --git a/...-chart-with-points-outside-of-the-domain-with-y-0-accessor-correctly-1-snap.png b/...-chart-with-points-outside-of-the-domain-with-y-0-accessor-correctly-1-snap.png
diff --git a/...ain-should-render-line-chart-with-points-outside-of-domain-correctly-1-snap.png b/...ain-should-render-line-chart-with-points-outside-of-domain-correctly-1-snap.png
diff --git a/integration/tests/area_stories.test.ts b/integration/tests/area_stories.test.ts
@@ -23,22 +23,6 @@ describe('Area series stories', () => {
     });
   });
 
-  describe('scale to extents', () => {
-    describe('domain.fit is true', () => {
-      const trueUrl = 'http://localhost:9001/?path=/story/area-chart--stacked-band&knob-fit Y domain=true';
-      it('should show correct extents - Banded', async () => {
-        await common.expectChartAtUrlToMatchScreenshot(trueUrl);
-      });
-    });
-
-    describe('domain.fit is false', () => {
-      const falseUrl = 'http://localhost:9001/?path=/story/area-chart--stacked-band&knob-fit Y domain=false';
-
-      it('should show correct extents - Banded', async () => {
-        await common.expectChartAtUrlToMatchScreenshot(falseUrl);
-      });
-    });
-  });
   describe('Non-Stacked Linear Area with discontinuous data points', () => {
     it('with fit', async () => {
       await common.expectChartAtUrlToMatchScreenshot(

diff --git a/integration/tests/mixed_stories.test.ts b/integration/tests/mixed_stories.test.ts
@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import { Fit } from '../../packages/charts/src';
+import { Fit, StackMode, SeriesType } from '../../packages/charts/src';
 import { common } from '../page_objects';
 
 describe('Mixed series stories', () => {
@@ -195,4 +195,27 @@ describe('Mixed series stories', () => {
       });
     });
   });
+
+  describe.each(Object.values(StackMode))('Stack mode - %s', (mode) => {
+    describe.each(['Mixed', 'Positive', 'Negative'])('Polarity - %s', (polarity) => {
+      describe.each([SeriesType.Bar, SeriesType.Area])('%s series', (type) => {
+        it('should display correct stacking', async () => {
+          await common.expectChartAtUrlToMatchScreenshot(
+            `http://localhost:9001/?path=/story/mixed-charts---polarized-stacked&globals=theme:light&knob-stacked=true&knob-data polarity=${polarity}&knob-custom domain=false&knob-stackMode=${mode}&knob-SeriesType=${type}`,
+          );
+        });
+
+        it('should show area chart with toggled series and mouse over', async () => {
+          const action = async () => {
+            await page.click('.echLegendItem:nth-child(2) .echLegendItem__label');
+          };
+          await common.expectChartWithMouseAtUrlToMatchScreenshot(
+            `http://localhost:9001/?path=/story/mixed-charts---polarized-stacked&globals=theme:light&knob-stacked=true&knob-data polarity=${polarity}&knob-custom domain=false&knob-stackMode=${mode}&knob-SeriesType=${type}`,
+            { top: 170, left: 490 },
+            { action },
+          );
+        });
+      });
+    });
+  });
 });
diff --git a/package.json b/package.json
@@ -86,7 +86,7 @@
     "@types/d3-collection": "^1.0.8",
     "@types/d3-interpolate": "^1.3.1",
     "@types/d3-scale": "^2.1.1",
-    "@types/d3-shape": "^1.3.1",
+    "@types/d3-shape": "^2.0.0",
     "@types/enzyme": "^3.9.0",
     "@types/enzyme-adapter-react-16": "^1.0.5",
     "@types/expect-puppeteer": "^4.4.5",

diff --git a/packages/charts/package.json b/packages/charts/package.json
@@ -40,7 +40,7 @@
     "d3-collection": "^1.0.7",
     "d3-interpolate": "^1.4.0",
     "d3-scale": "^1.0.7",
-    "d3-shape": "^1.3.4",
+    "d3-shape": "^2.0.0",
     "prop-types": "^15.7.2",
     "re-reselect": "^3.4.0",
     "react-redux": "^7.1.0",

diff --git a/packages/charts/src/chart_types/xy_chart/domains/y_domain.ts b/packages/charts/src/chart_types/xy_chart/domains/y_domain.ts
@@ -47,14 +47,14 @@ function mergeYDomainForGroup(
   const dataSeries = [...stacked, ...nonStacked];
   if (dataSeries.length === 0) return null;
 
-  const [{ stackMode, spec }] = dataSeries;
+  const [{ isStacked, stackMode, spec }] = dataSeries;
   const groupId = getSpecDomainGroupId(spec);
   const { customDomain, type, nice, desiredTickCount } = yScaleConfig[groupId];
   const newCustomDomain: YDomainRange = customDomain ? { ...customDomain } : { min: NaN, max: NaN };
   const { paddingUnit, padding, constrainPadding } = newCustomDomain;
 
   let mergedDomain: ContinuousDomain;
-  if (stackMode === StackMode.Percentage) {
+  if (isStacked && stackMode === StackMode.Percentage) {
     mergedDomain = computeContinuousDataDomain([0, 1], type, customDomain);
   } else {
     const stackedDomain = computeYDomain(stacked, hasZeroBaselineSpecs, type, newCustomDomain);

diff --git a/packages/charts/src/chart_types/xy_chart/legend/legend.ts b/packages/charts/src/chart_types/xy_chart/legend/legend.ts
@@ -27,7 +27,7 @@ import {
   getSeriesName,
   DataSeries,
   getSeriesKey,
-  isDataSeriesBanded,
+  isBandedSpec,
   getSeriesIdentifierFromDataSeries,
 } from '../utils/series';
 import {
@@ -111,7 +111,7 @@ export function computeLegend(
 
   dataSeries.forEach((series) => {
     const { specId, yAccessor } = series;
-    const banded = isDataSeriesBanded(series);
+    const banded = isBandedSpec(series.spec);
     const key = getSeriesKey(series, series.groupId);
     const spec = getSpecsById<BasicSeriesSpec>(specs, specId);
     const dataSeriesKey = getSeriesKey(

diff --git a/packages/charts/src/chart_types/xy_chart/rendering/area.ts b/packages/charts/src/chart_types/xy_chart/rendering/area.ts
@@ -36,7 +36,7 @@ export function renderArea(
   panel: Dimensions,
   color: Color,
   curve: CurveType,
-  hasY0Accessors: boolean,
+  isBandedSpec: boolean,
   xScaleOffset: number,
   seriesStyle: AreaSeriesStyle,
   markSizeOptions: MarkSizeOptions,
@@ -57,14 +57,14 @@ export function renderArea(
     .y1(y1Fn)
     .y0(y0Fn)
     .defined((datum) => {
-      return definedFn(datum, y1DatumAccessor) && (hasY0Accessors ? definedFn(datum, y0DatumAccessor) : true);
+      return definedFn(datum, y1DatumAccessor) && (isBandedSpec ? definedFn(datum, y0DatumAccessor) : true);
     })
     .curve(getCurveFactory(curve));
 
   const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset);
 
   const lines: string[] = [];
-  const y0Line = hasY0Accessors && pathGenerator.lineY0()(dataSeries.data);
+  const y0Line = isBandedSpec && pathGenerator.lineY0()(dataSeries.data);
   const y1Line = pathGenerator.lineY1()(dataSeries.data);
   if (y1Line) lines.push(y1Line);
   if (y0Line) lines.push(y0Line);
@@ -77,7 +77,7 @@ export function renderArea(
     panel,
     color,
     seriesStyle.point,
-    hasY0Accessors,
+    isBandedSpec,
     markSizeOptions,
     pointStyleAccessor,
     false,

diff --git a/packages/charts/src/chart_types/xy_chart/rendering/points.ts b/packages/charts/src/chart_types/xy_chart/rendering/points.ts
@@ -35,7 +35,7 @@ export function renderPoints(
   panel: Dimensions,
   color: Color,
   pointStyle: PointStyle,
-  hasY0Accessors: boolean,
+  isBandChart: boolean,
   markSizeOptions: MarkSizeOptions,
   styleAccessor?: PointStyleAccessor,
   spatial = false,
@@ -66,12 +66,12 @@ export function renderPoints(
     if (Number.isNaN(x)) return acc;
 
     const points: PointGeometry[] = [];
-    const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = hasY0Accessors ? ['y0', 'y1'] : ['y1'];
+    const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = isBandChart ? ['y0', 'y1'] : ['y1'];
 
     yDatumKeyNames.forEach((yDatumKeyName, keyIndex) => {
       const valueAccessor = getYDatumValueFn(yDatumKeyName);
       const y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum);
-      const originalY = getDatumYValue(datum, keyIndex === 0, hasY0Accessors, dataSeries.stackMode);
+      const originalY = getDatumYValue(datum, keyIndex === 0, isBandChart, dataSeries.stackMode);
       const seriesIdentifier: XYChartSeriesIdentifier = {
         key: dataSeries.key,
         specId: dataSeries.specId,
@@ -98,7 +98,7 @@ export function renderPoints(
           x: xValue,
           y: originalY,
           mark,
-          accessor: hasY0Accessors && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
+          accessor: isBandChart && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
           datum: datum.datum,
         },
         transform: {

diff --git a/packages/charts/src/chart_types/xy_chart/state/utils/utils.ts b/packages/charts/src/chart_types/xy_chart/state/utils/utils.ts
@@ -37,7 +37,13 @@ import { fillSeries } from '../../utils/fill_series';
 import { groupBy } from '../../utils/group_data_series';
 import { IndexedGeometryMap } from '../../utils/indexed_geometry_map';
 import { computeXScale, computeYScales } from '../../utils/scales';
-import { DataSeries, getDataSeriesFromSpecs, getFormattedDataSeries, getSeriesKey } from '../../utils/series';
+import {
+  DataSeries,
+  getDataSeriesFromSpecs,
+  getFormattedDataSeries,
+  getSeriesKey,
+  isBandedSpec,
+} from '../../utils/series';
 import {
   AxisSpec,
   BasicSeriesSpec,
@@ -46,7 +52,6 @@ import {
   HistogramModeAlignment,
   HistogramModeAlignments,
   isAreaSeriesSpec,
-  isBandedSpec,
   isBarSeriesSpec,
   isBubbleSeriesSpec,
   isLineSeriesSpec,
@@ -384,7 +389,7 @@ function renderGeometries(
         yScale,
         color,
         panel,
-        isBandedSpec(spec.y0Accessors),
+        isBandedSpec(spec),
         xScaleOffset,
         bubbleSeriesStyle,
         {
@@ -418,7 +423,7 @@ function renderGeometries(
         panel,
         color,
         spec.curve || CurveType.LINEAR,
-        isBandedSpec(spec.y0Accessors),
+        isBandedSpec(spec),
         xScaleOffset,
         lineSeriesStyle,
         {
@@ -451,7 +456,7 @@ function renderGeometries(
         panel,
         color,
         spec.curve || CurveType.LINEAR,
-        isBandedSpec(spec.y0Accessors),
+        isBandedSpec(spec),
         xScaleOffset,
         areaSeriesStyle,
         {

diff --git a/packages/charts/src/chart_types/xy_chart/tooltip/tooltip.ts b/packages/charts/src/chart_types/xy_chart/tooltip/tooltip.ts
@@ -13,15 +13,8 @@ import { getAccessorFormatLabel } from '../../../utils/accessor';
 import { isDefined } from '../../../utils/common';
 import { BandedAccessorType, IndexedGeometry } from '../../../utils/geometry';
 import { defaultTickFormatter } from '../utils/axis_utils';
-import { getSeriesName } from '../utils/series';
-import {
-  AxisSpec,
-  BasicSeriesSpec,
-  isAreaSeriesSpec,
-  isBandedSpec,
-  isBarSeriesSpec,
-  TickFormatterOptions,
-} from '../utils/specs';
+import { getSeriesName, isBandedSpec } from '../utils/series';
+import { AxisSpec, BasicSeriesSpec, isAreaSeriesSpec, isBarSeriesSpec, TickFormatterOptions } from '../utils/specs';
 
 /** @internal */
 export const Y0_ACCESSOR_POSTFIX = ' - lower';
@@ -66,7 +59,7 @@ export function formatTooltip(
 ): TooltipValue {
   let label = getSeriesName(seriesIdentifier, hasSingleSeries, true, spec);
 
-  if (isBandedSpec(spec.y0Accessors) && (isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))) {
+  if (isBandedSpec(spec) && (isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))) {
     const { y0AccessorFormat = Y0_ACCESSOR_POSTFIX, y1AccessorFormat = Y1_ACCESSOR_POSTFIX } = spec;
     const formatter = accessor === BandedAccessorType.Y0 ? y0AccessorFormat : y1AccessorFormat;
     label = getAccessorFormatLabel(formatter, label);