diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx index 9545bd3c840d..b100c885466a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx @@ -24,6 +24,7 @@ jest.mock('../../../id_generator'); describe('LayerPanel', () => { let mockVisualization: jest.Mocked; + let mockVisualization2: jest.Mocked; let mockDatasource: DatasourceMock; function getDefaultProps() { @@ -36,6 +37,7 @@ describe('LayerPanel', () => { activeVisualizationId: 'vis1', visualizationMap: { vis1: mockVisualization, + vis2: mockVisualization2, }, activeDatasourceId: 'ds1', datasourceMap: { @@ -72,6 +74,18 @@ describe('LayerPanel', () => { ], }; + mockVisualization2 = { + ...createMockVisualization(), + id: 'testVis2', + visualizationTypes: [ + { + icon: 'empty', + id: 'testVis2', + label: 'TEST2', + }, + ], + }; + mockVisualization.getLayerIds.mockReturnValue(['first']); mockDatasource = createMockDatasource('ds1'); }); @@ -209,16 +223,6 @@ describe('LayerPanel', () => { const panel = mount(group.prop('panel')); expect(panel.find('EuiTabbedContent').prop('tabs')).toHaveLength(2); - act(() => { - panel.find('EuiTab#visualization').simulate('click'); - }); - expect(mockVisualization.renderDimensionEditor).toHaveBeenCalledWith( - expect.any(Element), - expect.objectContaining({ - groupId: 'a', - accessor: 'newid', - }) - ); }); it('should keep the popover open when configuring a new dimension', () => { @@ -267,5 +271,56 @@ describe('LayerPanel', () => { expect(component.find(EuiPopover).prop('isOpen')).toBe(true); }); + it('should close the popover when the active visualization changes', () => { + /** + * The ID generation system for new dimensions has been messy before, so + * this tests that the ID used in the first render is used to keep the popover + * open in future renders + */ + + (generateId as jest.Mock).mockReturnValueOnce(`newid`); + (generateId as jest.Mock).mockReturnValueOnce(`bad`); + mockVisualization.getConfiguration.mockReturnValueOnce({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: [], + filterOperations: () => true, + supportsMoreColumns: true, + dataTestSubj: 'lnsGroup', + }, + ], + }); + // Normally the configuration would change in response to a state update, + // but this test is updating it directly + mockVisualization.getConfiguration.mockReturnValueOnce({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: ['newid'], + filterOperations: () => true, + supportsMoreColumns: false, + dataTestSubj: 'lnsGroup', + }, + ], + }); + + const component = mountWithIntl(); + + const group = component.find('DimensionPopover'); + const triggerButton = mountWithIntl(group.prop('trigger')); + act(() => { + triggerButton.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); + }); + component.update(); + expect(component.find(EuiPopover).prop('isOpen')).toBe(true); + act(() => { + component.setProps({ activeVisualizationId: 'vis2' }); + }); + component.update(); + expect(component.find(EuiPopover).prop('isOpen')).toBe(false); + }); }); }); 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 f72b1429967d..a384e339e8fb 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 @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useContext, useState } from 'react'; +import React, { useContext, useState, useEffect } from 'react'; import { EuiPanel, EuiSpacer, @@ -26,6 +26,13 @@ import { generateId } from '../../../id_generator'; import { ConfigPanelWrapperProps, DimensionPopoverState } from './types'; import { DimensionPopover } from './dimension_popover'; +const initialPopoverState = { + isOpen: false, + openId: null, + addingToGroupId: null, + tabId: null, +}; + export function LayerPanel( props: Exclude & { layerId: string; @@ -41,15 +48,15 @@ export function LayerPanel( } ) { const dragDropContext = useContext(DragContext); - const [popoverState, setPopoverState] = useState({ - isOpen: false, - openId: null, - addingToGroupId: null, - tabId: null, - }); + const [popoverState, setPopoverState] = useState(initialPopoverState); const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer } = props; const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId]; + + useEffect(() => { + setPopoverState(initialPopoverState); + }, [props.activeVisualizationId]); + if ( !datasourcePublicAPI || !props.activeVisualizationId || @@ -243,12 +250,7 @@ export function LayerPanel( suggestedPriority: group.suggestedPriority, togglePopover: () => { if (popoverState.isOpen) { - setPopoverState({ - isOpen: false, - openId: null, - addingToGroupId: null, - tabId: null, - }); + setPopoverState(initialPopoverState); } else { setPopoverState({ isOpen: true, @@ -264,7 +266,7 @@ export function LayerPanel( panel={ t.id === popoverState.tabId)} + selectedTab={tabs.find((t) => t.id === popoverState.tabId) || tabs[0]} size="s" onTabClick={(tab) => { setPopoverState({ @@ -358,12 +360,7 @@ export function LayerPanel( })} onClick={() => { if (popoverState.isOpen) { - setPopoverState({ - isOpen: false, - openId: null, - addingToGroupId: null, - tabId: null, - }); + setPopoverState(initialPopoverState); } else { setPopoverState({ isOpen: true,