Skip to content

Commit

Permalink
feat(xy): render sorting (#2524)
Browse files Browse the repository at this point in the history
This enables sorting of defined series for ordering in rendering, legend and tooltip for xy charts.

BREAKING CHANGE: The way mixed stacked/nonstacked series are colored now is different from the previous behaviour. Now we color them not by their insert index but by the way we display them in the rendering: from the left to right, bottom top, stacked, nonstacked. This align correctly also the legend colors by default. This does **not** affect colors assigned via a `SeriesColorAccessor`.
  • Loading branch information
nickofthyme authored Oct 8, 2024
1 parent 47d43cc commit c514571
Show file tree
Hide file tree
Showing 29 changed files with 3,278 additions and 2,855 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions e2e/tests/bar_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,10 @@ test.describe('Bar series stories', () => {
'http://localhost:9001/?path=/story/bar-chart--min-height&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Banded=true&knob-Custom No Results message=No Results&knob-Nice y ticks=true&knob-Scale Type=linear&knob-Series Type=bar&knob-Show positive data=true&knob-Split=true&knob-logMinLimit=1&knob-minBarHeight=5&knob-scale=log&knob-scaleType=log',
);
});

test('should sort legend independent from render sort', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/bar-chart--rendering-sort&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-use default color palette=&knob-stacked=true&knob-enable sorting_Render=false&knob-reverse sort order_Render=&knob-order_Render[0]=debug&knob-order_Render[1]=info&knob-order_Render[2]=notice&knob-order_Render[3]=warn&knob-order_Render[4]=error&knob-order_Render[5]=crit&knob-order_Render[6]=alert&knob-order_Render[7]=emerg&knob-order_Render[8]=other&knob-enable sorting_Legend=true&knob-reverse sort order_Legend=&knob-order_Legend[0]=debug&knob-order_Legend[1]=info&knob-order_Legend[2]=notice&knob-order_Legend[3]=warn&knob-order_Legend[4]=error&knob-order_Legend[5]=crit&knob-order_Legend[6]=alert&knob-order_Legend[7]=emerg&knob-order_Legend[8]=other&knob-enable sorting_Tooltip=false&knob-reverse sort order_Tooltip=&knob-order_Tooltip[0]=debug&knob-order_Tooltip[1]=info&knob-order_Tooltip[2]=notice&knob-order_Tooltip[3]=warn&knob-order_Tooltip[4]=error&knob-order_Tooltip[5]=crit&knob-order_Tooltip[6]=alert&knob-order_Tooltip[7]=emerg&knob-order_Tooltip[8]=other',
);
});
});
2 changes: 1 addition & 1 deletion e2e/tests/performance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ test.describe('Performance', () => {
// eslint-disable-next-line no-console
console.log('window.performance.getEntriesByType("measure")', getAllMeasures);
// using these two values as min/max to understand if we are increasing or decreasing the rendering speed.
expect(getAllMeasures[0].duration).toBeGreaterThan(1400);
expect(getAllMeasures[0].duration).toBeGreaterThan(1000);
expect(getAllMeasures[0].duration).toBeLessThan(1600);
});
}
Expand Down
6 changes: 4 additions & 2 deletions packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2756,7 +2756,7 @@ export const Settings: (props: SFProps<SettingsSpec, keyof (typeof settingsBuild
// Warning: (ae-forgotten-export) The symbol "BuildProps" needs to be exported by the entry point index.d.ts
//
// @public (undocumented)
export const settingsBuildProps: BuildProps<SettingsSpec, "id" | "chartType" | "specType", "debug" | "locale" | "rotation" | "baseTheme" | "rendering" | "animateData" | "externalPointerEvents" | "pointBuffer" | "pointerUpdateTrigger" | "brushAxis" | "minBrushDelta" | "allowBrushingLastHistogramBin" | "ariaLabelHeadingLevel" | "ariaUseDefaultSummary" | "dow" | "showLegend" | "legendPosition" | "legendValues" | "legendMaxDepth" | "legendSize" | "flatLegend", "ariaDescription" | "ariaLabel" | "xDomain" | "theme" | "debugState" | "onProjectionClick" | "onElementClick" | "onElementOver" | "onElementOut" | "onBrushEnd" | "onPointerUpdate" | "onResize" | "onRenderChange" | "onWillRender" | "onProjectionAreaChange" | "onAnnotationClick" | "resizeDebounce" | "pointerUpdateDebounce" | "roundHistogramBrushValues" | "orderOrdinalBinsBy" | "noResults" | "ariaLabelledBy" | "ariaDescribedBy" | "ariaTableCaption" | "legendStrategy" | "onLegendItemOver" | "onLegendItemOut" | "onLegendItemClick" | "onLegendItemPlusClick" | "onLegendItemMinusClick" | "legendAction" | "legendColorPicker" | "legendSort" | "customLegend" | "legendTitle", never>;
export const settingsBuildProps: BuildProps<SettingsSpec, "id" | "chartType" | "specType", "debug" | "locale" | "rotation" | "baseTheme" | "rendering" | "animateData" | "externalPointerEvents" | "pointBuffer" | "pointerUpdateTrigger" | "brushAxis" | "minBrushDelta" | "allowBrushingLastHistogramBin" | "ariaLabelHeadingLevel" | "ariaUseDefaultSummary" | "dow" | "showLegend" | "legendPosition" | "legendValues" | "legendMaxDepth" | "legendSize" | "flatLegend", "ariaDescription" | "ariaLabel" | "xDomain" | "theme" | "debugState" | "onProjectionClick" | "onElementClick" | "onElementOver" | "onElementOut" | "onBrushEnd" | "onPointerUpdate" | "onResize" | "onRenderChange" | "onWillRender" | "onProjectionAreaChange" | "onAnnotationClick" | "resizeDebounce" | "pointerUpdateDebounce" | "roundHistogramBrushValues" | "orderOrdinalBinsBy" | "renderingSort" | "noResults" | "ariaLabelledBy" | "ariaDescribedBy" | "ariaTableCaption" | "legendStrategy" | "onLegendItemOver" | "onLegendItemOut" | "onLegendItemClick" | "onLegendItemPlusClick" | "onLegendItemMinusClick" | "legendAction" | "legendColorPicker" | "legendSort" | "customLegend" | "legendTitle", never>;

// @public (undocumented)
export type SettingsProps = ComponentProps<typeof Settings>;
Expand Down Expand Up @@ -2810,6 +2810,7 @@ export interface SettingsSpec extends Spec, LegendSpec {
pointerUpdateTrigger: PointerUpdateTrigger;
// (undocumented)
rendering: Rendering;
renderingSort?: SeriesCompareFn;
// @deprecated
resizeDebounce?: number;
// (undocumented)
Expand Down Expand Up @@ -3136,7 +3137,7 @@ export type TooltipAction<D extends BaseDatum = Datum, SI extends SeriesIdentifi
};

// @public
export const tooltipBuildProps: BuildProps<TooltipSpec<any, SeriesIdentifier>, "id" | "chartType" | "specType", "body" | "footer" | "header" | "type" | "snap" | "showNullValues" | "actions" | "actionsLoading" | "noActionsLoaded" | "actionPrompt" | "pinningPrompt" | "selectionPrompt" | "maxTooltipItems" | "maxVisibleTooltipItems", "fallbackPlacements" | "placement" | "offset" | "boundary" | "boundaryPadding" | "unit" | "headerFormatter" | "customTooltip" | "stickTo", never>;
export const tooltipBuildProps: BuildProps<TooltipSpec<any, SeriesIdentifier>, "id" | "chartType" | "specType", "body" | "footer" | "header" | "type" | "snap" | "showNullValues" | "actions" | "actionsLoading" | "noActionsLoaded" | "actionPrompt" | "pinningPrompt" | "selectionPrompt" | "maxTooltipItems" | "maxVisibleTooltipItems", "sort" | "fallbackPlacements" | "placement" | "offset" | "boundary" | "boundaryPadding" | "unit" | "headerFormatter" | "customTooltip" | "stickTo", never>;

// @public
export type TooltipCellStyle = Pick<CSSProperties, 'maxHeight' | 'textAlign' | 'padding' | 'paddingTop' | 'paddingRight' | 'paddingBottom' | 'paddingLeft'>;
Expand Down Expand Up @@ -3251,6 +3252,7 @@ export interface TooltipSpec<D extends BaseDatum = Datum, SI extends SeriesIdent
selectionPrompt: string;
showNullValues: boolean;
snap: boolean;
sort?: SeriesCompareFn;
stickTo?: TooltipStickTo;
type: TooltipType;
// @alpha
Expand Down
88 changes: 87 additions & 1 deletion packages/charts/src/chart_types/xy_chart/legend/legend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Position, RecursivePartial } from '../../../utils/common';
import { AxisStyle } from '../../../utils/themes/theme';
import { computeLegendSelector } from '../state/selectors/compute_legend';
import { computeSeriesDomainsSelector } from '../state/selectors/compute_series_domains';
import { getSeriesName } from '../utils/series';
import { getSeriesName, XYChartSeriesIdentifier } from '../utils/series';
import { AxisSpec, BasicSeriesSpec, SeriesType } from '../utils/specs';

const spec1: BasicSeriesSpec = {
Expand Down Expand Up @@ -203,6 +203,92 @@ describe('Legends', () => {
expect(legend).toHaveLength(4);
expect(legend).toMatchObject(expected);
});

it('should order legend values by default sort when no legend sort defined', () => {
MockStore.addSpecs(
[
MockSeriesSpec.bar({
yAccessors: ['y1'],
splitSeriesAccessors: ['g'],
data: [
{
x: 0,
y1: 1,
g: 'c',
},
{
x: 0,
y1: 1,
g: 'b',
},
{
x: 0,
y1: 1,
g: 'a',
},
],
}),
MockGlobalSpec.settings({
showLegend: true,
theme: { colors: { vizColors: ['red', 'blue', 'violet', 'green'] } },
renderingSort: (a: XYChartSeriesIdentifier, b: XYChartSeriesIdentifier) => {
const aG = a.splitAccessors.get('g') as string;
const bG = b.splitAccessors.get('g') as string;

return aG.localeCompare(bG);
},
}),
],
store,
);
const legend = computeLegendSelector(store.getState());

expect(legend).toMatchObject([{ label: 'a' }, { label: 'b' }, { label: 'c' }]);
});

it('should order legend values by legend sort over render sort', () => {
const sort = (order: 'asc' | 'desc') => (a: XYChartSeriesIdentifier, b: XYChartSeriesIdentifier) => {
const aG = a.splitAccessors.get('g') as string;
const bG = b.splitAccessors.get('g') as string;
return aG.localeCompare(bG) * (order === 'asc' ? 1 : -1);
};
MockStore.addSpecs(
[
MockSeriesSpec.bar({
yAccessors: ['y1'],
splitSeriesAccessors: ['g'],
data: [
{
x: 0,
y1: 1,
g: 'c',
},
{
x: 0,
y1: 1,
g: 'b',
},
{
x: 0,
y1: 1,
g: 'a',
},
],
}),
MockGlobalSpec.settings({
showLegend: true,
theme: { colors: { vizColors: ['red', 'blue', 'violet', 'green'] } },
renderingSort: sort('asc'),
legendSort: sort('desc'),
}),
],
store,
);
const legend = computeLegendSelector(store.getState());

expect(legend).toMatchObject([{ label: 'c' }, { label: 'b' }, { label: 'a' }]);
});

it('compute legend for multiple specs', () => {
MockStore.addSpecs(
[
Expand Down
17 changes: 9 additions & 8 deletions packages/charts/src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { SeriesKey, SeriesIdentifier } from '../../../common/series_id';
import { SettingsSpec } from '../../../specs';
import { isDefined, mergePartial } from '../../../utils/common';
import { BandedAccessorType } from '../../../utils/geometry';
import { getLegendCompareFn, SeriesCompareFn } from '../../../utils/series_sort';
import { SeriesCompareFn } from '../../../utils/series_sort';
import { PointStyle, Theme } from '../../../utils/themes/theme';
import { XDomain } from '../domains/types';
import { isDatumFilled } from '../rendering/utils';
Expand Down Expand Up @@ -102,7 +102,7 @@ export function computeLegend(
specs: BasicSeriesSpec[],
axesSpecs: AxisSpec[],
settingsSpec: SettingsSpec,
serialIdentifierDataSeriesMap: Record<string, DataSeries>,
seriesIdentifierDataSeriesMap: Record<string, DataSeries>,
theme: Theme,
deselectedDataSeries: SeriesIdentifier[] = [],
): LegendItem[] {
Expand Down Expand Up @@ -183,15 +183,16 @@ export function computeLegend(
}
});

const legendSortFn = getLegendCompareFn((a, b) => {
const aDs = serialIdentifierDataSeriesMap[a.key];
const bDs = serialIdentifierDataSeriesMap[b.key];
const baseLegendSortFn: SeriesCompareFn = (a, b) => {
const aDs = seriesIdentifierDataSeriesMap[a.key];
const bDs = seriesIdentifierDataSeriesMap[b.key];
return defaultXYLegendSeriesSort(aDs, bDs);
});
const sortFn: SeriesCompareFn = settingsSpec.legendSort ?? legendSortFn;
};
const legendSort = settingsSpec.legendSort ?? baseLegendSortFn;

return groupBy(
legendItems.sort((a, b) =>
a.seriesIdentifiers[0] && b.seriesIdentifiers[0] ? sortFn(a.seriesIdentifiers[0], b.seriesIdentifiers[0]) : 0,
a.seriesIdentifiers[0] && b.seriesIdentifiers[0] ? legendSort(a.seriesIdentifiers[0], b.seriesIdentifiers[0]) : 0,
),
({ keys, childId }) => {
return [...keys, childId].join('__'); // childId is used for band charts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { isNil, Rotation } from '../../../../utils/common';
import { isValidPointerOverEvent } from '../../../../utils/events';
import { IndexedGeometry } from '../../../../utils/geometry';
import { Point } from '../../../../utils/point';
import { getTooltipCompareFn } from '../../../../utils/series_sort';
import { SeriesCompareFn } from '../../../../utils/series_sort';
import { isPointOnGeometry } from '../../rendering/utils';
import { formatTooltipHeader, formatTooltipValue } from '../../tooltip/tooltip';
import { defaultXYLegendSeriesSort } from '../../utils/default_series_sort_fn';
Expand Down Expand Up @@ -89,7 +89,7 @@ function getTooltipAndHighlightFromValue(
hasSingleSeries: boolean,
scales: ComputedScales,
matchingGeoms: IndexedGeometry[],
serialIdentifierDataSeriesMap: Record<string, DataSeries>,
seriesIdentifierDataSeriesMap: Record<string, DataSeries>,
externalPointerEvent: PointerEvent | null,
tooltip: TooltipSpec,
): TooltipAndHighlightedGeoms {
Expand Down Expand Up @@ -192,20 +192,19 @@ function getTooltipAndHighlightFromValue(
header = null;
}

const tooltipSortFn = getTooltipCompareFn((a, b) => {
const aDs = serialIdentifierDataSeriesMap[a.key];
const bDs = serialIdentifierDataSeriesMap[b.key];
const baseTooltipSortFn: SeriesCompareFn = (a, b) => {
const aDs = seriesIdentifierDataSeriesMap[a.key];
const bDs = seriesIdentifierDataSeriesMap[b.key];
return defaultXYLegendSeriesSort(aDs, bDs);
});

};
const tooltipSortFn = tooltip.sort ?? settings.legendSort ?? baseTooltipSortFn;
const sortedTooltipValues = values.sort((a, b) => {
return tooltipSortFn(a.seriesIdentifier, b.seriesIdentifier);
});

return {
tooltip: {
header,
// to avoid creating a breaking change because of a different sorting order on tooltip
values: sortedTooltipValues,
},
highlightedGeometries,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,15 @@ exports[`Chart State utils should compute and format specifications for non stac
},
],
"groupId": "group1",
"insertIndex": 0,
"insertOrder": 0,
"isFiltered": false,
"isStacked": false,
"key": "groupId{group1}spec{spec1}yAccessor{y}splitAccessors{}",
"seriesKeys": [
"y",
],
"seriesType": "line",
"sortOrder": 0,
"spec": {
"chartType": "xy_axis",
"data": [
Expand Down Expand Up @@ -169,14 +170,15 @@ exports[`Chart State utils should compute and format specifications for non stac
},
],
"groupId": "group2",
"insertIndex": 1,
"insertOrder": 1,
"isFiltered": false,
"isStacked": false,
"key": "groupId{group2}spec{spec2}yAccessor{y}splitAccessors{}",
"seriesKeys": [
"y",
],
"seriesType": "line",
"sortOrder": 1,
"spec": {
"chartType": "xy_axis",
"data": [
Expand Down Expand Up @@ -281,7 +283,7 @@ exports[`Chart State utils should compute and format specifications for stacked
},
],
"groupId": "group2",
"insertIndex": 2,
"insertOrder": 2,
"isFiltered": false,
"isStacked": true,
"key": "groupId{group2}spec{spec2}yAccessor{y}splitAccessors{g-a}",
Expand All @@ -290,6 +292,7 @@ exports[`Chart State utils should compute and format specifications for stacked
"y",
],
"seriesType": "line",
"sortOrder": 2,
"spec": {
"chartType": "xy_axis",
"data": [
Expand Down Expand Up @@ -421,7 +424,7 @@ exports[`Chart State utils should compute and format specifications for stacked
},
],
"groupId": "group2",
"insertIndex": 3,
"insertOrder": 3,
"isFiltered": false,
"isStacked": true,
"key": "groupId{group2}spec{spec2}yAccessor{y}splitAccessors{g-b}",
Expand All @@ -430,6 +433,7 @@ exports[`Chart State utils should compute and format specifications for stacked
"y",
],
"seriesType": "line",
"sortOrder": 3,
"spec": {
"chartType": "xy_axis",
"data": [
Expand Down Expand Up @@ -570,7 +574,7 @@ exports[`Chart State utils should compute and format specifications for stacked
},
],
"groupId": "group1",
"insertIndex": 0,
"insertOrder": 0,
"isFiltered": false,
"isStacked": false,
"key": "groupId{group1}spec{spec1}yAccessor{y}splitAccessors{g-a}",
Expand All @@ -579,6 +583,7 @@ exports[`Chart State utils should compute and format specifications for stacked
"y",
],
"seriesType": "line",
"sortOrder": 0,
"spec": {
"chartType": "xy_axis",
"data": [
Expand Down Expand Up @@ -711,7 +716,7 @@ exports[`Chart State utils should compute and format specifications for stacked
},
],
"groupId": "group1",
"insertIndex": 1,
"insertOrder": 1,
"isFiltered": false,
"isStacked": false,
"key": "groupId{group1}spec{spec1}yAccessor{y}splitAccessors{g-b}",
Expand All @@ -720,6 +725,7 @@ exports[`Chart State utils should compute and format specifications for stacked
"y",
],
"seriesType": "line",
"sortOrder": 1,
"spec": {
"chartType": "xy_axis",
"data": [
Expand Down
21 changes: 10 additions & 11 deletions packages/charts/src/chart_types/xy_chart/state/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
PointGeometry,
} from '../../../../utils/geometry';
import { GroupId, SpecId } from '../../../../utils/ids';
import { getRenderingCompareFn } from '../../../../utils/series_sort';
import { SeriesCompareFn } from '../../../../utils/series_sort';
import { ColorConfig, Theme } from '../../../../utils/themes/theme';
import { XDomain } from '../../domains/types';
import { mergeXDomain } from '../../domains/x_domain';
Expand Down Expand Up @@ -120,15 +120,21 @@ export function computeSeriesDomains(
seriesSpecs: BasicSeriesSpec[],
scaleConfigs: ScaleConfigs,
annotations: AnnotationSpec[],
settingsSpec: Pick<SettingsSpec, 'orderOrdinalBinsBy' | 'locale'>,
settingsSpec: Pick<SettingsSpec, 'orderOrdinalBinsBy' | 'locale' | 'renderingSort'>,
deselectedDataSeries: SeriesIdentifier[] = [],
smallMultiples?: SmallMultiplesGroupBy,
): SeriesDomainsAndData {
const { orderOrdinalBinsBy, locale } = settingsSpec;
const { orderOrdinalBinsBy, locale, renderingSort } = settingsSpec;
const renderingSortFn: SeriesCompareFn =
renderingSort ??
((a, b) => {
return defaultXYSeriesSort(a as DataSeries, b as DataSeries);
});
const { dataSeries, xValues, fallbackScale, smHValues, smVValues } = getDataSeriesFromSpecs(
seriesSpecs,
deselectedDataSeries,
orderOrdinalBinsBy,
renderingSortFn,
smallMultiples,
);
// compute the x domain merging any custom domain
Expand All @@ -137,15 +143,8 @@ export function computeSeriesDomains(
// fill series with missing x values
const filledDataSeries = fillSeries(dataSeries, xValues, xDomain.type);

const seriesSortFn = getRenderingCompareFn((a: SeriesIdentifier, b: SeriesIdentifier) => {
return defaultXYSeriesSort(a as DataSeries, b as DataSeries);
});

const formattedDataSeries = getFormattedDataSeries(seriesSpecs, filledDataSeries, xValues, xDomain.type).sort(
seriesSortFn,
);
const formattedDataSeries = getFormattedDataSeries(seriesSpecs, filledDataSeries, xValues, xDomain.type);
const annotationYValueMap = getAnnotationYValueMap(annotations, scaleConfigs.y);

// let's compute the yDomains after computing all stacked values
const yDomains = mergeYDomain(scaleConfigs.y, formattedDataSeries, annotationYValueMap);

Expand Down
Loading

0 comments on commit c514571

Please sign in to comment.