Skip to content

Commit

Permalink
Make sure color mapping setting is respected for legacy palette (#95164)
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 authored Mar 24, 2021
1 parent 52d0fc0 commit 585f6f2
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 17 deletions.
2 changes: 1 addition & 1 deletion docs/management/advanced-options.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ Enables the legacy charts library for aggregation-based area, line, and bar char

[[visualization-colormapping]]`visualization:colorMapping`::
**This setting is deprecated and will not be supported as of 8.0.**
Maps values to specific colors in *Visualize* charts and *TSVB*. This setting does not apply to *Lens*.
Maps values to specific colors in charts using the *Compatibility* palette.

[[visualization-dimmingopacity]]`visualization:dimmingOpacity`::
The opacity of the chart items that are dimmed when highlighting another element
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/charts/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ export type Start = jest.Mocked<ReturnType<ChartsPlugin['start']>>;
const createSetupContract = (): Setup => ({
legacyColors: colorsServiceMock,
theme: themeServiceMock,
palettes: paletteServiceMock.setup({} as any, {} as any),
palettes: paletteServiceMock.setup({} as any),
});

const createStartContract = (): Start => ({
legacyColors: colorsServiceMock,
theme: themeServiceMock,
palettes: paletteServiceMock.setup({} as any, {} as any),
palettes: paletteServiceMock.setup({} as any),
});

export { colorMapsMock } from './static/color_maps/mock';
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/charts/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class ChartsPlugin implements Plugin<ChartsPluginSetup, ChartsPluginStart
dependencies.expressions.registerFunction(systemPalette);
this.themeService.init(core.uiSettings);
this.legacyColorsService.init(core.uiSettings);
this.palettes = this.paletteService.setup(core, this.legacyColorsService);
this.palettes = this.paletteService.setup(this.legacyColorsService);

return {
legacyColors: this.legacyColorsService,
Expand Down
1 change: 1 addition & 0 deletions src/plugins/charts/public/services/legacy_colors/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ export const colorsServiceMock: LegacyColorsService = {
mappedColors: {
mapKeys: jest.fn(),
get: jest.fn(),
getColorFromConfig: jest.fn(),
},
} as any;
55 changes: 50 additions & 5 deletions src/plugins/charts/public/services/palettes/palettes.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@
* Side Public License, v 1.
*/

import { coreMock } from '../../../../../core/public/mocks';
import { createColorPalette as createLegacyColorPalette } from '../../../../../../src/plugins/charts/public';
import { PaletteDefinition } from './types';
import { buildPalettes } from './palettes';
import { colorsServiceMock } from '../legacy_colors/mock';
import { euiPaletteColorBlind, euiPaletteColorBlindBehindText } from '@elastic/eui';

describe('palettes', () => {
const palettes: Record<string, PaletteDefinition> = buildPalettes(
coreMock.createStart().uiSettings,
colorsServiceMock
);
const palettes: Record<string, PaletteDefinition> = buildPalettes(colorsServiceMock);
describe('default palette', () => {
describe('syncColors: false', () => {
it('should return different colors based on behind text flag', () => {
Expand Down Expand Up @@ -302,6 +298,7 @@ describe('palettes', () => {

beforeEach(() => {
(colorsServiceMock.mappedColors.mapKeys as jest.Mock).mockClear();
(colorsServiceMock.mappedColors.getColorFromConfig as jest.Mock).mockReset();
(colorsServiceMock.mappedColors.get as jest.Mock).mockClear();
});

Expand All @@ -323,6 +320,30 @@ describe('palettes', () => {
expect(colorsServiceMock.mappedColors.get).not.toHaveBeenCalled();
});

it('should respect the advanced settings color mapping', () => {
const configColorGetter = colorsServiceMock.mappedColors.getColorFromConfig as jest.Mock;
configColorGetter.mockImplementation(() => 'blue');
const result = palette.getColor(
[
{
name: 'abc',
rankAtDepth: 2,
totalSeriesAtDepth: 10,
},
{
name: 'def',
rankAtDepth: 0,
totalSeriesAtDepth: 10,
},
],
{
syncColors: false,
}
);
expect(result).toEqual('blue');
expect(configColorGetter).toHaveBeenCalledWith('abc');
});

it('should return a color from the legacy palette based on position of first series', () => {
const result = palette.getColor(
[
Expand Down Expand Up @@ -363,6 +384,30 @@ describe('palettes', () => {
expect(colorsServiceMock.mappedColors.get).toHaveBeenCalledWith('abc');
});

it('should respect the advanced settings color mapping', () => {
const configColorGetter = colorsServiceMock.mappedColors.getColorFromConfig as jest.Mock;
configColorGetter.mockImplementation(() => 'blue');
const result = palette.getColor(
[
{
name: 'abc',
rankAtDepth: 2,
totalSeriesAtDepth: 10,
},
{
name: 'def',
rankAtDepth: 0,
totalSeriesAtDepth: 10,
},
],
{
syncColors: false,
}
);
expect(result).toEqual('blue');
expect(configColorGetter).toHaveBeenCalledWith('abc');
});

it('should always use root series', () => {
palette.getColor(
[
Expand Down
7 changes: 3 additions & 4 deletions src/plugins/charts/public/services/palettes/palettes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// @ts-ignore
import chroma from 'chroma-js';
import { i18n } from '@kbn/i18n';
import { IUiSettingsClient } from 'src/core/public';
import {
euiPaletteColorBlind,
euiPaletteCool,
Expand Down Expand Up @@ -130,7 +129,8 @@ function buildSyncedKibanaPalette(
colors.mappedColors.mapKeys([series[0].name]);
outputColor = colors.mappedColors.get(series[0].name);
} else {
outputColor = staticColors[series[0].rankAtDepth % staticColors.length];
const configColor = colors.mappedColors.getColorFromConfig(series[0].name);
outputColor = configColor || staticColors[series[0].rankAtDepth % staticColors.length];
}

if (!chartConfiguration.maxDepth || chartConfiguration.maxDepth === 1) {
Expand Down Expand Up @@ -199,9 +199,8 @@ function buildCustomPalette(): PaletteDefinition {
}

export const buildPalettes: (
uiSettings: IUiSettingsClient,
legacyColorsService: LegacyColorsService
) => Record<string, PaletteDefinition> = (uiSettings, legacyColorsService) => {
) => Record<string, PaletteDefinition> = (legacyColorsService) => {
return {
default: {
title: i18n.translate('charts.palettes.defaultPaletteLabel', {
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/charts/public/services/palettes/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import { CoreSetup } from 'kibana/public';
import { ExpressionsSetup } from '../../../../../../src/plugins/expressions/public';
import {
ChartsPluginSetup,
Expand All @@ -24,12 +23,12 @@ export class PaletteService {
private palettes: Record<string, PaletteDefinition<unknown>> | undefined = undefined;
constructor() {}

public setup(core: CoreSetup, colorsService: LegacyColorsService) {
public setup(colorsService: LegacyColorsService) {
return {
getPalettes: async (): Promise<PaletteRegistry> => {
if (!this.palettes) {
const { buildPalettes } = await import('./palettes');
this.palettes = buildPalettes(core.uiSettings, colorsService);
this.palettes = buildPalettes(colorsService);
}
return {
get: (name: string) => {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/charts/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class ChartsServerPlugin implements Plugin<object, object> {
type: 'json',
description: i18n.translate('charts.advancedSettings.visualization.colorMappingText', {
defaultMessage:
'Maps values to specific colors in <strong>Visualize</strong> charts and <strong>TSVB</strong>. This setting does not apply to <strong>Lens.</strong>',
'Maps values to specific colors in charts using the <strong>Compatibility</strong> palette.',
}),
deprecation: {
message: i18n.translate(
Expand Down

0 comments on commit 585f6f2

Please sign in to comment.