From 155577b74bb9da71a1b77c568092b879a09b754a Mon Sep 17 00:00:00 2001 From: Nick Partridge Date: Mon, 10 Feb 2025 20:41:09 -0700 Subject: [PATCH] [Lens][Partition] Fix behind text coloring for `syncColors` (#209632) ## Summary When switching to borealis palettes there is no longer a need for `euiPaletteColorBlindBehindText` palette. Removing this created the linked issue as there was no default applied when accessing the mapped colors. This change attempts to lookup behind text color otherwise defaults to normal color. This was only reachable when the dashboard `syncColors` option was enabled. Fixes #209610 ### 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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ## Release note This fixes and issues where behind text colors were not correctly assigned, such as in `Pie`, `Treemap` and `Mosaic` charts. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../services/palettes/palettes.test.tsx | 103 ++++++++++-------- .../public/services/palettes/palettes.tsx | 4 +- 2 files changed, 62 insertions(+), 45 deletions(-) diff --git a/src/platform/plugins/shared/charts/public/services/palettes/palettes.test.tsx b/src/platform/plugins/shared/charts/public/services/palettes/palettes.test.tsx index a962da6776c4e..3d5ae1570fa8e 100644 --- a/src/platform/plugins/shared/charts/public/services/palettes/palettes.test.tsx +++ b/src/platform/plugins/shared/charts/public/services/palettes/palettes.test.tsx @@ -10,9 +10,10 @@ import { buildPalettes } from './palettes'; import { euiPaletteColorBlind, euiPaletteColorBlindBehindText } from '@elastic/eui'; -describe('palettes', () => { - const palettes = buildPalettes({ name: 'amsterdam', darkMode: false }); - +describe.each([ + ['palettes', false, buildPalettes({ name: 'borealis', darkMode: false })], + ['legacyPalettes', true, buildPalettes({ name: 'amsterdam', darkMode: false })], +])('%s', (_, legacy, palettes) => { describe('default palette', () => { describe('syncColors: false', () => { it('should return different colors based on behind text flag', () => { @@ -37,7 +38,12 @@ describe('palettes', () => { behindText: true, } ); - expect(color1).not.toEqual(color2); + if (legacy) { + expect(color1).not.toEqual(color2); + } else { + // no behind text coloring in new palettes + expect(color1).toEqual(color2); + } }); it('should return different colors based on rank at current series', () => { @@ -120,7 +126,13 @@ describe('palettes', () => { syncColors: true, } ); - expect(color1).not.toEqual(color2); + + if (legacy) { + expect(color1).not.toEqual(color2); + } else { + // no behind text coloring in new palettes + expect(color1).toEqual(color2); + } }); it('should return different colors for different keys', () => { @@ -223,48 +235,51 @@ describe('palettes', () => { expect(color1).toEqual(color2); }); - it('should return the same index of the behind text palette for same key', () => { - const palette = palettes.default; + if (legacy) { + it('should return the same index of the behind text palette for same key', () => { + const palette = palettes.default; - const color1 = palette.getCategoricalColor( - [ - { - name: 'klm', - rankAtDepth: 0, - totalSeriesAtDepth: 5, - }, - { - name: 'def', - rankAtDepth: 0, - totalSeriesAtDepth: 2, - }, - ], - { - syncColors: true, - } - ); - const color2 = palette.getCategoricalColor( - [ + const color1 = palette.getCategoricalColor( + [ + { + name: 'klm', + rankAtDepth: 0, + totalSeriesAtDepth: 5, + }, + { + name: 'def', + rankAtDepth: 0, + totalSeriesAtDepth: 2, + }, + ], { - name: 'klm', - rankAtDepth: 3, - totalSeriesAtDepth: 5, - }, + syncColors: true, + } + ); + const color2 = palette.getCategoricalColor( + [ + { + name: 'klm', + rankAtDepth: 3, + totalSeriesAtDepth: 5, + }, + { + name: 'ghj', + rankAtDepth: 1, + totalSeriesAtDepth: 1, + }, + ], { - name: 'ghj', - rankAtDepth: 1, - totalSeriesAtDepth: 1, - }, - ], - { - syncColors: true, - behindText: true, - } - ); - const color1Index = euiPaletteColorBlind({ rotations: 2 }).indexOf(color1!); - const color2Index = euiPaletteColorBlindBehindText({ rotations: 2 }).indexOf(color2!); - expect(color1Index).toEqual(color2Index); - }); + syncColors: true, + behindText: true, + } + ); + const color1Index = euiPaletteColorBlind({ rotations: 2 }).indexOf(color1!); + const color2Index = euiPaletteColorBlindBehindText({ rotations: 2 }).indexOf(color2!); + + expect(color1Index).toEqual(color2Index); + }); + } }); }); diff --git a/src/platform/plugins/shared/charts/public/services/palettes/palettes.tsx b/src/platform/plugins/shared/charts/public/services/palettes/palettes.tsx index 901e3a7fae282..213342dad2909 100644 --- a/src/platform/plugins/shared/charts/public/services/palettes/palettes.tsx +++ b/src/platform/plugins/shared/charts/public/services/palettes/palettes.tsx @@ -38,7 +38,9 @@ function buildRoundRobinCategoricalWithMappedColors( const colorKey = series[0].name; mappedColors.mapKeys([colorKey]); const mappedColor = mappedColors.get(colorKey); - outputColor = chartConfiguration.behindText ? behindTextColorMap[mappedColor] : mappedColor; + outputColor = chartConfiguration.behindText + ? behindTextColorMap[mappedColor] ?? mappedColor + : mappedColor; } else { outputColor = chartConfiguration.behindText && behindTextColors