[Lens] Fix partition chart color assignments (#207178)

Fixes the color assignment for partition charts consistent with the legend ordering. Aligns legacy and color mapping color logic.
This commit is contained in:
Krzysztof Kowalczyk 2025-03-04 18:20:43 +01:00 committed by GitHub
parent 5903c7a552
commit 28dc0f6ffc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 351 additions and 423 deletions

View file

@ -12,401 +12,352 @@ import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
import { byDataColorPaletteMap, SimplifiedArrayNode } from './get_color';
import type { SeriesLayer } from '@kbn/coloring';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { fieldFormatsMock } from '@kbn/field-formats-plugin/common/mocks';
import type { DataPublicPluginStart } from '@kbn/data-plugin/public';
import { getColor } from './get_color';
import { createMockVisData, createMockBucketColumns, createMockPieParams } from '../../mocks';
import { generateFormatters } from '../formatters';
import { ChartTypes } from '../../../common/types';
import { getDistinctSeries } from '..';
import { getColorCategories } from '@kbn/chart-expressions-common';
describe('#byDataColorPaletteMap', () => {
let paletteDefinition: PaletteDefinition;
let palette: PaletteOutput;
const visData = createMockVisData();
const defaultFormatter = jest.fn((...args) => fieldFormatsMock.deserialize(...args));
const formatters = generateFormatters(visData, defaultFormatter);
describe('get color', () => {
describe('#byDataColorPaletteMap', () => {
let paletteDefinition: PaletteDefinition;
let palette: PaletteOutput;
let colorIndexMap: Map<string, number>;
const visData = createMockVisData();
const categories = getColorCategories(visData.rows, visData.columns[1]?.id);
beforeEach(() => {
paletteDefinition = chartPluginMock.createPaletteRegistry().get('default');
palette = { type: 'palette' } as PaletteOutput;
});
beforeEach(() => {
paletteDefinition = chartPluginMock.createPaletteRegistry().get('default');
palette = { type: 'palette' } as PaletteOutput;
colorIndexMap = new Map(categories.map((c, i) => [String(c), i]));
});
it('should create byDataColorPaletteMap', () => {
expect(
byDataColorPaletteMap(
visData.rows,
visData.columns[0],
it('should create byDataColorPaletteMap', () => {
const colorPaletteMap = byDataColorPaletteMap(paletteDefinition, palette, colorIndexMap);
expect(colorPaletteMap.getColor).toEqual(expect.any(Function));
});
it('should get color', () => {
const colorPaletteMap = byDataColorPaletteMap(paletteDefinition, palette, colorIndexMap);
expect(colorPaletteMap.getColor('Logstash Airways')).toBe('black');
});
it('should cache duplicate values', () => {
const colorPaletteMap = byDataColorPaletteMap(paletteDefinition, palette, colorIndexMap);
colorPaletteMap.getColor('JetBeats');
colorPaletteMap.getColor('JetBeats');
expect(paletteDefinition.getCategoricalColor).toBeCalledTimes(1);
});
it('should order rankAtDepth based on colorIndexMap for each value', () => {
const customColorIndexMap = new Map<string, number>();
customColorIndexMap.set('Logstash Airways', 0);
customColorIndexMap.set('empty - 1', 1);
customColorIndexMap.set('JetBeats', 2);
customColorIndexMap.set('empty - 2', 3);
customColorIndexMap.set('__other__', 5);
const colorPaletteMap = byDataColorPaletteMap(
paletteDefinition,
palette,
formatters,
fieldFormatsMock
)
).toMatchInlineSnapshot(`
Object {
"getColor": [Function],
}
`);
customColorIndexMap
);
// should produce same color regardless of call order
['__other__', 'JetBeats', 'Logstash Airways'].forEach((key) => {
colorPaletteMap.getColor(key);
});
expect(paletteDefinition.getCategoricalColor).toHaveBeenNthCalledWith(
1,
[{ name: '__other__', rankAtDepth: 5, totalSeriesAtDepth: 5 }],
{ behindText: false },
undefined
);
expect(paletteDefinition.getCategoricalColor).toHaveBeenNthCalledWith(
2,
[{ name: 'JetBeats', rankAtDepth: 2, totalSeriesAtDepth: 5 }],
{ behindText: false },
undefined
);
expect(paletteDefinition.getCategoricalColor).toHaveBeenNthCalledWith(
3,
[{ name: 'Logstash Airways', rankAtDepth: 0, totalSeriesAtDepth: 5 }],
{ behindText: false },
undefined
);
});
});
it('should get color', () => {
const colorPaletteMap = byDataColorPaletteMap(
visData.rows,
visData.columns[0],
paletteDefinition,
palette,
formatters,
fieldFormatsMock
);
describe('#getColor', () => {
const visData = createMockVisData();
const buckets = createMockBucketColumns();
const visParams = createMockPieParams();
const colors = ['color1', 'color2', 'color3', 'color4'];
const categories = (chartType?: ChartTypes) =>
chartType === ChartTypes.MOSAIC && visData.columns.length === 2
? getColorCategories(visData.rows, visData.columns[1]?.id)
: getColorCategories(visData.rows, visData.columns[0]?.id);
const colorIndexMap = (chartType?: ChartTypes) =>
new Map(categories(chartType).map((d, i) => [d[0], i]));
const dataMock = dataPluginMock.createStartContract();
interface RangeProps {
gte: number;
lt: number;
}
const distinctSeries = getDistinctSeries(visData.rows, buckets);
const dataLength = { columnsLength: buckets.length, rowsLength: visData.rows.length };
expect(colorPaletteMap.getColor('Logstash Airways')).toBe('black');
});
it('should return undefined in case if values not in datatable', () => {
const colorPaletteMap = byDataColorPaletteMap(
visData.rows,
visData.columns[0],
paletteDefinition,
palette,
formatters,
fieldFormatsMock
);
expect(colorPaletteMap.getColor('wrong')).toBeUndefined();
});
it('should increase rankAtDepth for each new value', () => {
const colorPaletteMap = byDataColorPaletteMap(
visData.rows,
visData.columns[0],
paletteDefinition,
palette,
formatters,
fieldFormatsMock
);
colorPaletteMap.getColor('Logstash Airways');
colorPaletteMap.getColor('JetBeats');
expect(paletteDefinition.getCategoricalColor).toHaveBeenNthCalledWith(
1,
[{ name: 'Logstash Airways', rankAtDepth: 0, totalSeriesAtDepth: 4 }],
{ behindText: false },
undefined
);
expect(paletteDefinition.getCategoricalColor).toHaveBeenNthCalledWith(
2,
[{ name: 'JetBeats', rankAtDepth: 1, totalSeriesAtDepth: 4 }],
{ behindText: false },
undefined
);
});
});
describe('getColor', () => {
const visData = createMockVisData();
const buckets = createMockBucketColumns();
const visParams = createMockPieParams();
const colors = ['color1', 'color2', 'color3', 'color4'];
const dataMock = dataPluginMock.createStartContract();
interface RangeProps {
gte: number;
lt: number;
}
const defaultFormatter = jest.fn((...args) => fieldFormatsMock.deserialize(...args));
const formatters = generateFormatters(visData, defaultFormatter);
const distinctSeries = getDistinctSeries(visData.rows, buckets);
const dataLength = { columnsLength: buckets.length, rowsLength: visData.rows.length };
dataMock.fieldFormats = {
deserialize: jest.fn(() => ({
convert: jest.fn((s: RangeProps) => {
return `${s.gte} and < ${s.lt}`;
}),
})),
} as unknown as DataPublicPluginStart['fieldFormats'];
const getPaletteRegistry = () => {
const mockPalette1: jest.Mocked<PaletteDefinition> = {
id: 'default',
title: 'My Palette',
getCategoricalColor: jest.fn((layer: SeriesLayer[]) => colors[layer[0].rankAtDepth]),
getCategoricalColors: jest.fn((num: number) => colors),
toExpression: jest.fn(() => ({
type: 'expression',
chain: [
{
type: 'function',
function: 'system_palette',
arguments: {
name: ['default'],
},
},
],
dataMock.fieldFormats = {
deserialize: jest.fn(() => ({
convert: jest.fn((s: RangeProps) => {
return `${s.gte} and < ${s.lt}`;
}),
})),
};
} as unknown as DataPublicPluginStart['fieldFormats'];
return {
get: () => mockPalette1,
getAll: () => [mockPalette1],
};
};
it('should return the correct color based on the parent sortIndex', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [
['ES-Air', undefined],
['Kibana Airlines', undefined],
],
depth: 0,
sortIndex: 0,
},
children: [],
};
const color = getColor(
ChartTypes.PIE,
'ES-Air',
d,
0,
false,
{},
distinctSeries,
dataLength,
visParams,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(color).toEqual(colors[0]);
});
it('slices with the same label should have the same color for small multiples', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [
['ES-Air', undefined],
['Kibana Airlines', undefined],
],
depth: 0,
sortIndex: 0,
},
children: [],
};
const color = getColor(
ChartTypes.PIE,
'ES-Air',
d,
0,
true,
{},
distinctSeries,
dataLength,
visParams,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(color).toEqual('color3');
});
it('returns the overwriteColor if exists', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [
['ES-Air', undefined],
['Kibana Airlines', undefined],
],
depth: 0,
sortIndex: 0,
},
children: [],
};
const color = getColor(
ChartTypes.PIE,
'ES-Air',
d,
0,
true,
{ 'ES-Air': '#000028' },
distinctSeries,
dataLength,
visParams,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(color).toEqual('#000028');
});
it('returns the overwriteColor for older visualizations with formatted values', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [
[
const getPaletteRegistry = () => {
const mockPalette1: jest.Mocked<PaletteDefinition> = {
id: 'default',
title: 'My Palette',
getCategoricalColor: jest.fn((layer: SeriesLayer[]) => colors[layer[0].rankAtDepth]),
getCategoricalColors: jest.fn((num: number) => colors),
toExpression: jest.fn(() => ({
type: 'expression',
chain: [
{
gte: 1000,
lt: 2000,
}.toString(),
undefined,
type: 'function',
function: 'system_palette',
arguments: {
name: ['default'],
},
},
],
[
{
gte: 2000,
lt: 3000,
}.toString(),
undefined,
],
],
depth: 0,
sortIndex: 0,
},
children: [],
};
const visParamsNew = {
...visParams,
distinctColors: true,
};
const column = {
...visData.columns[0],
format: {
id: 'range',
params: {
id: 'number',
},
},
};
const color = getColor(
ChartTypes.PIE,
// There is the unhandled situation that the categoricalName passed is not a plain string but a RangeKey
// In this case, the internal code, thankfully, requires the stringified version of it and/or the formatted one
// handling also this badly configured type
// FIXME when getColor could handle both strings and RangeKey
{ gte: 1000, lt: 2000 } as unknown as string,
d,
0,
true,
{ '≥ 1000 and < 2000': '#3F6833' },
distinctSeries,
dataLength,
visParamsNew,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
column,
formatters
);
expect(color).toEqual('#3F6833');
});
})),
};
it('should only pass the second layer for mosaic', () => {
const d: SimplifiedArrayNode = {
depth: 2,
sortIndex: 0,
parent: {
children: [
['Second level 1', undefined],
['Second level 2', undefined],
],
return {
get: () => mockPalette1,
getAll: () => [mockPalette1],
};
};
it('should return the correct color based on the parent sortIndex', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [['First level', undefined]],
children: [
['ES-Air', undefined],
['Kibana Airlines', undefined],
],
depth: 0,
sortIndex: 0,
},
},
children: [],
};
const registry = getPaletteRegistry();
getColor(
ChartTypes.MOSAIC,
'Second level 1',
d,
1,
true,
{},
distinctSeries,
dataLength,
visParams,
registry,
undefined,
true,
false,
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(registry.get().getCategoricalColor).toHaveBeenCalledWith(
[expect.objectContaining({ name: 'Second level 1' })],
expect.anything(),
expect.anything()
);
});
children: [],
};
it('should only pass the first layer for treemap', () => {
const d: SimplifiedArrayNode = {
depth: 2,
sortIndex: 0,
parent: {
children: [
['Second level 1', undefined],
['Second level 2', undefined],
],
const color = getColor(
ChartTypes.PIE,
'ES-Air',
d,
0,
false,
{},
distinctSeries,
dataLength,
visParams,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
visData.columns[0],
colorIndexMap(ChartTypes.PIE)
);
expect(color).toEqual(colors[0]);
});
it('slices with the same label should have the same color for small multiples', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [['First level', undefined]],
children: [
['ES-Air', undefined],
['Kibana Airlines', undefined],
],
depth: 0,
sortIndex: 0,
},
},
children: [],
};
const registry = getPaletteRegistry();
getColor(
ChartTypes.TREEMAP,
'Second level 1',
d,
1,
true,
{},
distinctSeries,
dataLength,
visParams,
registry,
undefined,
true,
false,
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(registry.get().getCategoricalColor).toHaveBeenCalledWith(
[expect.objectContaining({ name: 'First level' })],
expect.anything(),
expect.anything()
);
children: [],
};
const color = getColor(
ChartTypes.PIE,
'ES-Air',
d,
0,
true,
{},
distinctSeries,
dataLength,
visParams,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
visData.columns[0],
colorIndexMap(ChartTypes.PIE)
);
expect(color).toEqual('color3');
});
it('returns the overwriteColor if exists', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [
['ES-Air', undefined],
['Kibana Airlines', undefined],
],
depth: 0,
sortIndex: 0,
},
children: [],
};
const color = getColor(
ChartTypes.PIE,
'ES-Air',
d,
0,
true,
{ 'ES-Air': '#000028' },
distinctSeries,
dataLength,
visParams,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
visData.columns[0],
colorIndexMap(ChartTypes.PIE)
);
expect(color).toEqual('#000028');
});
it('returns the overwriteColor for older visualizations with formatted values', () => {
const d: SimplifiedArrayNode = {
depth: 1,
sortIndex: 0,
parent: {
children: [
[
{
gte: 1000,
lt: 2000,
}.toString(),
undefined,
],
[
{
gte: 2000,
lt: 3000,
}.toString(),
undefined,
],
],
depth: 0,
sortIndex: 0,
},
children: [],
};
const visParamsNew = {
...visParams,
distinctColors: true,
};
const column = {
...visData.columns[0],
format: {
id: 'range',
params: {
id: 'number',
},
},
};
const color = getColor(
ChartTypes.PIE,
// There is the unhandled situation that the categoricalName passed is not a plain string but a RangeKey
// In this case, the internal code, thankfully, requires the stringified version of it and/or the formatted one
// handling also this badly configured type
// FIXME when getColor could handle both strings and RangeKey
{ gte: 1000, lt: 2000 } as unknown as string,
d,
0,
true,
{ '≥ 1000 and < 2000': '#3F6833' },
distinctSeries,
dataLength,
visParamsNew,
getPaletteRegistry(),
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats,
column,
colorIndexMap(ChartTypes.PIE)
);
expect(color).toEqual('#3F6833');
});
it('should only pass the second layer for mosaic', () => {
const d: SimplifiedArrayNode = {
depth: 2,
sortIndex: 0,
parent: {
children: [
['Second level 1', undefined],
['Second level 2', undefined],
],
depth: 1,
sortIndex: 0,
parent: {
children: [['First level', undefined]],
depth: 0,
sortIndex: 0,
},
},
children: [],
};
const registry = getPaletteRegistry();
getColor(
ChartTypes.MOSAIC,
'Second level 1',
d,
1,
true,
{},
distinctSeries,
dataLength,
visParams,
registry,
undefined,
true,
false,
dataMock.fieldFormats,
visData.columns[0],
colorIndexMap(ChartTypes.MOSAIC)
);
expect(registry.get().getCategoricalColor).toHaveBeenCalledWith(
[expect.objectContaining({ name: 'Second level 1' })],
expect.anything(),
expect.anything()
);
});
});
});

