Skip to content

Commit

Permalink
[Lens][Partition] Fix behind text coloring for syncColors (elastic#…
Browse files Browse the repository at this point in the history
…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 elastic#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>
  • Loading branch information
nickofthyme and kibanamachine authored Feb 11, 2025
1 parent 37465f4 commit 155577b
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
}
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 155577b

Please sign in to comment.