From d678acb5bf73d4f4fe180456505ff5c9dd5ee07b Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 1 Dec 2020 14:45:15 +0100 Subject: [PATCH 1/6] [Lens] allow drag and drop on xyChart for y dimension --- .../editor_frame/config_panel/layer_panel.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 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 67c6068dd4d91..af5444c8b0a64 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 @@ -241,8 +241,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' From a5eaaff0457a516c353921a346944daabf9bd477 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 2 Dec 2020 15:49:40 +0100 Subject: [PATCH 2/6] wip --- .../lens/public/xy_visualization/color_assignment.ts | 12 +++++++----- .../lens/public/xy_visualization/visualization.tsx | 9 +++++---- 2 files changed, 12 insertions(+), 9 deletions(-) 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 68c47e11acfc0..eafa5234d3aa1 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -22,7 +22,7 @@ export type ColorAssignments = Record< string, { totalSeriesCount: number; - getRank(layer: LayerColorConfig, seriesKey: string, yAccessor: string): number; + getRank(sortedLayer: LayerColorConfig, seriesKey: string, yAccessor: string): number; } >; @@ -70,8 +70,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 ( @@ -80,8 +80,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) ); }, }; diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index 872eb179e6a5c..44d6790cf1ed4 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -192,8 +192,10 @@ export const getXyVisualization = ({ mappedAccessors = getAccessorColorConfig( colorAssignments, frame, - layer, - sortedAccessors, + { + ...layer, + accessors: sortedAccessors.filter((sorted) => layer.accessors.includes(sorted)), + }, paletteService ); } @@ -379,13 +381,12 @@ 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 { From e1258edbce39e3f2306b6fe1ac7cac311da01eba Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 2 Dec 2020 19:53:31 +0100 Subject: [PATCH 3/6] tests --- .../xy_visualization/visualization.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) 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 546cf06d4014e..1eda269a66de3 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,27 @@ 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', () => { + const colorAssignment = require('./color_assignment'); + const getAccessorColorConfigSpy = jest.spyOn(colorAssignment, 'getAccessorColorConfig'); + mockDatasource.publicAPIMock.getTableSpec.mockReturnValue([ + { columnId: 'c' }, + { columnId: 'b' }, + ]); + callConfigForYConfigs({}); + expect(getAccessorColorConfigSpy).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + { + accessors: ['c', 'b'], + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + }, + expect.anything() + ); + }); }); }); From 1241d81493837095faff30ecac3ab6147f260fce Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 3 Dec 2020 09:25:36 +0100 Subject: [PATCH 4/6] dimension panel fix --- .../public/xy_visualization/xy_config_panel.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) 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 cd8a5993d3ecb..dc6ce285754fc 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); From aff9415985bce5619b45dabcff97d1c2daf0188f Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 3 Dec 2020 11:04:54 +0100 Subject: [PATCH 5/6] eslint --- .../plugins/lens/public/xy_visualization/visualization.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1eda269a66de3..76f0076fc1fd9 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -560,7 +560,7 @@ describe('xy_visualization', () => { }); it('should respect the order of accessors coming from datasource', () => { - const colorAssignment = require('./color_assignment'); + const colorAssignment = require('./color_assignment'); // eslint-disable-line @typescript-eslint/no-var-requires const getAccessorColorConfigSpy = jest.spyOn(colorAssignment, 'getAccessorColorConfig'); mockDatasource.publicAPIMock.getTableSpec.mockReturnValue([ { columnId: 'c' }, From 8e3203f90367d5be12374298d2a2037a42680841 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 3 Dec 2020 14:20:02 +0100 Subject: [PATCH 6/6] fix test --- .../xy_visualization/visualization.test.ts | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) 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 5747e6592688d..cab1a0185333f 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -560,24 +560,27 @@ describe('xy_visualization', () => { }); it('should respect the order of accessors coming from datasource', () => { - const colorAssignment = require('./color_assignment'); // eslint-disable-line @typescript-eslint/no-var-requires - const getAccessorColorConfigSpy = jest.spyOn(colorAssignment, 'getAccessorColorConfig'); mockDatasource.publicAPIMock.getTableSpec.mockReturnValue([ { columnId: 'c' }, { columnId: 'b' }, ]); - callConfigForYConfigs({}); - expect(getAccessorColorConfigSpy).toHaveBeenCalledWith( - expect.anything(), - expect.anything(), - { - accessors: ['c', 'b'], - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - }, - expect.anything() - ); + 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(); }); }); });