github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 09:19:04 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Lens] Fix transition to custom palette inconsistency when in number mode (#110852)

Browse source 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Marco Liberati 2021-09-03 15:57:57 +02:00 • committed by GitHub
parent 9ba00ee594
commit 21b4752dba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 251 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

5
src/plugins/charts/public/services/palettes/helpers.ts
View file

@ -70,7 +70,10 @@ export function workoutColorForValue(
const comparisonFn = (v: number, threshold: number) => v - threshold;
// if steps are defined consider the specific rangeMax/Min as data boundaries
const maxRange = stops.length ? rangeMax : dataRangeArguments[1];
// as of max reduce its value by 1/colors.length for correct continuity checks
const maxRange = stops.length
? rangeMax
: dataRangeArguments[1] - (dataRangeArguments[1] - dataRangeArguments[0]) / colors.length;
const minRange = stops.length ? rangeMin : dataRangeArguments[0];
// in case of shorter rangers, extends the steps on the sides to cover the whole set

192
x-pack/plugins/lens/public/shared_components/coloring/palette_configuration.test.tsx
View file

@ -6,7 +6,7 @@
*/
import React from 'react';
import { EuiColorPalettePickerPaletteProps } from '@elastic/eui';
import { EuiButtonGroup, EuiColorPalettePickerPaletteProps } from '@elastic/eui';
import { mountWithIntl } from '@kbn/test/jest';
import { chartPluginMock } from 'src/plugins/charts/public/mocks';
import type { PaletteOutput, PaletteRegistry } from 'src/plugins/charts/public';
@ -14,6 +14,7 @@ import { ReactWrapper } from 'enzyme';
import type { CustomPaletteParams } from '../../../common';
import { applyPaletteParams } from './utils';
import { CustomizablePalette } from './palette_configuration';
import { act } from 'react-dom/test-utils';
// mocking random id generator function
jest.mock('@elastic/eui', () => {
@ -128,71 +129,136 @@ describe('palette panel', () => {
});
});
describe('reverse option', () => {
beforeEach(() => {
props = {
activePalette: { type: 'palette', name: 'positive' },
palettes: paletteRegistry,
setPalette: jest.fn(),
dataBounds: { min: 0, max: 100 },
};
});
it('should rewrite the min/max range values on palette change', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);
function toggleReverse(instance: ReactWrapper, checked: boolean) {
return instance
.find('[data-test-subj="lnsPalettePanel_dynamicColoring_reverse"]')
.first()
.prop('onClick')!({} as React.MouseEvent);
}
changePaletteIn(instance, 'custom');
it('should reverse the colorStops on click', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);
toggleReverse(instance, true);
expect(props.setPalette).toHaveBeenCalledWith(
expect.objectContaining({
params: expect.objectContaining({
reverse: true,
}),
})
);
});
});
describe('custom stops', () => {
beforeEach(() => {
props = {
activePalette: { type: 'palette', name: 'positive' },
palettes: paletteRegistry,
setPalette: jest.fn(),
dataBounds: { min: 0, max: 100 },
};
});
it('should be visible for predefined palettes', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);
expect(
instance.find('[data-test-subj="lnsPalettePanel_dynamicColoring_custom_stops"]').exists()
).toEqual(true);
});
it('should be visible for custom palettes', () => {
const instance = mountWithIntl(
<CustomizablePalette
{...props}
activePalette={{
type: 'palette',
name: 'custom',
params: {
name: 'custom',
},
}}
/>
);
expect(
instance.find('[data-test-subj="lnsPalettePanel_dynamicColoring_custom_stops"]').exists()
).toEqual(true);
expect(props.setPalette).toHaveBeenCalledWith({
type: 'palette',
name: 'custom',
params: expect.objectContaining({
rangeMin: 0,
rangeMax: 50,
}),
});
});
});
describe('reverse option', () => {
beforeEach(() => {
props = {
activePalette: { type: 'palette', name: 'positive' },
palettes: paletteRegistry,
setPalette: jest.fn(),
dataBounds: { min: 0, max: 100 },
};
});
function toggleReverse(instance: ReactWrapper, checked: boolean) {
return instance
.find('[data-test-subj="lnsPalettePanel_dynamicColoring_reverse"]')
.first()
.prop('onClick')!({} as React.MouseEvent);
}
it('should reverse the colorStops on click', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);
toggleReverse(instance, true);
expect(props.setPalette).toHaveBeenCalledWith(
expect.objectContaining({
params: expect.objectContaining({
reverse: true,
}),
})
);
});
});
describe('percentage / number modes', () => {
beforeEach(() => {
props = {
activePalette: { type: 'palette', name: 'positive' },
palettes: paletteRegistry,
setPalette: jest.fn(),
dataBounds: { min: 5, max: 200 },
};
});
it('should switch mode and range boundaries on click', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);
act(() => {
instance
.find('[data-test-subj="lnsPalettePanel_dynamicColoring_custom_range_groups"]')
.find(EuiButtonGroup)
.prop('onChange')!('number');
});
act(() => {
instance
.find('[data-test-subj="lnsPalettePanel_dynamicColoring_custom_range_groups"]')
.find(EuiButtonGroup)
.prop('onChange')!('percent');
});
expect(props.setPalette).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
params: expect.objectContaining({
rangeType: 'number',
rangeMin: 5,
rangeMax: 102.5 /* (200 - (200-5)/ colors.length: 2) */,
}),
})
);
expect(props.setPalette).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
params: expect.objectContaining({
rangeType: 'percent',
rangeMin: 0,
rangeMax: 50 /* 100 - (100-0)/ colors.length: 2 */,
}),
})
);
});
});
describe('custom stops', () => {
beforeEach(() => {
props = {
activePalette: { type: 'palette', name: 'positive' },
palettes: paletteRegistry,
setPalette: jest.fn(),
dataBounds: { min: 0, max: 100 },
};
});
it('should be visible for predefined palettes', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);
expect(
instance.find('[data-test-subj="lnsPalettePanel_dynamicColoring_custom_stops"]').exists()
).toEqual(true);
});
it('should be visible for custom palettes', () => {
const instance = mountWithIntl(
<CustomizablePalette
{...props}
activePalette={{
type: 'palette',
name: 'custom',
params: {
name: 'custom',
},
}}
/>
);
expect(
instance.find('[data-test-subj="lnsPalettePanel_dynamicColoring_custom_stops"]').exists()
).toEqual(true);
});
});
});

