fix: group legend items by label and color (opensearch-project#999)

This commit fixes the duplicated legend elements when displaying small multiples.
The legend item concept should allow the user to uniquely link a geometry color to a label. There are situations where multiple geometries or series are colored with the same color, like in a small multiple, and all of these series are related to the same metric for example. This PR removes any duplicate of the key color, label in the legend item, and introduce the ability to have multiple series referenced by the same legend item.

BREAKING CHANGE: The `LegendActionProps` and the `LegendColorPickerProps`, used to add actions and color picker through the legend now receive an array of `SeriesIdentifiers`

packages/osd-charts/.playground/index.html

@@ -8,7 +8,7 @@
     <style>
       html,
       body {
-        background: blanchedalmond !important;
+        background: white !important;
         width: 100%;
         height: 100%;
         overflow: hidden;

packages/osd-charts/.playground/index.tsx

@@ -22,6 +22,7 @@ import React from 'react';
 import ReactDOM from 'react-dom';
 
 import '../src/theme_light.scss';
+import '../node_modules/@elastic/eui/dist/eui_theme_light.css';
 import { Playground } from './playground';
 
 ReactDOM.render(<Playground />, document.getElementById('root') as HTMLElement);

packages/osd-charts/.playground/webpack.config.js

@@ -35,6 +35,16 @@ module.exports = {
           transpileOnly: true,
         },
       },
+      {
+        test: /\.css$/,
+        use: [
+          'style-loader',
+          {
+            loader: 'css-loader',
+            options: { importLoaders: 1 },
+          },
+        ],
+      },
       {
         test: /\.scss$/,
         use: [

packages/osd-charts/api/charts.api.md

@@ -1207,7 +1207,7 @@ export type LegendAction = ComponentType<LegendActionProps>;
 export interface LegendActionProps {
     color: string;
     label: string;
-    series: SeriesIdentifier;
+    series: SeriesIdentifier[];
 }
 
 // Warning: (ae-missing-release-tag) "LegendColorPicker" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
@@ -1223,13 +1223,13 @@ export interface LegendColorPickerProps {
     color: Color;
     onChange: (color: Color | null) => void;
     onClose: () => void;
-    seriesIdentifier: SeriesIdentifier;
+    seriesIdentifiers: SeriesIdentifier[];
 }
 
 // Warning: (ae-missing-release-tag) "LegendItemListener" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public (undocumented)
-export type LegendItemListener = (series: SeriesIdentifier | null) => void;
+export type LegendItemListener = (series: SeriesIdentifier[]) => void;
 
 // @public (undocumented)
 export type LegendPath = LegendPathElement[];
@@ -1778,6 +1778,11 @@ export type SeriesIdentifier = {
     key: SeriesKey;
 };
 
+// Warning: (ae-missing-release-tag) "SeriesKey" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+//
+// @public
+export type SeriesKey = CategoryKey;
+
 // @public (undocumented)
 export type SeriesName = string | number | null;
 
@@ -2163,6 +2168,11 @@ export type TickStyle = StrokeStyle & Visible & {
 // @public (undocumented)
 export function timeFormatter(format: string): TickFormatter;
 
+// Warning: (ae-missing-release-tag) "toEntries" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+//
+// @public
+export function toEntries<T extends Record<string, string>, S>(array: T[], accessor: keyof T, staticValue: S): Record<string, S>;
+
 // @public
 export interface TooltipInfo {
     header: TooltipValue | null;
@@ -2348,7 +2358,6 @@ export type YDomainRange = YDomainBase & DomainRange;
 // src/chart_types/heatmap/layout/types/config_types.ts:62:5 - (ae-forgotten-export) The symbol "TextBaseline" needs to be exported by the entry point index.d.ts
 // src/chart_types/partition_chart/layout/types/config_types.ts:130:5 - (ae-forgotten-export) The symbol "TimeMs" needs to be exported by the entry point index.d.ts
 // src/chart_types/partition_chart/layout/types/config_types.ts:131:5 - (ae-forgotten-export) The symbol "AnimKeyframe" needs to be exported by the entry point index.d.ts
-// src/common/series_id.ts:40:3 - (ae-forgotten-export) The symbol "SeriesKey" needs to be exported by the entry point index.d.ts
 
 // (No @packageDocumentation comment for this package)
 

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

@@ -45,10 +45,11 @@ export const computeLegendSelector = createCachedSelector(
       return {
         color,
         label: `> ${tick}`,
-        seriesIdentifier,
+        seriesIdentifiers: [seriesIdentifier],
         isSeriesHidden: deselectedDataSeries.some((dataSeries) => dataSeries.key === seriesIdentifier.key),
         isToggleable: true,
         path: [{ index: 0, value: seriesIdentifier.key }],
+        keys: [],
       };
     });
   },

packages/osd-charts/src/chart_types/partition_chart/layout/utils/legend.ts

@@ -76,7 +76,8 @@ export function getLegendItems(
       childId: dataName,
       depth: useHierarchicalLegend ? depth - 1 : 0,
       path,
-      seriesIdentifier: { key: dataName, specId: id },
+      seriesIdentifiers: [{ key: dataName, specId: id }],
+      keys: [],
     };
   });
 }

