From 186b936fbbda5c7b1d1e34a13697258512e93506 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 7 Jul 2021 16:19:56 +0300 Subject: [PATCH] [VisTypePie] Fixes color migrations of pie charts (#104373) * [Pie] fix color migrations of pie charts * Fix color picker * Fix functional test * revert * Apply PR comments Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../public/utils/get_color_picker.tsx | 2 +- .../public/utils/get_layers.test.ts | 76 ++++++++++++++++++- .../vis_type_pie/public/utils/get_layers.ts | 44 +++++++++-- 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/plugins/vis_type_pie/public/utils/get_color_picker.tsx b/src/plugins/vis_type_pie/public/utils/get_color_picker.tsx index 436ce81d3ce3..628c2d74dc43 100644 --- a/src/plugins/vis_type_pie/public/utils/get_color_picker.tsx +++ b/src/plugins/vis_type_pie/public/utils/get_color_picker.tsx @@ -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'; diff --git a/src/plugins/vis_type_pie/public/utils/get_layers.test.ts b/src/plugins/vis_type_pie/public/utils/get_layers.test.ts index e0658eaa295f..d6f80b3eb231 100644 --- a/src/plugins/vis_type_pie/public/utils/get_layers.test.ts +++ b/src/plugins/vis_type_pie/public/utils/get_layers.test.ts @@ -7,6 +7,8 @@ */ 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'; @@ -14,6 +16,20 @@ 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 = { 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'); + }); }); diff --git a/src/plugins/vis_type_pie/public/utils/get_layers.ts b/src/plugins/vis_type_pie/public/utils/get_layers.ts index 5a82871bf368..b995df83c0bb 100644 --- a/src/plugins/vis_type_pie/public/utils/get_layers.ts +++ b/src/plugins/vis_type_pie/public/utils/get_layers.ts @@ -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)';