[Lens] Fix coloring/palette assignment on partition charts (#215426)

Fixes an issue where reordering the groups within a layer would incorrectly assign the color mapping to a group other than the first.
This commit is contained in:
Nick Partridge 2025-03-27 14:29:01 -05:00 committed by GitHub
parent f7ef9602a5
commit 2cbfe8641c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 301 additions and 191 deletions

View file

@ -0,0 +1,127 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import React from 'react';
import { render, screen } from '@testing-library/react';
import { createMockDatasource, createMockFramePublicAPI } from '../../mocks';
import { PieVisualizationState } from '../../../common/types';
import { DimensionEditor, DimensionEditorProps } from './dimension_editor';
import { getKbnPalettes } from '@kbn/palettes';
import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
const darkMode = false;
const paletteServiceMock = chartPluginMock.createPaletteRegistry();
const palettes = getKbnPalettes({ name: 'borealis', darkMode });
describe('DimensionEditor', () => {
let defaultState: PieVisualizationState;
let defaultProps: DimensionEditorProps;
let buildProps: (props?: Partial<DimensionEditorProps>) => DimensionEditorProps;
beforeEach(() => {
defaultState = {
shape: 'pie',
layers: [
{
layerId: 'layer-id',
primaryGroups: ['group-1', 'group-2'],
metrics: ['metrics-id'],
numberDisplay: 'percent',
categoryDisplay: 'default',
legendDisplay: 'default',
nestedLegend: false,
layerType: 'data',
colorMapping: {
assignments: [],
specialAssignments: [
{ rule: { type: 'other' }, color: { type: 'loop' }, touched: false },
],
paletteId: 'default',
colorMode: { type: 'categorical' },
},
},
],
};
const mockFrame = createMockFramePublicAPI();
mockFrame.datasourceLayers = Object.fromEntries(
defaultState.layers.map(({ layerId: id }) => [id, createMockDatasource(id).publicAPIMock])
);
defaultProps = {
state: defaultState,
layerId: defaultState.layers[0].layerId,
accessor: defaultState.layers[0].primaryGroups[0],
frame: mockFrame,
datasource: createMockDatasource().publicAPIMock,
groupId: 'primaryGroups',
setState: jest.fn(),
addLayer: jest.fn(),
removeLayer: jest.fn(),
panelRef: { current: null },
palettes,
isDarkMode: darkMode,
paletteService: paletteServiceMock,
};
buildProps = (props = {}) => {
const state = props.state ?? defaultState;
const layerId = props.layerId ?? state.layers[0].layerId;
const accessor =
props.accessor ??
state.layers.find((l) => l.layerId === layerId)?.primaryGroups[0] ??
'accessor-id';
const frame = props.frame ?? mockFrame;
const datasource = props.datasource ?? frame.datasourceLayers[layerId];
return {
...defaultProps,
...props,
state,
layerId,
accessor,
frame,
datasource,
};
};
});
const renderDimensionEditor = (propOverrides: Partial<DimensionEditorProps> = {}) => {
const props = buildProps(propOverrides);
render(<DimensionEditor {...props} />);
return props;
};
describe('Dimension Editor', () => {
describe('Color mapping', () => {
test.each([
['show', 'first', 1],
['hide', 'second', 2],
])('should %s color mapping row for the %s group', (a, b, groupNumber) => {
const layer = defaultProps.state.layers.find((l) => l.layerId === defaultProps.layerId)!;
const primaryGroups = layer.primaryGroups.slice();
layer.primaryGroups.reverse(); // should not care about this order
defaultProps.frame.datasourceLayers[layer.layerId]!.getTableSpec = () => {
return primaryGroups.map((id) => ({ columnId: id, fields: [] }));
};
renderDimensionEditor({ accessor: `group-${groupNumber}` });
const colorMappingBtn = screen.queryByRole('button', { name: 'Edit colors' });
if (groupNumber === 1) {
expect(colorMappingBtn).toBeInTheDocument();
} else {
expect(colorMappingBtn).not.toBeInTheDocument();
}
});
});
});
});

View file

@ -34,8 +34,9 @@ import {
isCollapsed,
} from './visualization';
import { trackUiCounterEvents } from '../../lens_ui_telemetry';
import { getSortedAccessorsForGroup } from './to_expression';
type DimensionEditorProps = VisualizationDimensionEditorProps<PieVisualizationState> & {
export type DimensionEditorProps = VisualizationDimensionEditorProps<PieVisualizationState> & {
paletteService: PaletteRegistry;
palettes: KbnPalettes;
isDarkMode: boolean;
@ -102,9 +103,12 @@ export function DimensionEditor(props: DimensionEditorProps) {
return null;
}
const firstNonCollapsedColumnId = currentLayer.primaryGroups.find(
(id) => !isCollapsed(id, currentLayer)
const originalGroupOrder = getSortedAccessorsForGroup(
props.datasource,
currentLayer,
'primaryGroups'
);
const firstNonCollapsedColumnId = originalGroupOrder.find((id) => !isCollapsed(id, currentLayer));
const showColorPicker =
currentLayer.metrics.includes(props.accessor) && currentLayer.allowMultipleMetrics;

View file

@ -246,11 +246,9 @@ describe('pie_visualization', () => {
frame: mockFrame(),
});
expect(newState.layers[0].colorsByDimension).toMatchInlineSnapshot(`
Object {
"1": "custom-color1",
}
`);
expect(newState.layers[0].colorsByDimension).toEqual({
'1': 'custom-color1',
});
});
it('removes custom palette if removing final slice-by dimension in multi-metric chart', () => {
const state = getExampleState();
@ -285,6 +283,7 @@ describe('pie_visualization', () => {
describe('#getConfiguration', () => {
describe('assigning icons to accessors', () => {
const colIds = ['1', '2', '3', '4'];
const randomColOrder = colIds.slice().reverse();
const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));
@ -296,7 +295,7 @@ describe('pie_visualization', () => {
it('applies palette and collapse icons for single slice-by group', () => {
const state = getExampleState();
state.layers[0].primaryGroups = colIds;
state.layers[0].primaryGroups = randomColOrder; // should get order from datasource
state.layers[0].collapseFns = {
'1': 'sum',
'3': 'max',
@ -308,30 +307,24 @@ describe('pie_visualization', () => {
});
// palette should be assigned to the first non-collapsed dimension
expect(configuration.groups[0].accessors).toMatchInlineSnapshot(`
Array [
Object {
"columnId": "1",
"triggerIconType": "aggregate",
},
Object {
"columnId": "2",
"palette": Array [
"red",
"black",
],
"triggerIconType": "colorBy",
},
Object {
"columnId": "3",
"triggerIconType": "aggregate",
},
Object {
"columnId": "4",
"triggerIconType": undefined,
},
]
`);
expect(configuration.groups[0].accessors).toEqual([
{
columnId: '1',
triggerIconType: 'aggregate',
},
{
columnId: '2',
palette: ['red', 'black'],
triggerIconType: 'colorBy',
},
{
columnId: '3',
triggerIconType: 'aggregate',
},
{
columnId: '4',
},
]);
});
it('applies palette and collapse icons with multiple slice-by groups (mosaic)', () => {
@ -349,35 +342,30 @@ describe('pie_visualization', () => {
layerId: mosaicState.layers[0].layerId,
});
expect(mosaicConfiguration.groups.map(({ accessors }) => accessors)).toMatchInlineSnapshot(`
Array [
Array [
Object {
"columnId": "1",
"triggerIconType": "aggregate",
},
Object {
"columnId": "2",
"palette": Array [
"red",
"black",
],
"triggerIconType": "colorBy",
},
],
Array [
Object {
"columnId": "3",
"triggerIconType": "aggregate",
},
Object {
"columnId": "4",
"triggerIconType": undefined,
},
],
Array [],
]
`);
expect(mosaicConfiguration.groups.map(({ accessors }) => accessors)).toEqual([
[
{
columnId: '1',
triggerIconType: 'aggregate',
},
{
columnId: '2',
palette: ['red', 'black'],
triggerIconType: 'colorBy',
},
],
[
{
columnId: '3',
triggerIconType: 'aggregate',
},
{
columnId: '4',
triggerIconType: undefined,
},
],
[],
]);
});
it('applies color swatch icons with multiple metrics', () => {
@ -393,66 +381,62 @@ describe('pie_visualization', () => {
layerId: state.layers[0].layerId,
});
expect(config.groups.map(({ accessors }) => accessors)).toMatchInlineSnapshot(`
Array [
Array [],
Array [
Object {
"color": "overridden-color",
"columnId": "1",
"triggerIconType": "color",
},
Object {
"color": "black",
"columnId": "2",
"triggerIconType": "color",
},
Object {
"color": "black",
"columnId": "3",
"triggerIconType": "color",
},
Object {
"color": "black",
"columnId": "4",
"triggerIconType": "color",
},
],
]
`);
expect(config.groups.map(({ accessors }) => accessors)).toEqual([
[],
[
{
color: 'overridden-color',
columnId: '1',
triggerIconType: 'color',
},
{
color: 'black',
columnId: '2',
triggerIconType: 'color',
},
{
color: 'black',
columnId: '3',
triggerIconType: 'color',
},
{
color: 'black',
columnId: '4',
triggerIconType: 'color',
},
],
]);
const palette = paletteServiceMock.get('default');
expect((palette.getCategoricalColor as jest.Mock).mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Array [
Object {
"name": "Label for 2",
"rankAtDepth": 1,
"totalSeriesAtDepth": 4,
},
],
expect((palette.getCategoricalColor as jest.Mock).mock.calls).toEqual([
[
[
{
name: 'Label for 2',
rankAtDepth: 1,
totalSeriesAtDepth: 4,
},
],
Array [
Array [
Object {
"name": "Label for 3",
"rankAtDepth": 2,
"totalSeriesAtDepth": 4,
},
],
],
[
[
{
name: 'Label for 3',
rankAtDepth: 2,
totalSeriesAtDepth: 4,
},
],
Array [
Array [
Object {
"name": "Label for 4",
"rankAtDepth": 3,
"totalSeriesAtDepth": 4,
},
],
],
[
[
{
name: 'Label for 4',
rankAtDepth: 3,
totalSeriesAtDepth: 4,
},
],
]
`);
],
]);
});
it("applies color swatch icons on multiple metrics if there's a collapsed slice-by", () => {
@ -472,30 +456,28 @@ describe('pie_visualization', () => {
layerId: state.layers[0].layerId,
});
expect(findMetricGroup(config)?.accessors).toMatchInlineSnapshot(`
Array [
Object {
"color": "overridden-color",
"columnId": "1",
"triggerIconType": "color",
},
Object {
"color": "black",
"columnId": "2",
"triggerIconType": "color",
},
Object {
"color": "black",
"columnId": "3",
"triggerIconType": "color",
},
Object {
"color": "black",
"columnId": "4",
"triggerIconType": "color",
},
]
`);
expect(findMetricGroup(config)?.accessors).toEqual([
{
color: 'overridden-color',
columnId: '1',
triggerIconType: 'color',
},
{
color: 'black',
columnId: '2',
triggerIconType: 'color',
},
{
color: 'black',
columnId: '3',
triggerIconType: 'color',
},
{
color: 'black',
columnId: '4',
triggerIconType: 'color',
},
]);
expect(palette.getCategoricalColor).toHaveBeenCalledTimes(3); // one for each of the defaultly assigned colors
});
@ -515,22 +497,20 @@ describe('pie_visualization', () => {
layerId: state.layers[0].layerId,
});
expect(findMetricGroup(config)?.accessors).toMatchInlineSnapshot(`
Array [
Object {
"columnId": "2",
"triggerIconType": "disabled",
},
Object {
"columnId": "3",
"triggerIconType": "disabled",
},
Object {
"columnId": "4",
"triggerIconType": "disabled",
},
]
`);
expect(findMetricGroup(config)?.accessors).toEqual([
{
columnId: '2',
triggerIconType: 'disabled',
},
{
columnId: '3',
triggerIconType: 'disabled',
},
{
columnId: '4',
triggerIconType: 'disabled',
},
]);
const palette = paletteServiceMock.get('default');
expect(palette.getCategoricalColor).not.toHaveBeenCalled();
@ -563,30 +543,28 @@ describe('pie_visualization', () => {
layerId: state.layers[0].layerId,
});
expect(findMetricGroup(config)?.accessors).toMatchInlineSnapshot(`
Array [
Object {
"color": "color 1",
"columnId": "4",
"triggerIconType": "color",
},
Object {
"color": "color 2",
"columnId": "3",
"triggerIconType": "color",
},
Object {
"color": "color 3",
"columnId": "2",
"triggerIconType": "color",
},
Object {
"color": "color 4",
"columnId": "1",
"triggerIconType": "color",
},
]
`);
expect(findMetricGroup(config)?.accessors).toEqual([
{
color: 'color 1',
columnId: '4',
triggerIconType: 'color',
},
{
color: 'color 2',
columnId: '3',
triggerIconType: 'color',
},
{
color: 'color 3',
columnId: '2',
triggerIconType: 'color',
},
{
color: 'color 4',
columnId: '1',
triggerIconType: 'color',
},
]);
});
describe('dimension limits', () => {

View file

@ -215,21 +215,22 @@ export const getPieVisualization = ({
const getPrimaryGroupConfig = (): VisualizationDimensionGroupConfig => {
const originalOrder = getSortedAccessorsForGroup(datasource, layer, 'primaryGroups');
const firstNonCollapsedColumnId = originalOrder.find((id) => !isCollapsed(id, layer));
// When we add a column it could be empty, and therefore have no order
const accessors = originalOrder.map<AccessorConfig>((accessor) => ({
columnId: accessor,
triggerIconType: isCollapsed(accessor, layer) ? 'aggregate' : undefined,
...(isCollapsed(accessor, layer)
? {
triggerIconType: 'aggregate',
}
: firstNonCollapsedColumnId === accessor
? {
triggerIconType: 'colorBy',
palette: colors,
}
: undefined),
}));
const firstNonCollapsedColumnId = layer.primaryGroups.find((id) => !isCollapsed(id, layer));
accessors.forEach((accessorConfig) => {
if (firstNonCollapsedColumnId === accessorConfig.columnId) {
accessorConfig.triggerIconType = 'colorBy';
accessorConfig.palette = colors;
}
});
const primaryGroupConfigBaseProps = {
groupId: 'primaryGroups',
accessors,