packages/osd-charts/src/chart_types/partition_chart/partition.test.tsx

@@ -103,7 +103,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 0,
           label: 'A',
-          seriesIdentifier: { key: 'A', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'A', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'A',
@@ -115,7 +116,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'A',
-          seriesIdentifier: { key: 'A', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'A', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'B',
@@ -127,7 +129,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'B',
-          seriesIdentifier: { key: 'B', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'B', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'B',
@@ -138,7 +141,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 0,
           label: 'B',
-          seriesIdentifier: { key: 'B', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'B', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'A',
@@ -150,7 +154,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'A',
-          seriesIdentifier: { key: 'A', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'A', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'B',
@@ -162,7 +167,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'B',
-          seriesIdentifier: { key: 'B', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'B', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'C',
@@ -173,7 +179,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 0,
           label: 'C',
-          seriesIdentifier: { key: 'C', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'C', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'A',
@@ -185,7 +192,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'A',
-          seriesIdentifier: { key: 'A', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'A', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'B',
@@ -197,7 +205,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'B',
-          seriesIdentifier: { key: 'B', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'B', specId: 'spec1' }],
+          keys: [],
         },
       ]);
     });
@@ -220,7 +229,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 0,
           label: 'A',
-          seriesIdentifier: { key: 'A', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'A', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'A',
@@ -242,7 +252,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
 
           depth: 1,
           label: 'A',
-          seriesIdentifier: { key: 'A', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'A', specId: 'spec1' }],
+          keys: [],
         },
       ]);
     });
@@ -265,7 +276,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 0,
           label: 'C',
-          seriesIdentifier: { key: 'C', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'C', specId: 'spec1' }],
+          keys: [],
         },
         {
           childId: 'B',
@@ -286,7 +298,8 @@ describe('Retain hierarchy even with arbitrary names', () => {
           ],
           depth: 1,
           label: 'B',
-          seriesIdentifier: { key: 'B', specId: 'spec1' },
+          seriesIdentifiers: [{ key: 'B', specId: 'spec1' }],
+          keys: [],
         },
       ]);
     });

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

