From 72e131a786e0c5add776634fb7d6456cb26d03c6 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Wed, 29 Sep 2021 18:52:50 +0200 Subject: [PATCH] [Lens] Thresholds: moving a threshold to another group should carry also its styling (#112853) * :bug: When dragging a threshold to another group carry also its styling * :white_check_mark: Add functional test * :sparkles: Make duplicate carry style too * :white_check_mark: Add functional test for duplicate use case * :bug: Fix table duplication issue Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../editor_frame/config_panel/layer_panel.tsx | 24 +++++++-- x-pack/plugins/lens/public/types.ts | 2 + .../public/xy_visualization/visualization.tsx | 15 ++++-- .../test/functional/apps/lens/thresholds.ts | 52 +++++++++++++++++++ 4 files changed, 85 insertions(+), 8 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 8c947d3502f93..6e2e77af4f3b0 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 @@ -178,9 +178,9 @@ export function LayerPanel( setNextFocusedButtonId(columnId); } - const filterOperations = - groups.find(({ groupId: gId }) => gId === targetItem.groupId)?.filterOperations || - (() => false); + const group = groups.find(({ groupId: gId }) => gId === groupId); + + const filterOperations = group?.filterOperations || (() => false); const dropResult = layerDatasourceOnDrop({ ...layerDatasourceDropProps, @@ -193,12 +193,28 @@ export function LayerPanel( dropType, }); if (dropResult) { + let previousColumn = + typeof droppedItem.column === 'string' ? droppedItem.column : undefined; + + // make it inherit only for moving and duplicate + if (!previousColumn) { + // when duplicating check if the previous column is required + if ( + dropType === 'duplicate_compatible' && + typeof droppedItem.columnId === 'string' && + group?.requiresPreviousColumnOnDuplicate + ) { + previousColumn = droppedItem.columnId; + } else { + previousColumn = typeof dropResult === 'object' ? dropResult.deleted : undefined; + } + } const newVisState = setDimension({ columnId, groupId, layerId: targetLayerId, prevState: props.visualizationState, - previousColumn: typeof droppedItem.column === 'string' ? droppedItem.column : undefined, + previousColumn, frame: framePublicAPI, }); diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index cf6634c200d55..75ed5f4907e0b 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -478,6 +478,8 @@ export type VisualizationDimensionGroupConfig = SharedDimensionProps & { // some type of layers can produce groups even if invalid. Keep this information to visually show the user that. invalid?: boolean; invalidMessage?: string; + // need a special flag to know when to pass the previous column on duplicating + requiresPreviousColumnOnDuplicate?: boolean; }; interface VisualizationDimensionChangeProps { diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index ed1cc015806c5..33cd01c8fda7a 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -367,6 +367,7 @@ export const getXyVisualization = ({ defaultMessage: 'This threshold is assigned to an axis that no longer exists. You may move this threshold to another available axis or remove it.', }), + requiresPreviousColumnOnDuplicate: true, })), }; } @@ -422,7 +423,7 @@ export const getXyVisualization = ({ return state.layers[0].palette; }, - setDimension({ prevState, layerId, columnId, groupId }) { + setDimension({ prevState, layerId, columnId, groupId, previousColumn }) { const foundLayer = prevState.layers.find((l) => l.layerId === layerId); if (!foundLayer) { return prevState; @@ -441,12 +442,21 @@ export const getXyVisualization = ({ if (newLayer.layerType === layerTypes.THRESHOLD) { newLayer.accessors = [...newLayer.accessors.filter((a) => a !== columnId), columnId]; const hasYConfig = newLayer.yConfig?.some(({ forAccessor }) => forAccessor === columnId); + const previousYConfig = previousColumn + ? newLayer.yConfig?.find(({ forAccessor }) => forAccessor === previousColumn) + : false; if (!hasYConfig) { newLayer.yConfig = [ ...(newLayer.yConfig || []), // TODO: move this // add a default config if none is available { + icon: undefined, + lineStyle: 'solid', + lineWidth: 1, + // override with previous styling, + ...previousYConfig, + // but keep the new group & id config forAccessor: columnId, axisMode: groupId === 'xThreshold' @@ -454,9 +464,6 @@ export const getXyVisualization = ({ : groupId === 'yThresholdRight' ? 'right' : 'left', - icon: undefined, - lineStyle: 'solid', - lineWidth: 1, }, ]; } diff --git a/x-pack/test/functional/apps/lens/thresholds.ts b/x-pack/test/functional/apps/lens/thresholds.ts index bf6535acc7c8e..10e330114442b 100644 --- a/x-pack/test/functional/apps/lens/thresholds.ts +++ b/x-pack/test/functional/apps/lens/thresholds.ts @@ -64,5 +64,57 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Median of bytes', ]); }); + + it('should add a new group to the threshold layer when a right axis is enabled', async () => { + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'average', + field: 'bytes', + keepOpen: true, + }); + + await PageObjects.lens.changeAxisSide('right'); + + await PageObjects.lens.closeDimensionEditor(); + + await testSubjects.existOrFail('lnsXY_yThresholdRightPanel > lns-empty-dimension'); + }); + + it('should carry the style when moving a threshold to another group', async () => { + // style it enabling the fill + await testSubjects.click('lnsXY_yThresholdLeftPanel > lns-dimensionTrigger'); + await testSubjects.click('lnsXY_fill_below'); + await PageObjects.lens.closeDimensionEditor(); + + // drag and drop it to the left axis + await PageObjects.lens.dragDimensionToDimension( + 'lnsXY_yThresholdLeftPanel > lns-dimensionTrigger', + 'lnsXY_yThresholdRightPanel > lns-empty-dimension' + ); + + await testSubjects.click('lnsXY_yThresholdRightPanel > lns-dimensionTrigger'); + expect( + await find.existsByCssSelector('[data-test-subj="lnsXY_fill_below"][class$="isSelected"]') + ).to.be(true); + await PageObjects.lens.closeDimensionEditor(); + }); + + it('should duplicate also the original style when duplicating a threshold', async () => { + // drag and drop to the empty field to generate a duplicate + await PageObjects.lens.dragDimensionToDimension( + 'lnsXY_yThresholdRightPanel > lns-dimensionTrigger', + 'lnsXY_yThresholdRightPanel > lns-empty-dimension' + ); + + await ( + await find.byCssSelector( + '[data-test-subj="lnsXY_yThresholdRightPanel"]:nth-child(2) [data-test-subj="lns-dimensionTrigger"]' + ) + ).click(); + expect( + await find.existsByCssSelector('[data-test-subj="lnsXY_fill_below"][class$="isSelected"]') + ).to.be(true); + await PageObjects.lens.closeDimensionEditor(); + }); }); }