From 1ff2bb3d8a9c1519391f7c5830e1ba7102e1b1ec Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 3 Dec 2020 20:30:42 +0100 Subject: [PATCH] [Lens] allow drag and drop reorder on xyChart for y dimension (#84640) * [Lens] allow drag and drop on xyChart for y dimension * wip * tests * dimension panel fix * eslint * fix test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../editor_frame/config_panel/layer_panel.tsx | 3 +-- .../xy_visualization/color_assignment.ts | 15 ++++++------ .../xy_visualization/visualization.test.ts | 24 +++++++++++++++++++ .../public/xy_visualization/visualization.tsx | 6 +++-- .../xy_visualization/xy_config_panel.tsx | 13 +++++++--- 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 4231f4b539977c..f372c0c25b43f7 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -247,8 +247,7 @@ export function LayerPanel( const isFromTheSameGroup = isDraggedOperation(dragging) && dragging.groupId === group.groupId && - dragging.columnId !== accessor && - dragging.groupId !== 'y'; // TODO: remove this line when https://github.com/elastic/elastic-charts/issues/868 is fixed + dragging.columnId !== accessor; const isDroppable = isDraggedOperation(dragging) ? dragType === 'reorder' diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index e5764eaf0e8c0a..210101dc25c767 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -24,7 +24,7 @@ export type ColorAssignments = Record< string, { totalSeriesCount: number; - getRank(layer: LayerColorConfig, seriesKey: string, yAccessor: string): number; + getRank(sortedLayer: LayerColorConfig, seriesKey: string, yAccessor: string): number; } >; @@ -72,8 +72,8 @@ export function getColorAssignments( ); return { totalSeriesCount, - getRank(layer: LayerColorConfig, seriesKey: string, yAccessor: string) { - const layerIndex = paletteLayers.indexOf(layer); + getRank(sortedLayer: LayerColorConfig, seriesKey: string, yAccessor: string) { + const layerIndex = paletteLayers.findIndex((l) => sortedLayer.layerId === l.layerId); const currentSeriesPerLayer = seriesPerLayer[layerIndex]; const splitRank = currentSeriesPerLayer.splits.indexOf(seriesKey); return ( @@ -82,8 +82,10 @@ export function getColorAssignments( : seriesPerLayer .slice(0, layerIndex) .reduce((sum, perLayer) => sum + perLayer.numberOfSeries, 0)) + - (layer.splitAccessor && splitRank !== -1 ? splitRank * layer.accessors.length : 0) + - layer.accessors.indexOf(yAccessor) + (sortedLayer.splitAccessor && splitRank !== -1 + ? splitRank * sortedLayer.accessors.length + : 0) + + sortedLayer.accessors.indexOf(yAccessor) ); }, }; @@ -94,13 +96,12 @@ export function getAccessorColorConfig( colorAssignments: ColorAssignments, frame: FramePublicAPI, layer: LayerConfig, - sortedAccessors: string[], paletteService: PaletteRegistry ): AccessorConfig[] { const layerContainsSplits = Boolean(layer.splitAccessor); const currentPalette: PaletteOutput = layer.palette || { type: 'palette', name: 'default' }; const totalSeriesCount = colorAssignments[currentPalette.name].totalSeriesCount; - return sortedAccessors.map((accessor) => { + return layer.accessors.map((accessor) => { const currentYConfig = layer.yConfig?.find((yConfig) => yConfig.forAccessor === accessor); if (layerContainsSplits) { return { diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts index d780ce85bad69a..cab1a0185333f9 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -558,6 +558,30 @@ describe('xy_visualization', () => { const accessorConfig = breakdownConfig!.accessors[0]; expect(typeof accessorConfig !== 'string' && accessorConfig.palette).toEqual(customColors); }); + + it('should respect the order of accessors coming from datasource', () => { + mockDatasource.publicAPIMock.getTableSpec.mockReturnValue([ + { columnId: 'c' }, + { columnId: 'b' }, + ]); + const paletteGetter = jest.spyOn(paletteServiceMock, 'get'); + // overrite palette with a palette returning first blue, then green as color + paletteGetter.mockReturnValue({ + id: 'default', + title: '', + getColors: jest.fn(), + toExpression: jest.fn(), + getColor: jest.fn().mockReturnValueOnce('blue').mockReturnValueOnce('green'), + }); + + const yConfigs = callConfigForYConfigs({}); + expect(yConfigs?.accessors[0].columnId).toEqual('c'); + expect(yConfigs?.accessors[0].color).toEqual('blue'); + expect(yConfigs?.accessors[1].columnId).toEqual('b'); + expect(yConfigs?.accessors[1].color).toEqual('green'); + + paletteGetter.mockClear(); + }); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index ebf80c61e0cd17..e05871fd35a5e3 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -187,8 +187,10 @@ export const getXyVisualization = ({ mappedAccessors = getAccessorColorConfig( colorAssignments, frame, - layer, - sortedAccessors, + { + ...layer, + accessors: sortedAccessors.filter((sorted) => layer.accessors.includes(sorted)), + }, paletteService ); } diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx index cd8a5993d3ecb6..dc6ce285754fc3 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx @@ -51,6 +51,7 @@ import { TooltipWrapper } from './tooltip_wrapper'; import { getAxesConfiguration } from './axes_configuration'; import { PalettePicker } from '../shared_components'; import { getAccessorColorConfig, getColorAssignments } from './color_assignment'; +import { getSortedAccessors } from './to_expression'; type UnwrapArray = T extends Array ? P : T; type AxesSettingsConfigKeys = keyof AxesSettingsConfig; @@ -579,6 +580,9 @@ const ColorPicker = ({ const currentColor = useMemo(() => { if (overwriteColor || !frame.activeData) return overwriteColor; + const datasource = frame.datasourceLayers[layer.layerId]; + const sortedAccessors: string[] = getSortedAccessors(datasource, layer); + const colorAssignments = getColorAssignments( state.layers, { tables: frame.activeData }, @@ -587,11 +591,14 @@ const ColorPicker = ({ const mappedAccessors = getAccessorColorConfig( colorAssignments, frame, - layer, - [accessor], + { + ...layer, + accessors: sortedAccessors.filter((sorted) => layer.accessors.includes(sorted)), + }, paletteService ); - return mappedAccessors[0].color; + + return mappedAccessors.find((a) => a.columnId === accessor)?.color || null; }, [overwriteColor, frame, paletteService, state.layers, accessor, formatFactory, layer]); const [color, setColor] = useState(currentColor);