@@ -311,7 +311,7 @@ describe('Legends', () => {
       formattedDataSeries: [{ key, specId }],
     } = computeSeriesDomainsSelector(store.getState());
 
-    store.dispatch(onToggleDeselectSeriesAction({ key, specId }));
+    store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
     const legend = computeLegendSelector(store.getState());
     const visibility = legend.map((item) => !item.isSeriesHidden);
     expect(visibility).toEqual([false, true]);

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

@@ -29,6 +29,7 @@ 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';
+import { groupBy } from '../utils/group_data_series';
 import {
   getSeriesIndex,
   getSeriesName,
@@ -104,11 +105,21 @@ export function computeLegend(
   const legendItems: LegendItem[] = [];
 
   dataSeries.forEach((series) => {
-    const { specId } = series;
+    const { specId, yAccessor } = series;
     const banded = isDataSeriesBanded(series);
     const key = getSeriesKey(series, series.groupId);
     const spec = getSpecsById<BasicSeriesSpec>(specs, specId);
-    const color = seriesColors.get(key) || defaultColor;
+    const dataSeriesKey = getSeriesKey(
+      {
+        specId: series.specId,
+        yAccessor: series.yAccessor,
+        splitAccessors: series.splitAccessors,
+      },
+      series.groupId,
+    );
+
+    const color = seriesColors.get(dataSeriesKey) || defaultColor;
+
     const hasSingleSeries = dataSeries.length === 1;
     const name = getSeriesName(series, hasSingleSeries, false, spec);
     const isSeriesHidden = deselectedDataSeries ? getSeriesIndex(deselectedDataSeries, series) >= 0 : false;
@@ -129,26 +140,28 @@ export function computeLegend(
     legendItems.push({
       color,
       label: labelY1,
-      seriesIdentifier,
+      seriesIdentifiers: [seriesIdentifier],
       childId: BandedAccessorType.Y1,
       isSeriesHidden,
       isItemHidden: hideInLegend,
       isToggleable: true,
       defaultExtra: getLegendExtra(showLegendExtra, spec.xScaleType, formatter, 'y1', lastValue),
       path: [{ index: 0, value: seriesIdentifier.key }],
+      keys: [specId, spec.groupId, yAccessor, ...series.splitAccessors.values()],
     });
     if (banded) {
       const labelY0 = getBandedLegendItemLabel(name, BandedAccessorType.Y0, postFixes);
       legendItems.push({
         color,
         label: labelY0,
-        seriesIdentifier,
+        seriesIdentifiers: [seriesIdentifier],
         childId: BandedAccessorType.Y0,
         isSeriesHidden,
         isItemHidden: hideInLegend,
         isToggleable: true,
         defaultExtra: getLegendExtra(showLegendExtra, spec.xScaleType, formatter, 'y0', lastValue),
         path: [{ index: 0, value: seriesIdentifier.key }],
+        keys: [specId, spec.groupId, yAccessor, ...series.splitAccessors.values()],
       });
     }
   });
@@ -159,5 +172,17 @@ export function computeLegend(
     return defaultXYLegendSeriesSort(aDs, bDs);
   });
 
-  return legendItems.sort((a, b) => legendSortFn(a.seriesIdentifier, b.seriesIdentifier));
+  return groupBy(
+    legendItems.sort((a, b) => legendSortFn(a.seriesIdentifiers[0], b.seriesIdentifiers[0])),
+    ({ keys, childId }) => {
+      return [...keys, childId].join('__'); // childId is used for band charts
+    },
+    true,
+  ).map((d) => {
+    return {
+      ...d[0],
+      seriesIdentifiers: d.map(({ seriesIdentifiers: [s] }) => s),
+      path: d.map(({ path: [p] }) => p),
+    };
+  });
 }

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

@@ -133,22 +133,26 @@ describe('Rendering utils', () => {
     const highlightedLegendItem: LegendItem = {
       color: '',
       label: '',
-      seriesIdentifier,
+      seriesIdentifiers: [seriesIdentifier],
       isSeriesHidden: false,
       defaultExtra: {
         formatted: null,
         raw: null,
         legendSizingLabel: null,
       },
       path: [],
+      keys: [],
     };
 
     const unhighlightedLegendItem: LegendItem = {
       ...highlightedLegendItem,
-      seriesIdentifier: {
-        ...seriesIdentifier,
-        key: 'not me',
-      },
+      seriesIdentifiers: [
+        {
+          ...seriesIdentifier,
+          key: 'not me',
+        },
+      ],
+      keys: [],
     };
 
     const sharedThemeStyle: SharedGeometryStateStyle = {

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

@@ -108,7 +108,9 @@ export function getGeometryStateStyle(
   const { default: defaultStyles, highlighted, unhighlighted } = sharedGeometryStyle;
 
   if (highlightedLegendItem) {
-    const isPartOfHighlightedSeries = seriesIdentifier.key === highlightedLegendItem.seriesIdentifier.key;
+    const isPartOfHighlightedSeries = highlightedLegendItem.seriesIdentifiers.some(
+      ({ key }) => key === seriesIdentifier.key,
+    );
 
     return isPartOfHighlightedSeries ? highlighted : unhighlighted;
   }