fix(BarSeries): ignore histogram mode in determining stacked series (#…

…2225)

BREAKING CHANGE: now ignores histogram mode in determining stacked series

e2e/tests/bar_stories.test.ts

@@ -41,57 +41,51 @@ test.describe('Bar series stories', () => {
   });
 
   test.describe('[test] histogram mode (linear)', () => {
-    test.describe('enableHistogramMode is true', () => {
-      eachRotation.test(async ({ page, rotation }) => {
-        await common.expectChartAtUrlToMatchScreenshot(page)(
-          `http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=${rotation}&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=line&knob-point series alignment=center&knob-hasHistogramBarSeries=&knob-debug=true&knob-bars-1 enableHistogramMode=true&knob-bars-2 enableHistogramMode=`,
-        );
-      });
-    });
-
-    test.describe('enableHistogramMode is false', () => {
-      eachRotation.test(async ({ page, rotation }) => {
-        await common.expectChartAtUrlToMatchScreenshot(page)(
-          `http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=${rotation}&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=line&knob-point series alignment=center&knob-hasHistogramBarSeries=&knob-debug=true&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=`,
-        );
-      });
-    });
+    pwEach.describe([true, false])(
+      (enableHistogramMode) => `enableHistogramMode is ${enableHistogramMode}`,
+      (enableHistogramMode) => {
+        eachRotation.test(async ({ page, rotation }) => {
+          await common.expectChartAtUrlToMatchScreenshot(page)(
+            `http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=${rotation}&knob-stacked=false&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=line&knob-point series alignment=center&knob-hasHistogramBarSeries=&knob-debug=false&knob-bars-1 enableHistogramMode=${enableHistogramMode}&knob-bars-2 enableHistogramMode=`,
+          );
+        });
+      },
+    );
 
     test.describe('point alignment', () => {
       test('start', async ({ page }) => {
         await common.expectChartAtUrlToMatchScreenshot(page)(
-          'http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=-90&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=area&knob-point series alignment=start&knob-hasHistogramBarSeries=true&knob-debug=true&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=',
+          'http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=-90&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=area&knob-point series alignment=start&knob-hasHistogramBarSeries=true&knob-debug=false&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=',
         );
       });
       test('center', async ({ page }) => {
         await common.expectChartAtUrlToMatchScreenshot(page)(
-          'http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=-90&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=area&knob-point series alignment=center&knob-hasHistogramBarSeries=true&knob-debug=true&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=',
+          'http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=-90&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=area&knob-point series alignment=center&knob-hasHistogramBarSeries=true&knob-debug=false&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=',
         );
       });
       test('end', async ({ page }) => {
         await common.expectChartAtUrlToMatchScreenshot(page)(
-          'http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=-90&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=area&knob-point series alignment=end&knob-hasHistogramBarSeries=true&knob-debug=true&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=',
+          'http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-linear&knob-chartRotation=-90&knob-bars padding=0.25&knob-histogram padding=0.05&knob-other series=area&knob-point series alignment=end&knob-hasHistogramBarSeries=true&knob-debug=false&knob-bars-1 enableHistogramMode=&knob-bars-2 enableHistogramMode=',
         );
       });
     });
   });
 
   test.describe('[test] histogram mode (ordinal)', () => {
-    test.describe('enableHistogramMode is false, hasHistogramBarSeries is false', () => {
-      eachRotation.test(async ({ page, rotation }) => {
-        await common.expectChartAtUrlToMatchScreenshot(page)(
-          `http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-ordinal&knob-chartRotation=${rotation}&knob-bars padding=0.25&knob-hasHistogramBarSeries=&knob-debug=true&knob-bars-1 enableHistogramMode=true&knob-bars-2 enableHistogramMode=`,
-        );
-      });
-    });
-
-    test.describe('enableHistogramMode is true, hasHistogramBarSeries is true', () => {
-      eachRotation.test(async ({ page, rotation }) => {
-        await common.expectChartAtUrlToMatchScreenshot(page)(
-          `http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-ordinal&knob-chartRotation=${rotation}&knob-bars padding=0.25&knob-hasHistogramBarSeries=true&knob-debug=true&knob-bars-1 enableHistogramMode=true&knob-bars-2 enableHistogramMode=`,
-        );
-      });
-    });
+    pwEach.describe([
+      [true, true],
+      [false, false],
+    ])(
+      ([enableHistogramMode, hasHistogramBarSeries]) =>
+        `enableHistogramMode is ${enableHistogramMode}, hasHistogramBarSeries is ${hasHistogramBarSeries}`,
+      ([enableHistogramMode, hasHistogramBarSeries]) => {
+        eachRotation.test(async ({ page, rotation }) => {
+          await common.expectChartAtUrlToMatchScreenshot(page)(
+            `http://localhost:9001/?path=/story/bar-chart--test-histogram-mode-ordinal&knob-chartRotation=${rotation}&knob-bars padding=0.25&knob-hasHistogramBarSeries=${hasHistogramBarSeries}&knob-debug=false&knob-bars-1 enableHistogramMode=${enableHistogramMode}&knob-bars-2 enableHistogramMode=`,
+          );
+        });
+      },
+    );
   });
 
   test.describe('different groupId', () => {

packages/charts/api/charts.api.md

@@ -1512,13 +1512,13 @@ export interface HighlighterStyle {
 }
 
 // @public
-export const HistogramBarSeries: <D extends BaseDatum = any>(props: SFProps<HistogramBarSeriesSpec<D>, "chartType" | "specType" | "seriesType", "groupId" | "enableHistogramMode" | "xScaleType" | "yScaleType" | "hideInLegend", "name" | "color" | "timeZone" | "barSeriesStyle" | "xNice" | "yNice" | "useDefaultGroupDomain" | "displayValueSettings" | "y0AccessorFormat" | "y1AccessorFormat" | "filterSeriesInTooltip" | "tickFormat" | "y0Accessors" | "splitSeriesAccessors" | "markSizeAccessor" | "stackMode" | "styleAccessor" | "minBarHeight", "id" | "data" | "xAccessor" | "yAccessors">) => null;
+export const HistogramBarSeries: <D extends BaseDatum = any>(props: SFProps<HistogramBarSeriesSpec<D>, "chartType" | "specType" | "seriesType", "groupId" | "enableHistogramMode" | "xScaleType" | "yScaleType" | "hideInLegend", "name" | "color" | "timeZone" | "barSeriesStyle" | "xNice" | "yNice" | "useDefaultGroupDomain" | "displayValueSettings" | "y0AccessorFormat" | "y1AccessorFormat" | "filterSeriesInTooltip" | "tickFormat" | "y0Accessors" | "splitSeriesAccessors" | "stackAccessors" | "markSizeAccessor" | "stackMode" | "styleAccessor" | "minBarHeight", "id" | "data" | "xAccessor" | "yAccessors">) => null;
 
 // @public (undocumented)
 export type HistogramBarSeriesProps = ComponentProps<typeof HistogramBarSeries>;
 
 // @public
-export type HistogramBarSeriesSpec<D extends BaseDatum = Datum> = Omit<BarSeriesSpec<D>, 'stackAccessors'> & {
+export type HistogramBarSeriesSpec<D extends BaseDatum = Datum> = BarSeriesSpec<D> & {
     enableHistogramMode: true;
 };
 

packages/charts/src/chart_types/xy_chart/domains/y_domain.ts

@@ -141,7 +141,6 @@ export function groupSeriesByYGroup(specs: YBasicSeriesSpec[]) {
     { stackMode: StackMode | undefined; stacked: YBasicSeriesSpec[]; nonStacked: YBasicSeriesSpec[] }
   >();
 
-  const histogramEnabled = isHistogramEnabled(specs);
   // split each specs by groupId and by stacked or not
   specs.forEach((spec) => {
     const group = specsByGroupIds.get(spec.groupId) || {
@@ -150,7 +149,7 @@ export function groupSeriesByYGroup(specs: YBasicSeriesSpec[]) {
       nonStacked: [],
     };
 
-    if (isStackedSpec(spec, histogramEnabled)) {
+    if (isStackedSpec(spec)) {
       group.stacked.push(spec);
     } else {
       group.nonStacked.push(spec);
@@ -168,15 +167,13 @@ export function groupSeriesByYGroup(specs: YBasicSeriesSpec[]) {
 }
 
 /** @internal */
-export function isHistogramEnabled(specs: YBasicSeriesSpec[]) {
+export function hasHistogramBarSpec(specs: YBasicSeriesSpec[]) {
   return specs.some(({ seriesType, enableHistogramMode }) => seriesType === SeriesType.Bar && enableHistogramMode);
 }
 
 /** @internal */
-export function isStackedSpec(spec: YBasicSeriesSpec, histogramEnabled: boolean) {
-  const isBarAndHistogram = spec.seriesType === SeriesType.Bar && histogramEnabled;
-  const hasStackAccessors = spec.stackAccessors && spec.stackAccessors.length > 0;
-  return isBarAndHistogram || hasStackAccessors;
+export function isStackedSpec(spec: YBasicSeriesSpec) {
+  return spec.stackAccessors && spec.stackAccessors.length > 0;
 }
 
 /** @internal */

packages/charts/src/chart_types/xy_chart/legend/legend.ts

@@ -27,8 +27,8 @@ import {
   getSeriesName,
   DataSeries,
   getSeriesKey,
-  isBandedSpec,
   getSeriesIdentifierFromDataSeries,
+  isBandedSpec,
 } from '../utils/series';
 import {
   AxisSpec,

packages/charts/src/chart_types/xy_chart/state/selectors/count_bars_in_cluster.ts

@@ -7,44 +7,32 @@
  */
 
 import { computeSeriesDomainsSelector } from './compute_series_domains';
-import { isHistogramModeEnabledSelector } from './is_histogram_mode_enabled';
 import { SeriesType } from '../../../../specs';
 import { createCustomCachedSelector } from '../../../../state/create_selector';
 import { groupBy } from '../../utils/group_data_series';
-import { SeriesDomainsAndData } from '../utils/types';
 import { getBarIndexKey } from '../utils/utils';
 
 /** @internal */
 export const countBarsInClusterSelector = createCustomCachedSelector(
-  [computeSeriesDomainsSelector, isHistogramModeEnabledSelector],
-  countBarsInCluster,
-);
-
-/** @internal */
-export function countBarsInCluster({ formattedDataSeries }: SeriesDomainsAndData, isHistogramEnabled: boolean): number {
-  const barDataSeries = formattedDataSeries.filter(({ seriesType }) => seriesType === SeriesType.Bar);
-
-  const dataSeriesGroupedByPanel = groupBy(
-    barDataSeries,
-    ['smVerticalAccessorValue', 'smHorizontalAccessorValue'],
-    false,
-  );
-
-  const barIndexByPanel = Object.keys(dataSeriesGroupedByPanel).reduce<Record<string, string[]>>((acc, panelKey) => {
-    const panelBars = dataSeriesGroupedByPanel[panelKey] ?? [];
-    const barDataSeriesByBarIndex = groupBy(
-      panelBars,
-      (d) => {
-        return getBarIndexKey(d, isHistogramEnabled);
-      },
+  [computeSeriesDomainsSelector],
+  function countBarsInCluster({ formattedDataSeries }): number {
+    const barDataSeries = formattedDataSeries.filter(({ seriesType }) => seriesType === SeriesType.Bar);
+    const dataSeriesGroupedByPanel = groupBy(
+      barDataSeries,
+      ['smVerticalAccessorValue', 'smHorizontalAccessorValue'],
       false,
     );
 
-    acc[panelKey] = Object.keys(barDataSeriesByBarIndex);
-    return acc;
-  }, {});
+    const barIndexByPanel = Object.keys(dataSeriesGroupedByPanel).reduce<Record<string, string[]>>((acc, panelKey) => {
+      const panelBars = dataSeriesGroupedByPanel[panelKey] ?? [];
+      const barDataSeriesByBarIndex = groupBy(panelBars, getBarIndexKey, false);
+
+      acc[panelKey] = Object.keys(barDataSeriesByBarIndex);
+      return acc;
+    }, {});
 
-  return Object.values(barIndexByPanel).reduce((acc, curr) => {
-    return Math.max(acc, curr.length);
-  }, 0);
-}
+    return Object.values(barIndexByPanel).reduce((acc, curr) => {
+      return Math.max(acc, curr.length);
+    }, 0);
+  },
+);

packages/charts/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts

@@ -39,7 +39,7 @@ import { getTooltipCompareFn } from '../../../../utils/series_sort';
 import { isPointOnGeometry } from '../../rendering/utils';
 import { formatTooltipHeader, formatTooltipValue } from '../../tooltip/tooltip';
 import { defaultXYLegendSeriesSort } from '../../utils/default_series_sort_fn';
-import { DataSeries } from '../../utils/series';
+import { DataSeries, isBandedSpec } from '../../utils/series';
 import { BasicSeriesSpec, AxisSpec } from '../../utils/specs';
 import { getAxesSpecForSpecId, getSpecDomainGroupId, getSpecsById } from '../utils/spec';
 import { ComputedScales } from '../utils/types';
@@ -161,7 +161,14 @@ function getTooltipAndHighlightFromValue(
     }
 
     // format the tooltip values
-    const formattedTooltip = formatTooltipValue(indexedGeometry, spec, isHighlighted, hasSingleSeries, yAxis);
+    const formattedTooltip = formatTooltipValue(
+      indexedGeometry,
+      spec,
+      isHighlighted,
+      hasSingleSeries,
+      isBandedSpec(spec),
+      yAxis,
+    );
 
     // format only one time the x value
     if (!header) {

packages/charts/src/chart_types/xy_chart/state/selectors/on_brush_end_caller.ts

@@ -28,7 +28,7 @@ import { clamp, Rotation } from '../../../../utils/common';
 import { Dimensions } from '../../../../utils/dimensions';
 import { hasDragged, DragCheckProps } from '../../../../utils/events';
 import { GroupId } from '../../../../utils/ids';
-import { isHistogramEnabled } from '../../domains/y_domain';
+import { hasHistogramBarSpec } from '../../domains/y_domain';
 import { isVerticalRotation } from '../utils/common';
 
 const getLastDragSelector = (state: GlobalChartState) => state.interactions.pointer.lastDrag;
@@ -155,7 +155,7 @@ function getXBrushExtent(
     return;
   }
   const offset = histogramMode ? 0 : -(xScale.bandwidth + xScale.bandwidthPadding) / 2;
-  const histogramEnabled = isHistogramEnabled(seriesSpecs);
+  const histogramEnabled = hasHistogramBarSpec(seriesSpecs);
   const invertValue =
     histogramEnabled && roundHistogramBrushValues
       ? (value: number) => xScale.invertWithStep(value, xScale.domain).value

packages/charts/src/chart_types/xy_chart/state/utils/utils.ts

@@ -206,7 +206,7 @@ export function computeSeriesGeometries(
 
   const barIndexByPanel = Object.keys(dataSeriesGroupedByPanel).reduce<Record<string, string[]>>((acc, panelKey) => {
     const panelBars = dataSeriesGroupedByPanel[panelKey] ?? [];
-    const barDataSeriesByBarIndex = groupBy(panelBars, (d) => getBarIndexKey(d, enableHistogramMode), false);
+    const barDataSeriesByBarIndex = groupBy(panelBars, getBarIndexKey, false);
     acc[panelKey] = Object.keys(barDataSeriesByBarIndex);
     return acc;
   }, {});
@@ -370,7 +370,7 @@ function renderGeometries(
     const color = seriesColorsMap.get(dataSeriesKey) || defaultColor;
 
     if (isBarSeriesSpec(spec)) {
-      const shift = barIndexOrder.indexOf(getBarIndexKey(ds, enableHistogramMode));
+      const shift = barIndexOrder.indexOf(getBarIndexKey(ds));
 
       if (shift === -1) return; // skip bar dataSeries if index is not available
 
@@ -528,12 +528,8 @@ function hasFitFnConfigured(fit?: Fit | FitConfig) {
 }
 
 /** @internal */
-export function getBarIndexKey(
-  { spec, specId, groupId, yAccessor, splitAccessors }: DataSeries,
-  histogramModeEnabled: boolean,
-) {
-  const isStacked = isStackedSpec(spec, histogramModeEnabled);
-  if (isStacked) {
+export function getBarIndexKey({ spec, specId, groupId, yAccessor, splitAccessors }: DataSeries): string {
+  if (isStackedSpec(spec)) {
     return [groupId, '__stacked__'].join('__-__');
   }