fix(legend): improve last value handling (#2115)

This commit cleans up the concept of what the value, shown after the legend item label, represents. We removed every concept of bucket/time interval from this calculation to clarify the legend item value concept.
This allows us to decouple our code from Elasticsearch or other data sources.
A small, but fixable side-effect is: the last value depends entirely on the passed data, so if you pass just 10 values of a series that should cover 100 data points, we consider the last value the last of these 10 values: if the 10th value is not at the extreme edge of your data domain we still consider that 10th value as the last one. If you instead, are looking to depict the extreme edge of your data domain (the 100th element) that in the mentioned example is empty/null then you should pass all the 100 data points with null values where needed.

BREAKING CHANGE:  In cartesian charts, the default legend value now represents the data points that coincide with the latest datum in the X domain. Please consider passing every data point, even the empty ones (like empty buckets/bins/etc) if your x data domain doesn't fully cover a custom x domain passed to the chart configuration.

e2e/tests/test_cases_stories.test.ts

@@ -80,4 +80,31 @@ test.describe('Test cases stories', () => {
       );
     });
   });
+
+  test.describe('Legend last value should be aligned across areas and bars', () => {
+    test('data domain', async ({ page }) => {
+      await common.expectChartAtUrlToMatchScreenshot(page)(
+        'http://localhost:9001/?path=/story/test-cases--domain-edges&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-custom domain=&knob-start time=0&knob-end time=19&knob-subtract 1ms=',
+        { screenshotSelector: '#story-root' },
+      );
+    });
+    test('custom domain', async ({ page }) => {
+      await common.expectChartAtUrlToMatchScreenshot(page)(
+        'http://localhost:9001/?path=/story/test-cases--domain-edges&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-custom domain=true&knob-start time=0&knob-end time=19&knob-subtract 1ms=',
+        { screenshotSelector: '#story-root' },
+      );
+    });
+    test('custom -1ms', async ({ page }) => {
+      await common.expectChartAtUrlToMatchScreenshot(page)(
+        'http://localhost:9001/?path=/story/test-cases--domain-edges&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-custom domain=true&knob-start time=0&knob-end time=19&knob-subtract 1ms=true',
+        { screenshotSelector: '#story-root' },
+      );
+    });
+    test('custom empty', async ({ page }) => {
+      await common.expectChartAtUrlToMatchScreenshot(page)(
+        'http://localhost:9001/?path=/story/test-cases--domain-edges&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-custom domain=true&knob-end time=10.2&knob-start time=0&knob-subtract 1ms=true',
+        { screenshotSelector: '#story-root' },
+      );
+    });
+  });
 });

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

@@ -22,6 +22,7 @@ export type XDomain = Pick<LogScaleOptions, 'logBase'> & {
   /** the configured timezone in the specs or the fallback to the browser local timezone */
   timeZone: string;
   domain: OrdinalDomain | ContinuousDomain;
+  dataDomain: OrdinalDomain | ContinuousDomain;
   desiredTickCount: number;
 };
 

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

