Skip to content

Commit

Permalink
[VisTypePie] Fixes color migrations of pie charts (#104373)
Browse files Browse the repository at this point in the history
* [Pie] fix color migrations of pie charts

* Fix color picker

* Fix functional test

* revert

* Apply PR comments

Co-authored-by: Kibana Machine <[email protected]>
stratoula and kibanamachine authored Jul 7, 2021
1 parent 24afb1a commit 186b936
Showing 3 changed files with 110 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/plugins/vis_type_pie/public/utils/get_color_picker.tsx
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ import React, { useCallback } from 'react';
import Color from 'color';
import { LegendColorPicker, Position } from '@elastic/charts';
import { PopoverAnchorPosition, EuiPopover, EuiOutsideClickDetector } from '@elastic/eui';
import { DatatableRow } from '../../../expressions/public';
import type { DatatableRow } from '../../../expressions/public';
import type { PersistedState } from '../../../visualizations/public';
import { ColorPicker } from '../../../charts/public';
import { BucketColumns } from '../types';
76 changes: 73 additions & 3 deletions src/plugins/vis_type_pie/public/utils/get_layers.test.ts
Original file line number Diff line number Diff line change
@@ -7,13 +7,29 @@
*/
import { ShapeTreeNode } from '@elastic/charts';
import { PaletteDefinition, SeriesLayer } from '../../../charts/public';
import { dataPluginMock } from '../../../data/public/mocks';
import type { DataPublicPluginStart } from '../../../data/public';
import { computeColor } from './get_layers';
import { createMockVisData, createMockBucketColumns, createMockPieParams } from '../mocks';

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;
}

dataMock.fieldFormats = ({
deserialize: jest.fn(() => ({
convert: jest.fn((s: RangeProps) => {
return `≥ ${s.gte} and < ${s.lt}`;
}),
})),
} as unknown) as DataPublicPluginStart['fieldFormats'];

export const getPaletteRegistry = () => {
const mockPalette1: jest.Mocked<PaletteDefinition> = {
id: 'default',
@@ -60,7 +76,8 @@ describe('computeColor', () => {
visData.rows,
visParams,
getPaletteRegistry(),
false
false,
dataMock.fieldFormats
);
expect(color).toEqual(colors[0]);
});
@@ -84,7 +101,8 @@ describe('computeColor', () => {
visData.rows,
visParams,
getPaletteRegistry(),
false
false,
dataMock.fieldFormats
);
expect(color).toEqual('color3');
});
@@ -107,8 +125,60 @@ describe('computeColor', () => {
visData.rows,
visParams,
getPaletteRegistry(),
false
false,
dataMock.fieldFormats
);
expect(color).toEqual('#000028');
});

it('returns the overwriteColor for older visualizations with formatted values', () => {
const d = ({
dataName: {
gte: 1000,
lt: 2000,
},
depth: 1,
sortIndex: 0,
parent: {
children: [
[
{
gte: 1000,
lt: 2000,
},
],
[
{
gte: 2000,
lt: 3000,
},
],
],
depth: 0,
sortIndex: 0,
},
} as unknown) as ShapeTreeNode;
const visParamsNew = {
...visParams,
distinctColors: true,
};
const color = computeColor(
d,
true,
{ '≥ 1000 and < 2000': '#3F6833' },
buckets,
visData.rows,
visParamsNew,
getPaletteRegistry(),
false,
dataMock.fieldFormats,
{
id: 'range',
params: {
id: 'number',
},
}
);
expect(color).toEqual('#3F6833');
});
});
44 changes: 36 additions & 8 deletions src/plugins/vis_type_pie/public/utils/get_layers.ts
Original file line number Diff line number Diff line change
@@ -15,9 +15,9 @@ import {
} from '@elastic/charts';
import { isEqual } from 'lodash';
import { SeriesLayer, PaletteRegistry, lightenColor } from '../../../charts/public';
import { DataPublicPluginStart } from '../../../data/public';
import { DatatableRow } from '../../../expressions/public';
import { BucketColumns, PieVisParams, SplitDimensionParams } from '../types';
import type { DataPublicPluginStart } from '../../../data/public';
import type { DatatableRow } from '../../../expressions/public';
import type { BucketColumns, PieVisParams, SplitDimensionParams } from '../types';
import { getDistinctSeries } from './get_distinct_series';

const EMPTY_SLICE = Symbol('empty_slice');
@@ -30,14 +30,33 @@ export const computeColor = (
rows: DatatableRow[],
visParams: PieVisParams,
palettes: PaletteRegistry | null,
syncColors: boolean
syncColors: boolean,
formatter: DataPublicPluginStart['fieldFormats'],
format?: BucketColumns['format']
) => {
const { parentSeries, allSeries } = getDistinctSeries(rows, columns);
const dataName = d.dataName;

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

if (visParams.distinctColors) {
const dataName = d.dataName;
let overwriteColor;
// this is for supporting old visualizations (created by vislib plugin)
// it seems that there for some aggs, the uiState saved from vislib is
// different than the es-charts handle it
if (overwriteColors.hasOwnProperty(formattedLabel)) {
overwriteColor = overwriteColors[formattedLabel];
}

if (Object.keys(overwriteColors).includes(dataName.toString())) {
return overwriteColors[dataName];
overwriteColor = overwriteColors[dataName];
}

if (overwriteColor) {
return overwriteColor;
}

const index = allSeries.findIndex((name) => isEqual(name, dataName));
@@ -80,6 +99,13 @@ export const computeColor = (
}

let overwriteColor;
// this is for supporting old visualizations (created by vislib plugin)
// it seems that there for some aggs, the uiState saved from vislib is
// different than the es-charts handle it
if (overwriteColors.hasOwnProperty(formattedLabel)) {
overwriteColor = overwriteColors[formattedLabel];
}

seriesLayers.forEach((layer) => {
if (Object.keys(overwriteColors).includes(layer.name)) {
overwriteColor = overwriteColors[layer.name];
@@ -141,7 +167,7 @@ export const getLayers = (
if (name1 === '__other__' && name2 !== '__other__') return 1;
if (name2 === '__other__' && name1 !== '__other__') return -1;
// metric sorting
if (sort !== '_key') {
if (sort && sort !== '_key') {
if (params?.order === 'desc') {
return node2.value - node1.value;
} else {
@@ -169,7 +195,9 @@ export const getLayers = (
rows,
visParams,
palettes,
syncColors
syncColors,
formatter,
col.format
);

return outputColor || 'rgba(0,0,0,0)';

0 comments on commit 186b936

Please sign in to comment.