Skip to content

Commit

Permalink
[Lens] Fixes the syncing of other series color (#146785)
Browse files Browse the repository at this point in the history
## Summary

Closes #146655


It seems that there are cases that the series name is "__other__" and
other cases that is "Other". This PR fixes the color syncing of Other
series on a dashboard.

<img width="1155" alt="image"
src="https://user-images.githubusercontent.com/17003240/205063130-8d4aee0e-42a3-4502-a4ab-b2b3c0a38f59.png">


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

Co-authored-by: Andrew Tate <[email protected]>
  • Loading branch information
stratoula and drewdaemon authored Dec 5, 2022
1 parent aa34492 commit 156bfe4
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import { ShapeTreeNode } 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, DatatableRow } from '@kbn/expressions-plugin/public';
import { BucketColumns, ChartTypes, PartitionVisParams } from '../../../common/types';
import { DistinctSeries, getDistinctSeries } from '../get_distinct_series';
import { getNodeLabel } from './get_node_labels';

const isTreemapOrMosaicChart = (shape: ChartTypes) =>
[ChartTypes.MOSAIC, ChartTypes.TREEMAP].includes(shape);
Expand Down Expand Up @@ -109,15 +111,24 @@ const getDistinctColor = (
const createSeriesLayers = (
d: ShapeTreeNode,
parentSeries: DistinctSeries['parentSeries'],
isSplitChart: boolean
isSplitChart: boolean,
formatters: Record<string, FieldFormat | undefined>,
formatter: FieldFormatsStart,
column: Partial<BucketColumns>
) => {
const seriesLayers: SeriesLayer[] = [];
let tempParent: typeof d | typeof d['parent'] = d;
while (tempParent.parent && tempParent.depth > 0) {
const seriesName = String(tempParent.parent.children[tempParent.sortIndex][0]);
const isSplitParentLayer = isSplitChart && parentSeries.includes(seriesName);
const formattedName = getNodeLabel(
tempParent.parent.children[tempParent.sortIndex][0],
column,
formatters,
formatter.deserialize
);
seriesLayers.unshift({
name: seriesName,
name: formattedName ?? seriesName,
rankAtDepth: isSplitParentLayer
? parentSeries.findIndex((name) => name === seriesName)
: tempParent.sortIndex,
Expand Down Expand Up @@ -164,7 +175,8 @@ export const getColor = (
syncColors: boolean,
isDarkMode: boolean,
formatter: FieldFormatsStart,
format?: BucketColumns['format']
column: Partial<BucketColumns>,
formatters: Record<string, FieldFormat | undefined>
) => {
const distinctSeries = getDistinctSeries(rows, columns);
const { parentSeries } = distinctSeries;
Expand All @@ -175,8 +187,8 @@ export const getColor = (
const defaultColor = isDarkMode ? 'rgba(0,0,0,0)' : 'rgba(255,255,255,0)';

let name = '';
if (format) {
name = formatter.deserialize(format).convert(dataName) ?? '';
if (column.format) {
name = formatter.deserialize(column.format).convert(dataName) ?? '';
}

if (visParams.distinctColors) {
Expand All @@ -194,7 +206,14 @@ export const getColor = (
);
}

const seriesLayers = createSeriesLayers(d, parentSeries, isSplitChart);
const seriesLayers = createSeriesLayers(
d,
parentSeries,
isSplitChart,
formatters,
formatter,
column
);

const overriddenColor = overrideColors(seriesLayers, overwriteColors, name);
if (overriddenColor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
import { ShapeTreeNode } from '@elastic/charts';
import type { PaletteDefinition, 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';

const visData = createMockVisData();
Expand All @@ -22,6 +24,8 @@ interface RangeProps {
gte: number;
lt: number;
}
const defaultFormatter = jest.fn((...args) => fieldFormatsMock.deserialize(...args));
const formatters = generateFormatters(visData, defaultFormatter);

dataMock.fieldFormats = {
deserialize: jest.fn(() => ({
Expand Down Expand Up @@ -82,7 +86,9 @@ describe('computeColor', () => {
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(color).toEqual(colors[0]);
});
Expand Down Expand Up @@ -111,7 +117,9 @@ describe('computeColor', () => {
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(color).toEqual('color3');
});
Expand Down Expand Up @@ -139,7 +147,9 @@ describe('computeColor', () => {
{ getColor: () => undefined },
false,
false,
dataMock.fieldFormats
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(color).toEqual('#000028');
});
Expand Down Expand Up @@ -175,6 +185,15 @@ describe('computeColor', () => {
...visParams,
distinctColors: true,
};
const column = {
...visData.columns[0],
format: {
id: 'range',
params: {
id: 'number',
},
},
};
const color = getColor(
ChartTypes.PIE,
d,
Expand All @@ -189,12 +208,8 @@ describe('computeColor', () => {
false,
false,
dataMock.fieldFormats,
{
id: 'range',
params: {
id: 'number',
},
}
column,
formatters
);
expect(color).toEqual('#3F6833');
});
Expand Down Expand Up @@ -229,7 +244,9 @@ describe('computeColor', () => {
undefined,
true,
false,
dataMock.fieldFormats
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(registry.get().getCategoricalColor).toHaveBeenCalledWith(
[expect.objectContaining({ name: 'Second level 1' })],
Expand Down Expand Up @@ -268,7 +285,9 @@ describe('computeColor', () => {
undefined,
true,
false,
dataMock.fieldFormats
dataMock.fieldFormats,
visData.columns[0],
formatters
);
expect(registry.get().getCategoricalColor).toHaveBeenCalledWith(
[expect.objectContaining({ name: 'First level' })],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ export const getLayers = (
syncColors,
isDarkMode,
formatter,
col.format
col,
formatters
),
},
};
Expand Down

0 comments on commit 156bfe4

Please sign in to comment.