36
x-pack/plugins/lens/public/shared_components/coloring/palette_configuration.tsx
View file

@ -108,16 +108,21 @@ export function CustomizablePalette({
colorStops: undefined,
};
const newColorStops = getColorStops(palettes, [], activePalette, dataBounds);
if (isNewPaletteCustom) {
newParams.colorStops = getColorStops(palettes, [], activePalette, dataBounds);
newParams.colorStops = newColorStops;
}
newParams.stops = getPaletteStops(palettes, newParams, {
prevPalette:
isNewPaletteCustom || isCurrentPaletteCustom ? undefined : newPalette.name,
dataBounds,
mapFromMinValue: true,
});
newParams.rangeMin = newColorStops[0].stop;
newParams.rangeMax = newColorStops[newColorStops.length - 1].stop;
setPalette({
...newPalette,
params: newParams,
@ -266,18 +271,18 @@ export function CustomizablePalette({
) as RequiredPaletteParamTypes['rangeType'];
const params: CustomPaletteParams = { rangeType: newRangeType };
const { min: newMin, max: newMax } = getDataMinMax(newRangeType, dataBounds);
const { min: oldMin, max: oldMax } = getDataMinMax(
activePalette.params?.rangeType,
dataBounds
);
const newColorStops = remapStopsByNewInterval(colorStopsToShow, {
oldInterval: oldMax - oldMin,
newInterval: newMax - newMin,
newMin,
oldMin,
});
if (isCurrentPaletteCustom) {
const { min: newMin, max: newMax } = getDataMinMax(newRangeType, dataBounds);
const { min: oldMin, max: oldMax } = getDataMinMax(
activePalette.params?.rangeType,
dataBounds
);
const newColorStops = remapStopsByNewInterval(colorStopsToShow, {
oldInterval: oldMax - oldMin,
newInterval: newMax - newMin,
newMin,
oldMin,
});
const stops = getPaletteStops(
palettes,
{ ...activePalette.params, colorStops: newColorStops, ...params },
@ -285,8 +290,6 @@ export function CustomizablePalette({
);
params.colorStops = newColorStops;
params.stops = stops;
params.rangeMin = newColorStops[0].stop;
params.rangeMax = newColorStops[newColorStops.length - 1].stop;
} else {
params.stops = getPaletteStops(
palettes,
@ -294,6 +297,11 @@ export function CustomizablePalette({
{ prevPalette: activePalette.name, dataBounds }
);
}
// why not use newMin/newMax here?
// That's because there's the concept of continuity to accomodate, where in some scenarios it has to
// take into account the stop value rather than the data value
params.rangeMin = newColorStops[0].stop;
params.rangeMax = newColorStops[newColorStops.length - 1].stop;
setPalette(mergePaletteParams(activePalette, params));
}}
/>

73
x-pack/plugins/lens/public/shared_components/coloring/utils.test.ts
View file

@ -8,6 +8,7 @@
import { chartPluginMock } from 'src/plugins/charts/public/mocks';
import {
applyPaletteParams,
getColorStops,
getContrastColor,
getDataMinMax,
getPaletteStops,
@ -59,6 +60,78 @@ describe('applyPaletteParams', () => {
});
});
describe('getColorStops', () => {
const paletteRegistry = chartPluginMock.createPaletteRegistry();
it('should return the same colorStops if a custom palette is passed, avoiding recomputation', () => {
const colorStops = [
{ stop: 0, color: 'red' },
{ stop: 100, color: 'blue' },
];
expect(
getColorStops(
paletteRegistry,
colorStops,
{ name: 'custom', type: 'palette' },
{ min: 0, max: 100 }
)
).toBe(colorStops);
});
it('should get a fresh list of colors', () => {
expect(
getColorStops(
paletteRegistry,
[
{ stop: 0, color: 'red' },
{ stop: 100, color: 'blue' },
],
{ name: 'mocked', type: 'palette' },
{ min: 0, max: 100 }
)
).toEqual([
{ color: 'blue', stop: 0 },
{ color: 'yellow', stop: 50 },
]);
});
it('should get a fresh list of colors even if custom palette but empty colorStops', () => {
expect(
getColorStops(paletteRegistry, [], { name: 'mocked', type: 'palette' }, { min: 0, max: 100 })
).toEqual([
{ color: 'blue', stop: 0 },
{ color: 'yellow', stop: 50 },
]);
});
it('should correctly map the new colorStop to the current data bound and minValue', () => {
expect(
getColorStops(
paletteRegistry,
[],
{ name: 'mocked', type: 'palette', params: { rangeType: 'number' } },
{ min: 100, max: 1000 }
)
).toEqual([
{ color: 'blue', stop: 100 },
{ color: 'yellow', stop: 550 },
]);
});
it('should reverse the colors', () => {
expect(
getColorStops(
paletteRegistry,
[],
{ name: 'mocked', type: 'palette', params: { reverse: true } },
{ min: 100, max: 1000 }
)
).toEqual([
{ color: 'yellow', stop: 0 },
{ color: 'blue', stop: 50 },
]);
});
});
describe('remapStopsByNewInterval', () => {
it('should correctly remap the current palette from 0..1 to 0...100', () => {
expect(

5
x-pack/plugins/lens/public/shared_components/coloring/utils.ts
View file

@ -269,11 +269,10 @@ export function getColorStops(
palettes: PaletteRegistry,
colorStops: Required<CustomPaletteParams>['stops'],
activePalette: PaletteOutput<CustomPaletteParams>,
dataBounds: { min: number; max: number },
defaultPalette?: string
dataBounds: { min: number; max: number }
) {
// just forward the current stops if custom
if (activePalette?.name === CUSTOM_PALETTE) {
if (activePalette?.name === CUSTOM_PALETTE && colorStops?.length) {
return colorStops;
}
// for predefined palettes create some stops, then drop the last one.

21
x-pack/test/functional/apps/lens/table.ts
View file

@ -122,7 +122,28 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(styleObj['background-color']).to.be('rgb(235, 239, 245)');
});
it('should keep the coloring consistent when changing mode', async () => {
// Change mode from percent to number
await testSubjects.click('lnsPalettePanel_dynamicColoring_rangeType_groups_number');
await PageObjects.header.waitUntilLoadingHasFinished();
// check that all remained the same
const styleObj = await PageObjects.lens.getDatatableCellStyle(0, 2);
expect(styleObj['background-color']).to.be('rgb(235, 239, 245)');
});
it('should keep the coloring consistent when moving to custom palette from default', async () => {
await PageObjects.lens.changePaletteTo('custom');
await PageObjects.header.waitUntilLoadingHasFinished();
// check that all remained the same
const styleObj = await PageObjects.lens.getDatatableCellStyle(0, 2);
expect(styleObj['background-color']).to.be('rgb(235, 239, 245)');
});
it('tweak the color stops numeric value', async () => {
// restore default palette and percent mode
await PageObjects.lens.changePaletteTo('temperature');
await testSubjects.click('lnsPalettePanel_dynamicColoring_rangeType_groups_percent');
// now tweak the value
await testSubjects.setValue('lnsPalettePanel_dynamicColoring_stop_value_0', '30', {
clearWithKeyboard: true,
});