From 7638ba4bd2e96b56a1e279d8021be20a9f4121eb Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Mon, 18 Dec 2023 22:24:16 +0100 Subject: [PATCH] cleanup defaultExtra type --- .../__snapshots__/partition.test.tsx.snap | 13 ----- .../state/selectors/compute_legend.ts | 1 - .../xy_chart/legend/legend.test.ts | 46 ----------------- .../src/chart_types/xy_chart/legend/legend.ts | 51 +++++-------------- .../xy_chart/rendering/rendering.test.ts | 5 -- .../selectors/get_legend_items_labels.ts | 5 +- .../xy_chart/state/utils/common.test.ts | 8 +-- packages/charts/src/common/legend.ts | 5 +- .../charts/src/components/legend/extra.tsx | 2 +- .../src/components/legend/legend_item.tsx | 4 +- .../charts/src/components/legend/utils.ts | 2 +- 11 files changed, 23 insertions(+), 119 deletions(-) diff --git a/packages/charts/src/chart_types/partition_chart/__snapshots__/partition.test.tsx.snap b/packages/charts/src/chart_types/partition_chart/__snapshots__/partition.test.tsx.snap index 11a5d6ef39..a94b5ac6ff 100644 --- a/packages/charts/src/chart_types/partition_chart/__snapshots__/partition.test.tsx.snap +++ b/packages/charts/src/chart_types/partition_chart/__snapshots__/partition.test.tsx.snap @@ -7,7 +7,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "2", - "legendSizingLabel": "2", "raw": 2, }, "depth": 0, @@ -39,7 +38,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -75,7 +73,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -111,7 +108,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "2", - "legendSizingLabel": "2", "raw": 2, }, "depth": 0, @@ -143,7 +139,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -179,7 +174,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -215,7 +209,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "2", - "legendSizingLabel": "2", "raw": 2, }, "depth": 0, @@ -247,7 +240,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -283,7 +275,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems all distinct "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -324,7 +315,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems special case: "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 0, @@ -356,7 +346,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems special case: "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, @@ -397,7 +386,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems special case: "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 0, @@ -429,7 +417,6 @@ exports[`Retain hierarchy even with arbitrary names getLegendItems special case: "color": "rgba(128, 0, 0, 0.5)", "defaultExtra": { "formatted": "1", - "legendSizingLabel": "1", "raw": 1, }, "depth": 1, diff --git a/packages/charts/src/chart_types/partition_chart/state/selectors/compute_legend.ts b/packages/charts/src/chart_types/partition_chart/state/selectors/compute_legend.ts index 3d70692861..aa19966389 100644 --- a/packages/charts/src/chart_types/partition_chart/state/selectors/compute_legend.ts +++ b/packages/charts/src/chart_types/partition_chart/state/selectors/compute_legend.ts @@ -126,7 +126,6 @@ function walkTree( defaultExtra: { raw: node[AGGREGATE_KEY], formatted: valueFormatter(node[AGGREGATE_KEY]), - legendSizingLabel: `${node[AGGREGATE_KEY]}`, }, }, node, diff --git a/packages/charts/src/chart_types/xy_chart/legend/legend.test.ts b/packages/charts/src/chart_types/xy_chart/legend/legend.test.ts index fc64d40ffb..5463965912 100644 --- a/packages/charts/src/chart_types/xy_chart/legend/legend.test.ts +++ b/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'; @@ -23,12 +22,6 @@ import { computeSeriesDomainsSelector } from '../state/selectors/compute_series_ import { getSeriesName } from '../utils/series'; import { AxisSpec, BasicSeriesSpec, SeriesType } from '../utils/specs'; -const nullDisplayValue = { - formatted: null, - raw: null, - legendSizingLabel: null, -}; - const spec1: BasicSeriesSpec = { chartType: ChartType.XYAxis, specType: SpecType.Series, @@ -105,7 +98,6 @@ describe('Legends', () => { isItemHidden: false, isSeriesHidden: false, isToggleable: true, - defaultExtra: nullDisplayValue, path: [{ index: 0, value: 'groupId{__global__}spec{spec1}yAccessor{y1}splitAccessors{}' }], }; expect(legend[0]).toMatchObject(expected); @@ -148,7 +140,6 @@ describe('Legends', () => { isItemHidden: false, isSeriesHidden: false, isToggleable: true, - defaultExtra: nullDisplayValue, path: [{ index: 0, value: 'groupId{__global__}spec{spec1}yAccessor{y1}splitAccessors{g-a}' }], }, { @@ -158,7 +149,6 @@ describe('Legends', () => { isItemHidden: false, isSeriesHidden: false, isToggleable: true, - defaultExtra: nullDisplayValue, path: [{ index: 0, value: 'groupId{__global__}spec{spec1}yAccessor{y2}splitAccessors{g-a}' }], }, { @@ -168,7 +158,6 @@ describe('Legends', () => { isItemHidden: false, isSeriesHidden: false, isToggleable: true, - defaultExtra: nullDisplayValue, path: [{ index: 0, value: 'groupId{__global__}spec{spec1}yAccessor{y1}splitAccessors{g-b}' }], }, { @@ -178,7 +167,6 @@ describe('Legends', () => { isItemHidden: false, isSeriesHidden: false, isToggleable: true, - defaultExtra: nullDisplayValue, path: [{ index: 0, value: 'groupId{__global__}spec{spec1}yAccessor{y2}splitAccessors{g-b}' }], }, ]; @@ -227,7 +215,6 @@ describe('Legends', () => { isItemHidden: false, isSeriesHidden: false, isToggleable: true, - defaultExtra: nullDisplayValue, path: [{ index: 0, value: 'groupId{__global__}spec{spec2}yAccessor{y}splitAccessors{}' }], }, ]; @@ -402,37 +389,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, - }); - }); }); diff --git a/packages/charts/src/chart_types/xy_chart/legend/legend.ts b/packages/charts/src/chart_types/xy_chart/legend/legend.ts index f55fe22574..25e10e9904 100644 --- a/packages/charts/src/chart_types/xy_chart/legend/legend.ts +++ b/packages/charts/src/chart_types/xy_chart/legend/legend.ts @@ -9,16 +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 { isDefined, mergePartial } from '../../../utils/common'; +import { SettingsSpec } from '../../../specs'; +import { isDefined, isFiniteNumber, mergePartial } from '../../../utils/common'; import { BandedAccessorType } from '../../../utils/geometry'; import { getLegendCompareFn, SeriesCompareFn } from '../../../utils/series_sort'; import { PointStyle, Theme } from '../../../utils/themes/theme'; 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'; @@ -63,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)) { @@ -141,14 +118,16 @@ export function computeLegend( const pointStyle = getPointStyle(spec, theme); - const itemValue = getLegendValue(series, xDomain, legendValueMode, (d) => { + const legendItemValue = 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) : ''; + const defaultExtra: LegendItem['defaultExtra'] = isFiniteNumber(legendItemValue) + ? { raw: legendItemValue, formatted: formatter(legendItemValue) } + : undefined; legendItems.push({ color, @@ -158,20 +137,18 @@ export function computeLegend( isSeriesHidden, isItemHidden: hideInLegend, isToggleable: true, - defaultExtra: { - raw: itemValue, - formatted: formattedItemValue, - legendSizingLabel: formattedItemValue, - }, + defaultExtra, path: [{ index: 0, value: seriesIdentifier.key }], keys: [specId, spec.groupId, yAccessor, ...series.splitAccessors.values()], pointStyle, }); if (banded) { - const bandedItemValue = getLegendValue(series, xDomain, legendValueMode, (d) => { + const bandLegendItemValue = getLegendValue(series, xDomain, legendValueMode, (d) => { return series.stackMode === StackMode.Percentage ? d.y0 : d.initialY0; }); - const bandedFormattedItemValue = bandedItemValue !== null ? formatter(bandedItemValue) : ''; + const bandDefaultExtra: LegendItem['defaultExtra'] = isFiniteNumber(bandLegendItemValue) + ? { raw: bandLegendItemValue, formatted: formatter(bandLegendItemValue) } + : undefined; const labelY0 = getBandedLegendItemLabel(name, BandedAccessorType.Y0, postFixes); legendItems.push({ @@ -182,11 +159,7 @@ export function computeLegend( isSeriesHidden, isItemHidden: hideInLegend, isToggleable: true, - defaultExtra: { - raw: bandedItemValue, - formatted: bandedFormattedItemValue, - legendSizingLabel: bandedFormattedItemValue, - }, + defaultExtra: bandDefaultExtra, path: [{ index: 0, value: seriesIdentifier.key }], keys: [specId, spec.groupId, yAccessor, ...series.splitAccessors.values()], pointStyle, diff --git a/packages/charts/src/chart_types/xy_chart/rendering/rendering.test.ts b/packages/charts/src/chart_types/xy_chart/rendering/rendering.test.ts index c132e7f886..4838fcc48e 100644 --- a/packages/charts/src/chart_types/xy_chart/rendering/rendering.test.ts +++ b/packages/charts/src/chart_types/xy_chart/rendering/rendering.test.ts @@ -128,11 +128,6 @@ describe('Rendering utils', () => { label: '', seriesIdentifiers: [seriesIdentifier], isSeriesHidden: false, - defaultExtra: { - formatted: null, - raw: null, - legendSizingLabel: null, - }, path: [], keys: [], }; diff --git a/packages/charts/src/chart_types/xy_chart/state/selectors/get_legend_items_labels.ts b/packages/charts/src/chart_types/xy_chart/state/selectors/get_legend_items_labels.ts index f70c3384e1..dae202929f 100644 --- a/packages/charts/src/chart_types/xy_chart/state/selectors/get_legend_items_labels.ts +++ b/packages/charts/src/chart_types/xy_chart/state/selectors/get_legend_items_labels.ts @@ -16,10 +16,7 @@ export const getLegendItemsLabelsSelector = createCustomCachedSelector( [computeLegendSelector, getSettingsSpecSelector], (legendItems, { showLegendExtra }): LegendItemLabel[] => legendItems.map(({ label, defaultExtra }) => ({ - label: - defaultExtra && (defaultExtra.legendSizingLabel ?? null) !== null - ? `${label}${showLegendExtra ? defaultExtra.legendSizingLabel : ''}` - : label, + label: `${label}${showLegendExtra ? defaultExtra?.formatted ?? '' : ''}`, depth: 0, })), ); diff --git a/packages/charts/src/chart_types/xy_chart/state/utils/common.test.ts b/packages/charts/src/chart_types/xy_chart/state/utils/common.test.ts index a2ecbf2385..ec419d7b25 100644 --- a/packages/charts/src/chart_types/xy_chart/state/utils/common.test.ts +++ b/packages/charts/src/chart_types/xy_chart/state/utils/common.test.ts @@ -133,7 +133,7 @@ describe('Type Checks', () => { specId: 'bars', }, ], - defaultExtra: { raw: 6, formatted: '6.00', legendSizingLabel: '6.00' }, + defaultExtra: { raw: 6, formatted: '6.00' }, isSeriesHidden: true, path: [], keys: [], @@ -147,7 +147,7 @@ describe('Type Checks', () => { specId: 'bars', }, ], - defaultExtra: { raw: 2, formatted: '2.00', legendSizingLabel: '2.00' }, + defaultExtra: { raw: 2, formatted: '2.00' }, isSeriesHidden: true, path: [], keys: [], @@ -166,7 +166,7 @@ describe('Type Checks', () => { specId: 'bars', }, ], - defaultExtra: { raw: 6, formatted: '6.00', legendSizingLabel: '6.00' }, + defaultExtra: { raw: 6, formatted: '6.00' }, isSeriesHidden: false, path: [], keys: [], @@ -180,7 +180,7 @@ describe('Type Checks', () => { specId: 'bars', }, ], - defaultExtra: { raw: 2, formatted: '2.00', legendSizingLabel: '2.00' }, + defaultExtra: { raw: 2, formatted: '2.00' }, isSeriesHidden: true, path: [], keys: [], diff --git a/packages/charts/src/common/legend.ts b/packages/charts/src/common/legend.ts index f67c8253f3..d75fb1ff17 100644 --- a/packages/charts/src/common/legend.ts +++ b/packages/charts/src/common/legend.ts @@ -31,9 +31,8 @@ export type LegendItem = { isSeriesHidden?: boolean; isItemHidden?: boolean; defaultExtra?: { - raw: number | null; - formatted: number | string | null; - legendSizingLabel: number | string | null; + raw: number; + formatted: string; }; // TODO: Remove when partition layers are toggleable isToggleable?: boolean; diff --git a/packages/charts/src/components/legend/extra.tsx b/packages/charts/src/components/legend/extra.tsx index d24a9fc07d..cad6e492b5 100644 --- a/packages/charts/src/components/legend/extra.tsx +++ b/packages/charts/src/components/legend/extra.tsx @@ -13,7 +13,7 @@ import React from 'react'; * @param extra * @param isSeriesHidden */ -export function renderExtra(extra: string | number) { +export function renderExtra(extra: string) { return (
{extra} diff --git a/packages/charts/src/components/legend/legend_item.tsx b/packages/charts/src/components/legend/legend_item.tsx index db92a91554..de9f9bfde8 100644 --- a/packages/charts/src/components/legend/legend_item.tsx +++ b/packages/charts/src/components/legend/legend_item.tsx @@ -190,7 +190,7 @@ export class LegendListItem extends Component 'echLegendItem--vertical': positionConfig.direction === LayoutDirection.Vertical, }); const hasColorPicker = Boolean(colorPicker); - const extra = showExtra && getExtra(extraValues, item, totalItems); + const style: CSSProperties = flatLegend ? {} : { @@ -225,7 +225,7 @@ export class LegendListItem extends Component onToggle={this.onLabelToggle(seriesIdentifiers)} isSeriesHidden={isSeriesHidden} /> - {extra && !isSeriesHidden && renderExtra(extra)} + {showExtra && !isSeriesHidden && renderExtra(getExtra(extraValues, item, totalItems))} {Action && (
diff --git a/packages/charts/src/components/legend/utils.ts b/packages/charts/src/components/legend/utils.ts index 2807048a06..6157f6a008 100644 --- a/packages/charts/src/components/legend/utils.ts +++ b/packages/charts/src/components/legend/utils.ts @@ -19,5 +19,5 @@ export function getExtra(extraValues: Map, item: const extraValueKey = path.map(({ index }) => index).join('__'); const itemExtraValues = extraValues.has(extraValueKey) ? extraValues.get(extraValueKey) : extraValues.get(key); const actionExtra = childId !== undefined && itemExtraValues?.get(childId); - return actionExtra ?? (extraValues.size === totalItems ? defaultExtra?.formatted : null) ?? ''; + return actionExtra && extraValues.size === totalItems ? defaultExtra?.formatted ?? '' : ''; }