Skip to content

Commit

Permalink
cleanup defaultExtra type
Browse files Browse the repository at this point in the history
  • Loading branch information
markov00 committed Dec 18, 2023
1 parent 1df6977 commit 7638ba4
Show file tree
Hide file tree
Showing 11 changed files with 23 additions and 119 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ function walkTree(
defaultExtra: {
raw: node[AGGREGATE_KEY],
formatted: valueFormatter(node[AGGREGATE_KEY]),
legendSizingLabel: `${node[AGGREGATE_KEY]}`,
},
},
node,
Expand Down
46 changes: 0 additions & 46 deletions packages/charts/src/chart_types/xy_chart/legend/legend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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}' }],
},
{
Expand All @@ -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}' }],
},
{
Expand All @@ -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}' }],
},
{
Expand All @@ -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}' }],
},
];
Expand Down Expand Up @@ -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{}' }],
},
];
Expand Down Expand Up @@ -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,
});
});
});
51 changes: 12 additions & 39 deletions packages/charts/src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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,
Expand All @@ -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({
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,6 @@ describe('Rendering utils', () => {
label: '',
seriesIdentifiers: [seriesIdentifier],
isSeriesHidden: false,
defaultExtra: {
formatted: null,
raw: null,
legendSizingLabel: null,
},
path: [],
keys: [],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})),
);
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand Down
5 changes: 2 additions & 3 deletions packages/charts/src/common/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 7638ba4

Please sign in to comment.