[Lens] refactor DimensionContainer and fix flyout bug (#79277)

elastic · Oct 5, 2020 · ce97982 · ce97982
1 parent 4db2203
commit ce97982
Show file tree

Hide file tree

Showing 5 changed files with 146 additions and 245 deletions.
diff --git a/...ugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss b/...ugins/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/...lugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx b/...lugins/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 ? (
     <EuiFocusTrap disabled={!focusTrapIsEnabled} clickOutsideDisables={true}>
-      <EuiOutsideClickDetector onOutsideClick={closeFlyout} isDisabled={!flyoutIsVisible}>
+      <EuiOutsideClickDetector onOutsideClick={closeFlyout} isDisabled={!isOpen}>
         <div
           role="dialog"
           aria-labelledby="lnsDimensionContainerTitle"
-          className={classNames('lnsDimensionContainer', {
-            'lnsDimensionContainer--noAnimation': openByCreation,
-          })}
+          className="lnsDimensionContainer"
         >
           <EuiFlyoutHeader hasBorder className="lnsDimensionContainer__header">
             <EuiTitle size="xs">
@@ -109,7 +62,14 @@ export function DimensionContainer({
                 iconType="sortLeft"
                 flush="left"
               >
-                <strong>{panelTitle}</strong>
+                <strong>
+                  {i18n.translate('xpack.lens.configure.configurePanelTitle', {
+                    defaultMessage: '{groupLabel} configuration',
+                    values: {
+                      groupLabel,
+                    },
+                  })}
+                </strong>
               </EuiButtonEmpty>
             </EuiTitle>
           </EuiFlyoutHeader>
@@ -126,12 +86,5 @@ export function DimensionContainer({
         </div>
       </EuiOutsideClickDetector>
     </EuiFocusTrap>
-  );
-
-  return (
-    <>
-      {trigger}
-      {flyout}
-    </>
-  );
+  ) : null;
 }
diff --git a/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/...k/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(<LayerPanel {...getDefaultProps()} />);
+      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(<LayerPanel {...getDefaultProps()} />);
-
-      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(<LayerPanel {...getDefaultProps()} />);
 
-      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);