View file

@ -11,60 +11,42 @@ import { ArrayNode } from '@elastic/charts';
import { isEqual } from 'lodash';
import type { PaletteRegistry, SeriesLayer, PaletteOutput, PaletteDefinition } from '@kbn/coloring';
import type { FieldFormatsStart } from '@kbn/field-formats-plugin/public';
import type { FieldFormat } from '@kbn/field-formats-plugin/common';
import { lightenColor } from '@kbn/charts-plugin/public';
import type { Datatable } from '@kbn/expressions-plugin/public';
import { BucketColumns, ChartTypes, PartitionVisParams } from '../../../common/types';
import { DistinctSeries } from '../get_distinct_series';
import { getNodeLabel } from './get_node_labels';
const isTreemapOrMosaicChart = (shape: ChartTypes) =>
[ChartTypes.MOSAIC, ChartTypes.TREEMAP].includes(shape);
export const byDataColorPaletteMap = (
rows: Datatable['rows'],
column: Partial<BucketColumns>,
paletteDefinition: PaletteDefinition,
{ params }: PaletteOutput,
formatters: Record<string, FieldFormat | undefined>,
formatter: FieldFormatsStart
colorIndexMap: Map<string, number>
) => {
const colorMap = new Map<string, string | undefined>(
rows.map((item) => {
const formattedName = getNodeLabel(
item[column.id ?? ''],
column,
formatters,
formatter.deserialize
);
return [formattedName, undefined];
})
);
let rankAtDepth = 0;
const colorCache = new Map<string, string | undefined>();
return {
getColor: (item: unknown) => {
getColor: (item: string) => {
const key = String(item);
if (!colorMap.has(key)) return;
let color = colorCache.get(key);
let color = colorMap.get(key);
if (color) {
return color;
}
if (color) return color;
const colorIndex = colorIndexMap.get(key) ?? -1;
color =
paletteDefinition.getCategoricalColor(
[
{
name: key,
totalSeriesAtDepth: colorMap.size,
rankAtDepth: rankAtDepth++,
totalSeriesAtDepth: colorIndexMap.size,
rankAtDepth: colorIndex,
},
],
{ behindText: false },
params
) || undefined;
colorMap.set(key, color);
colorCache.set(key, color);
return color;
},
};
@ -136,21 +118,20 @@ export interface SimplifiedArrayNode {
* (a node of a hierarchical tree, currently a partition tree) up to the root of the hierarchy tree.
* The resulting array only shows, for each parent, the name of the node, its child index within the parent branch
* (called rankInDepth) and the total number of children of the parent.
*
*/
const createSeriesLayers = (
arrayNode: SimplifiedArrayNode,
parentSeries: DistinctSeries['parentSeries'],
isSplitChart: boolean,
formatters: Record<string, FieldFormat | undefined>,
formatter: FieldFormatsStart,
column: Partial<BucketColumns>
colorIndexMap: Map<string, number>
): SeriesLayer[] => {
const seriesLayers: SeriesLayer[] = [];
let tempParent: typeof arrayNode | (typeof arrayNode)['parent'] = arrayNode;
while (tempParent.parent && tempParent.depth > 0) {
const nodeKey = tempParent.parent.children[tempParent.sortIndex][0];
const seriesName = String(nodeKey);
/**
* FIXME this is a bad implementation: The `parentSeries` is an array of both `string` and `RangeKey` even if its type
* is marked as `string[]` in `DistinctSeries`. Here instead we are checking if a stringified `RangeKey` is included into this array that
@ -158,15 +139,14 @@ const createSeriesLayers = (
* see https://github.com/elastic/kibana/issues/153437
*/
const isSplitParentLayer = isSplitChart && parentSeries.includes(seriesName);
const formattedName = getNodeLabel(nodeKey, column, formatters, formatter.deserialize);
const colorIndex = colorIndexMap.get(seriesName) ?? tempParent.sortIndex;
seriesLayers.unshift({
// by construction and types `formattedName` should be always be a string, but I leave this Nullish Coalescing
// because I don't trust much our formatting functions
name: formattedName ?? seriesName,
name: seriesName,
rankAtDepth: isSplitParentLayer
? // FIXME as described above this will not work correctly if the `nodeKey` is a `RangeKey`
parentSeries.findIndex((name) => name === seriesName)
: tempParent.sortIndex,
: colorIndex,
totalSeriesAtDepth: isSplitParentLayer
? parentSeries.length
: tempParent.parent.children.length,
@ -213,7 +193,7 @@ export const getColor = (
isDarkMode: boolean,
formatter: FieldFormatsStart,
column: Partial<BucketColumns>,
formatters: Record<string, FieldFormat | undefined>
colorIndexMap: Map<string, number>
) => {
// Mind the difference here: the contrast computation for the text ignores the alpha/opacity
// therefore change it for dark mode
@ -242,9 +222,7 @@ export const getColor = (
arrayNode,
distinctSeries.parentSeries,
isSplitChart,
formatters,
formatter,
column
colorIndexMap
);
const overriddenColor = overrideColors(seriesLayers, overwriteColors, name);
@ -257,18 +235,13 @@ export const getColor = (
return byDataPalette.getColor(seriesLayers[1].name) || defaultColor;
}
if (isTreemapOrMosaicChart(chartType)) {
if (layerIndex < columnsLength - 1) {
return defaultColor;
}
// for treemap use the top layer for coloring, for mosaic use the second layer
if (seriesLayers.length > 1) {
if (chartType === ChartTypes.MOSAIC) {
seriesLayers.shift();
} else {
seriesLayers.pop();
}
}
if (chartType === ChartTypes.MOSAIC && layerIndex < columnsLength - 1) {
return defaultColor;
}
// Mosaic - use the second layer for color
if (chartType === ChartTypes.MOSAIC && seriesLayers.length > 1) {
seriesLayers.shift();
}
const outputColor = paletteService?.get(visParams.palette.name).getCategoricalColor(

View file

@ -53,16 +53,20 @@ export const getLayers = (
fillLabel.valueFormatter = () => '';
}
const categories =
chartType === ChartTypes.MOSAIC && columns.length === 2
? getColorCategories(rows, columns[1]?.id)
: getColorCategories(rows, columns[0]?.id);
const colorIndexMap = new Map(categories.map((c, i) => [String(c), i]));
const isSplitChart = Boolean(visParams.dimensions.splitColumn || visParams.dimensions.splitRow);
let byDataPalette: ReturnType<typeof byDataColorPaletteMap>;
if (!syncColors && columns[1]?.id && paletteService && visParams.palette) {
byDataPalette = byDataColorPaletteMap(
rows,
columns[1],
paletteService?.get(visParams.palette.name),
visParams.palette,
formatters,
formatter
colorIndexMap
);
}
@ -113,7 +117,7 @@ export const getLayers = (
isDarkMode,
formatter,
col,
formatters
colorIndexMap
),
},
};