From ce97982522cfb2b1e6b69810e72310d30ba00120 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 5 Oct 2020 12:41:09 +0200 Subject: [PATCH] [Lens] refactor DimensionContainer and fix flyout bug (#79277) --- .../config_panel/dimension_container.scss | 10 - .../config_panel/dimension_container.tsx | 97 ++----- .../config_panel/layer_panel.test.tsx | 23 +- .../editor_frame/config_panel/layer_panel.tsx | 252 ++++++++---------- .../editor_frame/config_panel/types.ts | 9 +- 5 files changed, 146 insertions(+), 245 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss index f200e25453a2af..bd2789cf645c72 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss @@ -13,17 +13,7 @@ animation: euiFlyout $euiAnimSpeedNormal $euiAnimSlightResistance; } -.lnsDimensionContainer--noAnimation { - animation: none; -} - .lnsDimensionContainer__footer, .lnsDimensionContainer__header { padding: $euiSizeS; } - -.lnsDimensionContainer__trigger { - width: 100%; - display: block; - word-break: break-word; -} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx index 19f4c0428260e1..8f1b441d1d285c 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx @@ -16,89 +16,42 @@ import { EuiOutsideClickDetector, } from '@elastic/eui'; -import classNames from 'classnames'; import { i18n } from '@kbn/i18n'; -import { VisualizationDimensionGroupConfig } from '../../../types'; -import { DimensionContainerState } from './types'; export function DimensionContainer({ - dimensionContainerState, - setDimensionContainerState, - groups, - accessor, - groupId, - trigger, + isOpen, + groupLabel, + handleClose, panel, - panelTitle, }: { - dimensionContainerState: DimensionContainerState; - setDimensionContainerState: (newState: DimensionContainerState) => void; - groups: VisualizationDimensionGroupConfig[]; - accessor: string; - groupId: string; - trigger: React.ReactElement; + isOpen: boolean; + handleClose: () => void; panel: React.ReactElement; - panelTitle: React.ReactNode; + groupLabel: string; }) { - const [openByCreation, setIsOpenByCreation] = useState( - dimensionContainerState.openId === accessor - ); const [focusTrapIsEnabled, setFocusTrapIsEnabled] = useState(false); - const [flyoutIsVisible, setFlyoutIsVisible] = useState(false); - - const noMatch = dimensionContainerState.isOpen - ? !groups.some((d) => d.accessors.includes(accessor)) - : false; const closeFlyout = () => { - setDimensionContainerState({ - isOpen: false, - openId: null, - addingToGroupId: null, - }); - setIsOpenByCreation(false); + handleClose(); setFocusTrapIsEnabled(false); - setFlyoutIsVisible(false); - }; - - const openFlyout = () => { - setFlyoutIsVisible(true); - setTimeout(() => { - setFocusTrapIsEnabled(true); - }, 255); }; - const flyoutShouldBeOpen = - dimensionContainerState.isOpen && - (dimensionContainerState.openId === accessor || - (noMatch && dimensionContainerState.addingToGroupId === groupId)); - useEffect(() => { - if (flyoutShouldBeOpen) { - openFlyout(); + if (isOpen) { + // without setTimeout here the flyout pushes content when animating + setTimeout(() => { + setFocusTrapIsEnabled(true); + }, 255); } - }); + }, [isOpen]); - useEffect(() => { - if (!flyoutShouldBeOpen) { - if (flyoutIsVisible) { - setFlyoutIsVisible(false); - } - if (focusTrapIsEnabled) { - setFocusTrapIsEnabled(false); - } - } - }, [flyoutShouldBeOpen, flyoutIsVisible, focusTrapIsEnabled]); - - const flyout = flyoutIsVisible && ( + return isOpen ? ( - +
@@ -109,7 +62,14 @@ export function DimensionContainer({ iconType="sortLeft" flush="left" > - {panelTitle} + + {i18n.translate('xpack.lens.configure.configurePanelTitle', { + defaultMessage: '{groupLabel} configuration', + values: { + groupLabel, + }, + })} + @@ -126,12 +86,5 @@ export function DimensionContainer({
- ); - - return ( - <> - {trigger} - {flyout} - - ); + ) : null; } 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 a9e2d6dc696ab6..44dc22d20a4fe8 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 @@ -14,7 +14,6 @@ import { } from '../../mocks'; import { ChildDragDropProvider } from '../../../drag_drop'; import { EuiFormRow } from '@elastic/eui'; -import { mount } from 'enzyme'; import { mountWithIntl } from 'test_utils/enzyme_helpers'; import { Visualization } from '../../../types'; import { LayerPanel } from './layer_panel'; @@ -211,7 +210,7 @@ describe('LayerPanel', () => { groupId: 'a', accessors: ['newid'], filterOperations: () => true, - supportsMoreColumns: false, + supportsMoreColumns: true, dataTestSubj: 'lnsGroup', enableDimensionEditor: true, }, @@ -220,11 +219,14 @@ describe('LayerPanel', () => { mockVisualization.renderDimensionEditor = jest.fn(); const component = mountWithIntl(); + act(() => { + component.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); + }); + component.update(); - const group = component.find('DimensionContainer'); - const panel = mount(group.prop('panel')); - - expect(panel.children()).toHaveLength(2); + const group = component.find('DimensionContainer').first(); + const panel: React.ReactElement = group.prop('panel'); + expect(panel.props.children).toHaveLength(2); }); it('should keep the DimensionContainer open when configuring a new dimension', () => { @@ -263,11 +265,8 @@ describe('LayerPanel', () => { }); const component = mountWithIntl(); - - const group = component.find('DimensionContainer'); - const triggerButton = mountWithIntl(group.prop('trigger')); act(() => { - triggerButton.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); + component.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); }); component.update(); @@ -312,10 +311,8 @@ describe('LayerPanel', () => { const component = mountWithIntl(); - const group = component.find('DimensionContainer'); - const triggerButton = mountWithIntl(group.prop('trigger')); act(() => { - triggerButton.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); + component.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click'); }); component.update(); expect(component.find('EuiFlyoutHeader').exists()).toBe(true); 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 ce2955da890d7c..e72bf75b010c3e 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 @@ -23,13 +23,11 @@ import { DragContext, DragDrop, ChildDragDropProvider } from '../../../drag_drop import { LayerSettings } from './layer_settings'; import { trackUiEvent } from '../../../lens_ui_telemetry'; import { generateId } from '../../../id_generator'; -import { ConfigPanelWrapperProps, DimensionContainerState } from './types'; +import { ConfigPanelWrapperProps, ActiveDimensionState } from './types'; import { DimensionContainer } from './dimension_container'; -const initialDimensionContainerState = { - isOpen: false, - openId: null, - addingToGroupId: null, +const initialActiveDimensionState = { + isNew: false, }; function isConfiguration( @@ -70,15 +68,15 @@ export function LayerPanel( } ) { const dragDropContext = useContext(DragContext); - const [dimensionContainerState, setDimensionContainerState] = useState( - initialDimensionContainerState + const [activeDimension, setActiveDimension] = useState( + initialActiveDimensionState ); const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer, dataTestSubj } = props; const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId]; useEffect(() => { - setDimensionContainerState(initialDimensionContainerState); + setActiveDimension(initialActiveDimensionState); }, [props.activeVisualizationId]); if ( @@ -117,7 +115,7 @@ export function LayerPanel( const { groups } = activeVisualization.getConfiguration(layerVisualizationConfigProps); const isEmptyLayer = !groups.some((d) => d.accessors.length > 0); - + const { activeId, activeGroup } = activeDimension; return ( @@ -196,31 +194,6 @@ export function LayerPanel( > <> {group.accessors.map((accessor) => { - const datasourceDimensionEditor = ( - - ); - const visDimensionEditor = - activeVisualization.renderDimensionEditor && group.enableDimensionEditor ? ( -
- -
- ) : null; return (
- { - if (dimensionContainerState.isOpen) { - setDimensionContainerState(initialDimensionContainerState); - } else { - setDimensionContainerState({ - isOpen: true, - openId: accessor, - addingToGroupId: null, // not set for existing dimension - }); - } - }, - }} - /> - } - panel={ - <> - {datasourceDimensionEditor} - {visDimensionEditor} - - } - panelTitle={i18n.translate('xpack.lens.configure.configurePanelTitle', { - defaultMessage: '{groupLabel} configuration', - values: { - groupLabel: group.groupLabel, + { + if (activeId) { + setActiveDimension(initialActiveDimensionState); + } else { + setActiveDimension({ + isNew: false, + activeGroup: group, + activeId: accessor, + }); + } }, - })} + }} /> -
- { - if (dimensionContainerState.isOpen) { - setDimensionContainerState(initialDimensionContainerState); - } else { - setDimensionContainerState({ - isOpen: true, - openId: newId, - addingToGroupId: group.groupId, - }); - } - }} - > - - - } - panelTitle={i18n.translate('xpack.lens.configure.configurePanelTitle', { - defaultMessage: '{groupLabel} configuration', - values: { - groupLabel: group.groupLabel, - }, - })} - panel={ - { - props.updateAll( - datasourceId, - newState, - activeVisualization.setDimension({ - layerId, - groupId: group.groupId, - columnId: newId, - prevState: props.visualizationState, - }) - ); - setDimensionContainerState({ - isOpen: true, - openId: newId, - addingToGroupId: null, // clear now that dimension exists - }); - }, - }} - /> - } - /> + { + if (activeId) { + setActiveDimension(initialActiveDimensionState); + } else { + setActiveDimension({ + isNew: true, + activeGroup: group, + activeId: newId, + }); + } + }} + > + +
) : null} @@ -472,6 +378,60 @@ export function LayerPanel( ); })} + setActiveDimension(initialActiveDimensionState)} + panel={ + <> + {activeGroup && activeId && ( + { + props.updateAll( + datasourceId, + newState, + activeVisualization.setDimension({ + layerId, + groupId: activeGroup.groupId, + columnId: activeId, + prevState: props.visualizationState, + }) + ); + setActiveDimension({ + ...activeDimension, + isNew: false, + }); + }, + }} + /> + )} + {activeGroup && + activeId && + !activeDimension.isNew && + activeVisualization.renderDimensionEditor && + activeGroup?.enableDimensionEditor && ( +
+ +
+ )} + + } + /> diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts index d42c5c3b99e53b..c172c6da6848c2 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts @@ -10,6 +10,7 @@ import { FramePublicAPI, Datasource, DatasourceDimensionEditorProps, + VisualizationDimensionGroupConfig, } from '../../../types'; export interface ConfigPanelWrapperProps { @@ -30,8 +31,8 @@ export interface ConfigPanelWrapperProps { core: DatasourceDimensionEditorProps['core']; } -export interface DimensionContainerState { - isOpen: boolean; - openId: string | null; - addingToGroupId: string | null; +export interface ActiveDimensionState { + isNew: boolean; + activeId?: string; + activeGroup?: VisualizationDimensionGroupConfig; }