@@ -28,18 +28,20 @@ export function mergeXDomain(
   locale: string,
   fallbackScale?: XScaleType,
 ): XDomain {
-  let seriesXComputedDomains;
+  let domain;
+  let dataDomain;
   let minInterval = 0;
 
   if (type === ScaleType.Ordinal || fallbackScale === ScaleType.Ordinal) {
     if (type !== ScaleType.Ordinal) {
       Logger.warn(`Each X value in a ${type} x scale needs be be a number. Using ordinal x scale as fallback.`);
     }
 
-    seriesXComputedDomains = computeOrdinalDataDomain([...xValues], false, true, locale);
+    dataDomain = computeOrdinalDataDomain([...xValues], false, true, locale);
+    domain = dataDomain;
     if (customDomain) {
       if (Array.isArray(customDomain)) {
-        seriesXComputedDomains = [...customDomain];
+        domain = [...customDomain];
       } else {
         if (fallbackScale === ScaleType.Ordinal) {
           Logger.warn(`xDomain ignored for fallback ordinal scale. Options to resolve:
@@ -54,37 +56,38 @@ export function mergeXDomain(
     }
   } else {
     const domainOptions = { min: NaN, max: NaN, fit: true };
-    seriesXComputedDomains = computeContinuousDataDomain([...xValues] as number[], type, domainOptions);
+    dataDomain = computeContinuousDataDomain([...xValues] as number[], type, domainOptions);
+    domain = dataDomain;
     let customMinInterval: undefined | number;
 
     if (customDomain) {
       if (Array.isArray(customDomain)) {
         Logger.warn('xDomain for continuous scale should be a DomainRange object, not an array');
       } else {
         customMinInterval = customDomain.minInterval;
-        const [computedDomainMin, computedDomainMax] = seriesXComputedDomains;
+        const [computedDomainMin, computedDomainMax] = domain;
 
         if (Number.isFinite(customDomain.min) && Number.isFinite(customDomain.max)) {
           if (customDomain.min > customDomain.max) {
             Logger.warn('Custom xDomain is invalid: min is greater than max. Custom domain is ignored.');
           } else {
-            seriesXComputedDomains = [customDomain.min, customDomain.max];
+            domain = [customDomain.min, customDomain.max];
           }
         } else if (Number.isFinite(customDomain.min)) {
           if (customDomain.min > computedDomainMax) {
             Logger.warn(
               'Custom xDomain is invalid: custom min is greater than computed max. Custom domain is ignored.',
             );
           } else {
-            seriesXComputedDomains = [customDomain.min, computedDomainMax];
+            domain = [customDomain.min, computedDomainMax];
           }
         } else if (Number.isFinite(customDomain.max)) {
           if (computedDomainMin > customDomain.max) {
             Logger.warn(
               'Custom xDomain is invalid: computed min is greater than custom max. Custom domain is ignored.',
             );
           } else {
-            seriesXComputedDomains = [computedDomainMin, customDomain.max];
+            domain = [computedDomainMin, customDomain.max];
           }
         }
       }
@@ -97,7 +100,8 @@ export function mergeXDomain(
     type: fallbackScale ?? type,
     nice,
     isBandScale,
-    domain: seriesXComputedDomains,
+    domain,
+    dataDomain,
     minInterval,
     timeZone: getValidatedTimeZone(timeZone),
     logBase: customDomain && 'logBase' in customDomain ? customDomain.logBase : 10, // fixme preexisting TS workaround

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

@@ -8,7 +8,6 @@
 
 import { Store } from 'redux';
 
-import { getLegendExtra } from './legend';
 import { ChartType } from '../..';
 import { MockGlobalSpec, MockSeriesSpec } from '../../../mocks/specs/specs';
 import { MockStore } from '../../../mocks/store/store';
@@ -24,9 +23,9 @@ import { getSeriesName } from '../utils/series';
 import { AxisSpec, BasicSeriesSpec, SeriesType } from '../utils/specs';
 
 const nullDisplayValue = {
-  formatted: null,
   raw: null,
-  legendSizingLabel: null,
+  formatted: '',
+  legendSizingLabel: '',
 };
 
 const spec1: BasicSeriesSpec = {
@@ -402,37 +401,4 @@ describe('Legends', () => {
     name = getSeriesName(seriesIdentifier1, false, false, specWithSplit);
     expect(name).toBe('Spec 1 title');
   });
-  it('should return correct legendSizingLabel with linear scale and showExtraLegend set to true', () => {
-    const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
-    const lastValues = { y0: null, y1: 14 };
-    const showExtraLegend = true;
-    const xScaleIsLinear = ScaleType.Linear;
-
-    expect(getLegendExtra(showExtraLegend, xScaleIsLinear, formatter, 'y1', lastValues)).toMatchObject({
-      raw: 14,
-      formatted: '14.00 dogs',
-      legendSizingLabel: '14.00 dogs',
-    });
-  });
-  it('should return formatted to null with ordinal scale and showExtraLegend set to true', () => {
-    const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
-    const lastValues = { y0: null, y1: 14 };
-
-    expect(getLegendExtra(true, ScaleType.Ordinal, formatter, 'y1', lastValues)).toMatchObject({
-      raw: 14,
-      formatted: null,
-      legendSizingLabel: '14.00 dogs',
-    });
-  });
-  it('should return legendSizingLabel null with showLegendExtra set to false', () => {
-    const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
-    const lastValues = { y0: null, y1: 14 };
-    const showLegendExtra = false;
-
-    expect(getLegendExtra(showLegendExtra, ScaleType.Ordinal, formatter, 'y1', lastValues)).toMatchObject({
-      raw: null,
-      formatted: null,
-      legendSizingLabel: null,
-    });
-  });
 });

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

@@ -9,15 +9,14 @@
 import { Color } from '../../../common/colors';
 import { LegendItem } from '../../../common/legend';
 import { SeriesKey, SeriesIdentifier } from '../../../common/series_id';
-import { ScaleType } from '../../../scales/constants';
-import { SettingsSpec, TickFormatterOptions } from '../../../specs';
+import { SettingsSpec } from '../../../specs';
 import { isDefined, mergePartial } from '../../../utils/common';
 import { BandedAccessorType } from '../../../utils/geometry';
 import { getLegendCompareFn, SeriesCompareFn } from '../../../utils/series_sort';
 import { PointStyle, Theme } from '../../../utils/themes/theme';
-import { getXScaleTypeFromSpec } from '../scales/get_api_scales';
+import { XDomain } from '../domains/types';
+import { LegendValue, getLegendValue } from '../state/utils/get_last_value';
 import { getAxesSpecForSpecId, getSpecsById } from '../state/utils/spec';
-import { LastValues } from '../state/utils/types';
 import { Y0_ACCESSOR_POSTFIX, Y1_ACCESSOR_POSTFIX } from '../tooltip/tooltip';
 import { defaultTickFormatter } from '../utils/axis_utils';
 import { defaultXYLegendSeriesSort } from '../utils/default_series_sort_fn';
@@ -34,6 +33,7 @@ import {
   AxisSpec,
   BasicSeriesSpec,
   Postfixes,
+  StackMode,
   isAreaSeriesSpec,
   isBarSeriesSpec,
   isBubbleSeriesSpec,
@@ -61,27 +61,6 @@ function getBandedLegendItemLabel(name: string, yAccessor: BandedAccessorType, p
     : `${name}${postfixes.y0AccessorFormat}`;
 }
 
-/** @internal */
-export function getLegendExtra(
-  showLegendExtra: boolean,
-  xScaleType: ScaleType,
-  formatter: (value: any, options?: TickFormatterOptions | undefined) => string,
-  key: keyof LastValues,
-  lastValue?: LastValues,
-): LegendItem['defaultExtra'] {
-  if (showLegendExtra) {
-    const rawValue = (lastValue && lastValue[key]) ?? null;
-    const formattedValue = rawValue !== null ? formatter(rawValue) : null;
-
-    return {
-      raw: rawValue !== null ? rawValue : null,
-      formatted: xScaleType === ScaleType.Ordinal ? null : formattedValue,
-      legendSizingLabel: formattedValue,
-    };
-  }
-  return { raw: null, formatted: null, legendSizingLabel: null };
-}
-
 /** @internal */
 function getPointStyle(spec: BasicSeriesSpec, theme: Theme): PointStyle | undefined {
   if (isBubbleSeriesSpec(spec)) {
@@ -95,8 +74,8 @@ function getPointStyle(spec: BasicSeriesSpec, theme: Theme): PointStyle | undefi
 
 /** @internal */
 export function computeLegend(
+  xDomain: XDomain,
   dataSeries: DataSeries[],
-  lastValues: Map<SeriesKey, LastValues>,
   seriesColors: Map<SeriesKey, Color>,
   specs: BasicSeriesSpec[],
   axesSpecs: AxisSpec[],
@@ -108,10 +87,11 @@ export function computeLegend(
   const legendItems: LegendItem[] = [];
   const defaultColor = theme.colors.defaultVizColor;
 
+  const legendValueMode = LegendValue.LastValue;
+
   dataSeries.forEach((series) => {
     const { specId, yAccessor } = series;
     const banded = isBandedSpec(series.spec);
-    const key = getSeriesKey(series, series.groupId);
     const spec = getSpecsById<BasicSeriesSpec>(specs, specId);
     const dataSeriesKey = getSeriesKey(
       {
@@ -129,33 +109,47 @@ export function computeLegend(
     if (name === '' || !spec) return;
 
     const postFixes = getPostfix(spec);
-    const labelY1 = banded ? getBandedLegendItemLabel(name, BandedAccessorType.Y1, postFixes) : name;
-
     // Use this to get axis spec w/ tick formatter
     const { yAxis } = getAxesSpecForSpecId(axesSpecs, spec.groupId, settingsSpec.rotation);
     const formatter = spec.tickFormat ?? yAxis?.tickFormat ?? defaultTickFormatter;
     const { hideInLegend } = spec;
 
-    const lastValue = lastValues.get(key);
     const seriesIdentifier = getSeriesIdentifierFromDataSeries(series);
-    const xScaleType = getXScaleTypeFromSpec(spec.xScaleType);
 
     const pointStyle = getPointStyle(spec, theme);
 
+    const itemValue = getLegendValue(series, xDomain, legendValueMode, (d) => {
+      return series.stackMode === StackMode.Percentage
+        ? d.y1 === null || d.y0 === null
+          ? null
+          : d.y1 - d.y0
+        : d.initialY1;
+    });
+    const formattedItemValue = itemValue !== null ? formatter(itemValue) : '';
+
     legendItems.push({
       color,
-      label: labelY1,
+      label: banded ? getBandedLegendItemLabel(name, BandedAccessorType.Y1, postFixes) : name,
       seriesIdentifiers: [seriesIdentifier],
       childId: BandedAccessorType.Y1,
       isSeriesHidden,
       isItemHidden: hideInLegend,
       isToggleable: true,
-      defaultExtra: getLegendExtra(settingsSpec.showLegendExtra, xScaleType, formatter, 'y1', lastValue),
+      defaultExtra: {
+        raw: itemValue,
+        formatted: formattedItemValue,
+        legendSizingLabel: formattedItemValue,
+      },
       path: [{ index: 0, value: seriesIdentifier.key }],
       keys: [specId, spec.groupId, yAccessor, ...series.splitAccessors.values()],
       pointStyle,
     });
     if (banded) {
+      const bandedItemValue = getLegendValue(series, xDomain, legendValueMode, (d) => {
+        return series.stackMode === StackMode.Percentage ? d.y0 : d.initialY0;
+      });
+      const bandedFormattedItemValue = bandedItemValue !== null ? formatter(bandedItemValue) : '';
+
       const labelY0 = getBandedLegendItemLabel(name, BandedAccessorType.Y0, postFixes);
       legendItems.push({
         color,
@@ -165,7 +159,11 @@ export function computeLegend(
         isSeriesHidden,
         isItemHidden: hideInLegend,
         isToggleable: true,
-        defaultExtra: getLegendExtra(settingsSpec.showLegendExtra, xScaleType, formatter, 'y0', lastValue),
+        defaultExtra: {
+          raw: bandedItemValue,
+          formatted: bandedFormattedItemValue,
+          legendSizingLabel: bandedFormattedItemValue,
+        },
         path: [{ index: 0, value: seriesIdentifier.key }],
         keys: [specId, spec.groupId, yAccessor, ...series.splitAccessors.values()],
         pointStyle,

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

@@ -17,7 +17,6 @@ import { getDeselectedSeriesSelector } from '../../../../state/selectors/get_des
 import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_spec';
 import { computeLegend } from '../../legend/legend';
 import { DataSeries } from '../../utils/series';
-import { getLastValues } from '../utils/get_last_value';
 
 /** @internal */
 export const computeLegendSelector = createCustomCachedSelector(
@@ -42,8 +41,8 @@ export const computeLegendSelector = createCustomCachedSelector(
     siDataSeriesMap: Record<string, DataSeries>,
   ): LegendItem[] => {
     return computeLegend(
+      xDomain,
       formattedDataSeries,
-      getLastValues(formattedDataSeries, xDomain),
       seriesColors,
       seriesSpecs,
       axesSpecs,