Skip to content

Commit

Permalink
[Lens] Reverse colors should not reverse palette picker previews (#11…
Browse files Browse the repository at this point in the history
…0455) (#111276)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
  • Loading branch information
kibanamachine and dej611 authored Sep 6, 2021
1 parent 25de643 commit d971470
Show file tree
Hide file tree
Showing 9 changed files with 414 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ReactWrapper } from 'enzyme';
import type { CustomPaletteParams } from '../../../common';
import { applyPaletteParams } from './utils';
import { CustomizablePalette } from './palette_configuration';
import { CUSTOM_PALETTE } from './constants';
import { act } from 'react-dom/test-utils';

// mocking random id generator function
Expand Down Expand Up @@ -129,6 +130,21 @@ describe('palette panel', () => {
});
});

it('should restore the reverse initial state on transitioning', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);

changePaletteIn(instance, 'negative');

expect(props.setPalette).toHaveBeenCalledWith({
type: 'palette',
name: 'negative',
params: expect.objectContaining({
name: 'negative',
reverse: false,
}),
});
});

it('should rewrite the min/max range values on palette change', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);

Expand Down Expand Up @@ -175,6 +191,20 @@ describe('palette panel', () => {
})
);
});

it('should transition a predefined palette to a custom one on reverse click', () => {
const instance = mountWithIntl(<CustomizablePalette {...props} />);

toggleReverse(instance, true);

expect(props.setPalette).toHaveBeenCalledWith(
expect.objectContaining({
params: expect.objectContaining({
name: CUSTOM_PALETTE,
}),
})
);
});
});

describe('percentage / number modes', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export function CustomizablePalette({
...activePalette.params,
name: newPalette.name,
colorStops: undefined,
reverse: false, // restore the reverse flag
};

const newColorStops = getColorStops(palettes, [], activePalette, dataBounds);
Expand Down Expand Up @@ -317,28 +318,20 @@ export function CustomizablePalette({
className="lnsPalettePanel__reverseButton"
data-test-subj="lnsPalettePanel_dynamicColoring_reverse"
onClick={() => {
const params: CustomPaletteParams = { reverse: !activePalette.params?.reverse };
if (isCurrentPaletteCustom) {
params.colorStops = reversePalette(colorStopsToShow);
params.stops = getPaletteStops(
palettes,
{
...(activePalette?.params || {}),
colorStops: params.colorStops,
},
{ dataBounds }
);
} else {
params.stops = reversePalette(
activePalette?.params?.stops ||
getPaletteStops(
palettes,
{ ...activePalette.params, ...params },
{ prevPalette: activePalette.name, dataBounds }
)
);
}
setPalette(mergePaletteParams(activePalette, params));
// when reversing a palette, the palette is automatically transitioned to a custom palette
const newParams = getSwitchToCustomParams(
palettes,
activePalette,
{
colorStops: reversePalette(colorStopsToShow),
steps: activePalette.params?.steps || DEFAULT_COLOR_STEPS,
reverse: !activePalette.params?.reverse, // Store the reverse state
rangeMin: colorStopsToShow[0]?.stop,
rangeMax: colorStopsToShow[colorStopsToShow.length - 1]?.stop,
},
dataBounds
);
setPalette(newParams);
}}
>
<EuiFlexGroup alignItems="center" gutterSize="xs" responsive={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export function PalettePicker({
value: id,
title,
type: FIXED_PROGRESSION,
palette: activePalette?.params?.reverse ? colors.reverse() : colors,
palette: colors,
'data-test-subj': `${id}-palette`,
};
});
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/lens/server/embeddable/lens_embeddable_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { EmbeddableRegistryDefinition } from 'src/plugins/embeddable/server';
import type { SerializableRecord } from '@kbn/utility-types';
import { DOC_TYPE } from '../../common';
import {
commonMakeReversePaletteAsCustom,
commonRemoveTimezoneDateHistogramParam,
commonRenameOperationsForFormula,
commonUpdateVisLayerType,
Expand All @@ -17,6 +18,7 @@ import {
LensDocShape713,
LensDocShape715,
LensDocShapePre712,
VisState716,
VisStatePre715,
} from '../migrations/types';
import { extract, inject } from '../../common/embeddable_factory';
Expand Down Expand Up @@ -50,6 +52,14 @@ export const lensEmbeddableFactory = (): EmbeddableRegistryDefinition => {
attributes: migratedLensState,
} as unknown) as SerializableRecord;
},
'7.16.0': (state) => {
const lensState = (state as unknown) as { attributes: LensDocShape715<VisState716> };
const migratedLensState = commonMakeReversePaletteAsCustom(lensState.attributes);
return ({
...lensState,
attributes: migratedLensState,
} as unknown) as SerializableRecord;
},
},
extract,
inject,
Expand Down
57 changes: 56 additions & 1 deletion x-pack/plugins/lens/server/migrations/common_migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { cloneDeep } from 'lodash';
import { PaletteOutput } from 'src/plugins/charts/common';
import {
LensDocShapePre712,
OperationTypePre712,
Expand All @@ -15,8 +16,9 @@ import {
LensDocShape715,
VisStatePost715,
VisStatePre715,
VisState716,
} from './types';
import { layerTypes } from '../../common';
import { CustomPaletteParams, layerTypes } from '../../common';

export const commonRenameOperationsForFormula = (
attributes: LensDocShapePre712
Expand Down Expand Up @@ -98,3 +100,56 @@ export const commonUpdateVisLayerType = (
}
return newAttributes as LensDocShape715<VisStatePost715>;
};

function moveDefaultPaletteToPercentCustomInPlace(palette?: PaletteOutput<CustomPaletteParams>) {
if (palette?.params?.reverse && palette.params.name !== 'custom' && palette.params.stops) {
// change to palette type to custom and migrate to a percentage type of mode
palette.name = 'custom';
palette.params.name = 'custom';
// we can make strong assumptions here:
// because it was a default palette reversed it means that stops were the default ones
// so when migrating, because there's no access to active data, we could leverage the
// percent rangeType to define colorStops in percent.
//
// Stops should be defined, but reversed, as the previous code was rewriting them on reverse.
//
// The only change the user should notice should be the mode changing from number to percent
// but the final result *must* be identical
palette.params.rangeType = 'percent';
const steps = palette.params.stops.length;
palette.params.rangeMin = 0;
palette.params.rangeMax = 80;
palette.params.steps = steps;
palette.params.colorStops = palette.params.stops.map(({ color }, index) => ({
color,
stop: (index * 100) / steps,
}));
palette.params.stops = palette.params.stops.map(({ color }, index) => ({
color,
stop: ((1 + index) * 100) / steps,
}));
}
}

export const commonMakeReversePaletteAsCustom = (
attributes: LensDocShape715<VisState716>
): LensDocShape715<VisState716> => {
const newAttributes = cloneDeep(attributes);
const vizState = (newAttributes as LensDocShape715<VisState716>).state.visualization;
if (
attributes.visualizationType !== 'lnsDatatable' &&
attributes.visualizationType !== 'lnsHeatmap'
) {
return newAttributes;
}
if ('columns' in vizState) {
for (const column of vizState.columns) {
if (column.colorMode && column.colorMode !== 'none') {
moveDefaultPaletteToPercentCustomInPlace(column.palette);
}
}
} else {
moveDefaultPaletteToPercentCustomInPlace(vizState.palette);
}
return newAttributes;
};
Loading

0 comments on commit d971470

Please sign in to comment.