fix(legend): legend sizes with ordinal data (opensearch-project#867)

fixes opensearch-project#811

packages/osd-charts/integration/tests/interactions.test.ts

@@ -295,4 +295,10 @@ describe('Interactions', () => {
       );
     });
   });
+  it('should size legends with ordinal x axis', async () => {
+    await common.expectChartWithMouseAtUrlToMatchScreenshot(
+      'http://localhost:9001/iframe.html?id=axes--different-tooltip-formatter',
+      { left: 350, top: 130 },
+    );
+  });
 });

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

@@ -25,11 +25,12 @@ import { Position, RecursivePartial } from '../../../utils/commons';
 import { AxisStyle } from '../../../utils/themes/theme';
 import { SeriesCollectionValue, getSeriesName } from '../utils/series';
 import { AxisSpec, BasicSeriesSpec, SeriesTypes } from '../utils/specs';
-import { computeLegend } from './legend';
+import { computeLegend, getLegendExtra } from './legend';
 
 const nullDisplayValue = {
   formatted: null,
   raw: null,
+  legendSizingLabel: null,
 };
 const seriesCollectionValue1a = {
   seriesIdentifier: {
@@ -129,7 +130,7 @@ describe('Legends', () => {
   });
   it('compute legend for a single series', () => {
     seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
-    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
+    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
     const expected: LegendItem[] = [
       {
         color: 'red',
@@ -147,7 +148,7 @@ describe('Legends', () => {
   it('compute legend for a single spec but with multiple series', () => {
     seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
     seriesCollection.set('seriesCollectionValue1b', seriesCollectionValue1b);
-    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
+    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
     const expected: LegendItem[] = [
       {
         color: 'red',
@@ -175,7 +176,7 @@ describe('Legends', () => {
   it('compute legend for multiple specs', () => {
     seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
     seriesCollection.set('seriesCollectionValue2a', seriesCollectionValue2a);
-    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
+    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
     const expected: LegendItem[] = [
       {
         color: 'red',
@@ -202,13 +203,13 @@ describe('Legends', () => {
   });
   it('empty legend for missing spec', () => {
     seriesCollection.set('seriesCollectionValue2b', seriesCollectionValue2b);
-    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs);
+    const legend = computeLegend(seriesCollection, seriesCollectionMap, specs, 'violet', axesSpecs, false);
     expect(legend.length).toEqual(0);
   });
   it('compute legend with default color for missing series color', () => {
     seriesCollection.set('seriesCollectionValue1a', seriesCollectionValue1a);
     const emptyColorMap = new Map<string, string>();
-    const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs);
+    const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs, false);
     const expected: LegendItem[] = [
       {
         color: 'violet',
@@ -231,7 +232,7 @@ describe('Legends', () => {
 
     const emptyColorMap = new Map<string, string>();
 
-    const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs);
+    const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs, false);
 
     const visibility = [...legend.values()].map((item) => !item.isSeriesHidden);
 
@@ -246,7 +247,15 @@ describe('Legends', () => {
     const emptyColorMap = new Map<string, string>();
     const deselectedDataSeries = [seriesCollectionValue1a.seriesIdentifier, seriesCollectionValue1b.seriesIdentifier];
 
-    const legend = computeLegend(seriesCollection, emptyColorMap, specs, 'violet', axesSpecs, deselectedDataSeries);
+    const legend = computeLegend(
+      seriesCollection,
+      emptyColorMap,
+      specs,
+      'violet',
+      axesSpecs,
+      false,
+      deselectedDataSeries,
+    );
 
     const visibility = [...legend.values()].map((item) => !item.isSeriesHidden);
     expect(visibility).toEqual([false, false, true]);
@@ -344,4 +353,37 @@ 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/osd-charts/src/chart_types/xy_chart/legend/legend.ts

@@ -19,9 +19,12 @@
 
 import { LegendItem } from '../../../commons/legend';
 import { SeriesKey, SeriesIdentifier } from '../../../commons/series_id';
+import { ScaleType } from '../../../scales/constants';
+import { TickFormatterOptions } from '../../../specs';
 import { Color } from '../../../utils/commons';
 import { BandedAccessorType } from '../../../utils/geometry';
 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 {
@@ -56,16 +59,43 @@ 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 */
 export function computeLegend(
   seriesCollection: Map<SeriesKey, SeriesCollectionValue>,
   seriesColors: Map<SeriesKey, Color>,
   specs: BasicSeriesSpec[],
   defaultColor: string,
   axesSpecs: AxisSpec[],
+  showLegendExtra: boolean,
   deselectedDataSeries: SeriesIdentifier[] = [],
 ): LegendItem[] {
   const legendItems: LegendItem[] = [];
+
   const sortedCollection = getSortedDataSeriesColorsValuesMap(seriesCollection);
 
   sortedCollection.forEach((series, key) => {
@@ -79,6 +109,7 @@ export function computeLegend(
     if (name === '' || !spec) {
       return;
     }
+
     const postFixes = getPostfix(spec);
     const labelY1 = banded ? getBandedLegendItemLabel(name, BandedAccessorType.Y1, postFixes) : name;
 
@@ -95,10 +126,7 @@ export function computeLegend(
       isSeriesHidden,
       isItemHidden: hideInLegend,
       isToggleable: true,
-      defaultExtra: {
-        raw: lastValue && lastValue.y1 !== null ? lastValue.y1 : null,
-        formatted: lastValue && lastValue.y1 !== null ? formatter(lastValue.y1) : null,
-      },
+      defaultExtra: getLegendExtra(showLegendExtra, spec.xScaleType, formatter, 'y1', lastValue),
     });
     if (banded) {
       const labelY0 = getBandedLegendItemLabel(name, BandedAccessorType.Y0, postFixes);
@@ -110,10 +138,7 @@ export function computeLegend(
         isSeriesHidden,
         isItemHidden: hideInLegend,
         isToggleable: true,
-        defaultExtra: {
-          raw: lastValue && lastValue.y0 !== null ? lastValue.y0 : null,
-          formatted: lastValue && lastValue.y0 !== null ? formatter(lastValue.y0) : null,
-        },
+        defaultExtra: getLegendExtra(showLegendExtra, spec.xScaleType, formatter, 'y0', lastValue),
       });
     }
   });

packages/osd-charts/src/chart_types/xy_chart/rendering/rendering.test.ts

@@ -144,6 +144,7 @@ describe('Rendering utils', () => {
       defaultExtra: {
         formatted: null,
         raw: null,
+        legendSizingLabel: null,
       },
     };
 

packages/osd-charts/src/chart_types/xy_chart/state/chart_state.test.ts

@@ -77,10 +77,6 @@ describe.skip('Chart Store', () => {
       specId: SPEC_ID,
       key: 'color1',
     },
-    defaultExtra: {
-      raw: null,
-      formatted: 'formatted-last',
-    },
   };
 
   const secondLegendItem: LegendItem = {
@@ -90,10 +86,6 @@ describe.skip('Chart Store', () => {
       specId: SPEC_ID,
       key: 'color2',
     },
-    defaultExtra: {
-      raw: null,
-      formatted: 'formatted-last',
-    },
   };
   beforeEach(() => {
     store = null; // new ChartStore();

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

@@ -23,6 +23,7 @@ import { LegendItem } from '../../../../commons/legend';
 import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
 import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme';
 import { getDeselectedSeriesSelector } from '../../../../state/selectors/get_deselected_data_series';
+import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
 import { computeLegend } from '../../legend/legend';
 import { computeSeriesDomainsSelector } from './compute_series_domains';
 import { getSeriesColorsSelector } from './get_series_color_map';
@@ -37,14 +38,24 @@ export const computeLegendSelector = createCachedSelector(
     getSeriesColorsSelector,
     getAxisSpecsSelector,
     getDeselectedSeriesSelector,
+    getSettingsSpecSelector,
   ],
-  (seriesSpecs, seriesDomainsAndData, chartTheme, seriesColors, axesSpecs, deselectedDataSeries): LegendItem[] =>
+  (
+    seriesSpecs,
+    seriesDomainsAndData,
+    chartTheme,
+    seriesColors,
+    axesSpecs,
+    deselectedDataSeries,
+    { showLegendExtra },
+  ): LegendItem[] =>
     computeLegend(
       seriesDomainsAndData.seriesCollection,
       seriesColors,
       seriesSpecs,
       chartTheme.colors.defaultVizColor,
       axesSpecs,
+      showLegendExtra,
       deselectedDataSeries,
     ),
 )(getChartIdSelector);

packages/osd-charts/src/chart_types/xy_chart/state/selectors/get_legend_items_labels.ts

@@ -29,8 +29,8 @@ export const getLegendItemsLabelsSelector = createCachedSelector(
   [computeLegendSelector, getSettingsSpecSelector],
   (legendItems, { showLegendExtra }): LegendItemLabel[] =>
     legendItems.map(({ label, defaultExtra }) => {
-      if (defaultExtra?.formatted != null) {
-        return { label: `${label}${showLegendExtra ? defaultExtra.formatted : ''}`, depth: 0 };
+      if (defaultExtra?.legendSizingLabel != null) {
+        return { label: `${label}${showLegendExtra ? defaultExtra.legendSizingLabel : ''}`, depth: 0 };
       }
       return { label, depth: 0 };
     }),

packages/osd-charts/src/chart_types/xy_chart/state/utils/common.test.ts

@@ -141,7 +141,7 @@ describe('Type Checks', () => {
             key: 'specId:{bars},colors:{a}',
             specId: 'bars',
           },
-          defaultExtra: { raw: 6, formatted: '6.00' },
+          defaultExtra: { raw: 6, formatted: '6.00', legendSizingLabel: '6.00' },
           isSeriesHidden: true,
         },
         {
@@ -151,7 +151,7 @@ describe('Type Checks', () => {
             key: 'specId:{bars},colors:{b}',
             specId: 'bars',
           },
-          defaultExtra: { raw: 2, formatted: '2.00' },
+          defaultExtra: { raw: 2, formatted: '2.00', legendSizingLabel: '2.00' },
           isSeriesHidden: true,
         },
       ];
@@ -166,7 +166,7 @@ describe('Type Checks', () => {
             key: 'specId:{bars},colors:{a}',
             specId: 'bars',
           },
-          defaultExtra: { raw: 6, formatted: '6.00' },
+          defaultExtra: { raw: 6, formatted: '6.00', legendSizingLabel: '6.00' },
           isSeriesHidden: false,
         },
         {
@@ -176,7 +176,7 @@ describe('Type Checks', () => {
             key: 'specId:{bars},colors:{b}',
             specId: 'bars',
           },
-          defaultExtra: { raw: 2, formatted: '2.00' },
+          defaultExtra: { raw: 2, formatted: '2.00', legendSizingLabel: '2.00' },
           isSeriesHidden: true,
         },
       ];

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

@@ -19,7 +19,6 @@
 
 import { SeriesKey, SeriesIdentifier } from '../../../../commons/series_id';
 import { Scale } from '../../../../scales';
-import { ScaleType } from '../../../../scales/constants';
 import { OrderBy } from '../../../../specs/settings';
 import { mergePartial, Rotation, Color, isUniqueArray } from '../../../../utils/commons';
 import { CurveType } from '../../../../utils/curves';
@@ -139,9 +138,7 @@ function getLastValues(
   xDomain: XDomain,
 ): Map<SeriesKey, LastValues> {
   const lastValues = new Map<SeriesKey, LastValues>();
-  if (xDomain.scaleType === ScaleType.Ordinal) {
-    return lastValues;
-  }
+
   // we need to get the latest
   formattedDataSeries.stacked.forEach(({ dataSeries, stackMode }) => {
     dataSeries.forEach((series) => {

packages/osd-charts/src/commons/legend.ts

@@ -35,6 +35,7 @@ export type LegendItem = {
   defaultExtra?: {
     raw: number | null;
     formatted: number | string | null;
+    legendSizingLabel: number | string | null;
   };
   // TODO: Remove when partition layers are toggleable
   isToggleable?: boolean;

packages/osd-charts/stories/axes/6a_different_tooltip_formatter.tsx

@@ -41,12 +41,6 @@ export const Example = () => {
   return (
     <Chart className="story-chart">
       <Settings
-        theme={{
-          legend: {
-            // Used to allow space for long formatting
-            spacingBuffer: 60,
-          },
-        }}
         showLegendExtra
         showLegend={showLegend}
         tooltip={{

packages/osd-charts/stories/mixed/6_fitting_stacked.tsx

@@ -18,6 +18,7 @@
  */
 
 import { select, number } from '@storybook/addon-knobs';
+import numeral from 'numeral';
 import React from 'react';
 
 import { AreaSeries, Axis, Chart, CurveType, Position, ScaleType, Settings, Fit, StackMode } from '../../src';
@@ -158,6 +159,7 @@ export const Example = () => {
   const value = number('Explicit value (using Fit.Explicit)', 5);
   const xScaleType = dataKey === 'ordinal' ? ScaleType.Ordinal : ScaleType.Linear;
   const rng = getRandomNumberGenerator();
+  const tickFormat = stackMode === 'percentage' ? (d: any) => numeral(d).format('0[.]00%') : undefined;
   return (
     <Chart className="story-chart">
       <Settings
@@ -172,7 +174,7 @@ export const Example = () => {
         }}
       />
       <Axis id="bottom" position={Position.Bottom} title="Bottom axis" showOverlappingTicks />
-      <Axis id="left" title="Left axis" position={Position.Left} />
+      <Axis id="left" title="Left axis" position={Position.Left} tickFormat={tickFormat} />
       <AreaSeries
         id="test3"
         xScaleType={xScaleType}