simplify popover open state logic

elastic · Dec 9, 2020 · 1c0a004 · 1c0a004
1 parent b3bccc2
commit 1c0a004
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 42 deletions.
diff --git a/...ens/public/indexpattern_datasource/operations/definitions/filters/filter_popover.test.tsx b/...ens/public/indexpattern_datasource/operations/definitions/filters/filter_popover.test.tsx
@@ -5,7 +5,7 @@
  */
 
 import React, { MouseEventHandler } from 'react';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
 import { act } from 'react-dom/test-utils';
 import { EuiPopover, EuiLink } from '@elastic/eui';
 import { createMockedIndexPattern } from '../../../mocks';
@@ -28,8 +28,7 @@ const defaultProps = {
   Button: ({ onClick }: { onClick: MouseEventHandler }) => (
     <EuiLink onClick={onClick}>trigger</EuiLink>
   ),
-  isOpenByCreation: true,
-  setIsOpenByCreation: jest.fn(),
+  initiallyOpen: true,
 };
 
 describe('filter popover', () => {
@@ -39,16 +38,14 @@ describe('filter popover', () => {
     },
   }));
   it('should be open if is open by creation', () => {
-    const setIsOpenByCreation = jest.fn();
-    const instance = shallow(
-      <FilterPopover {...defaultProps} setIsOpenByCreation={setIsOpenByCreation} />
-    );
+    const instance = mount(<FilterPopover {...defaultProps} />);
+    instance.update();
     expect(instance.find(EuiPopover).prop('isOpen')).toEqual(true);
     act(() => {
       instance.find(EuiPopover).prop('closePopover')!();
     });
     instance.update();
-    expect(setIsOpenByCreation).toHaveBeenCalledWith(false);
+    expect(instance.find(EuiPopover).prop('isOpen')).toEqual(false);
   });
   it('should call setFilter when modifying QueryInput', () => {
     const setFilter = jest.fn();

diff --git a/...ins/lens/public/indexpattern_datasource/operations/definitions/filters/filter_popover.tsx b/...ins/lens/public/indexpattern_datasource/operations/definitions/filters/filter_popover.tsx
@@ -5,7 +5,7 @@
  */
 import './filter_popover.scss';
 
-import React, { MouseEventHandler, useState } from 'react';
+import React, { MouseEventHandler, useEffect, useState } from 'react';
 import useDebounce from 'react-use/lib/useDebounce';
 import { EuiPopover, EuiSpacer } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
@@ -19,23 +19,24 @@ export const FilterPopover = ({
   setFilter,
   indexPattern,
   Button,
-  isOpenByCreation,
-  setIsOpenByCreation,
+  initiallyOpen,
 }: {
   filter: FilterValue;
   setFilter: Function;
   indexPattern: IndexPattern;
   Button: React.FunctionComponent<{ onClick: MouseEventHandler }>;
-  isOpenByCreation: boolean;
-  setIsOpenByCreation: Function;
+  initiallyOpen: boolean;
 }) => {
   const [isPopoverOpen, setIsPopoverOpen] = useState(false);
   const inputRef = React.useRef<HTMLInputElement>();
 
+  // set popover open on start to work around EUI bug
+  useEffect(() => {
+    setIsPopoverOpen(initiallyOpen);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
   const closePopover = () => {
-    if (isOpenByCreation) {
-      setIsOpenByCreation(false);
-    }
     if (isPopoverOpen) {
       setIsPopoverOpen(false);
     }
@@ -59,15 +60,12 @@ export const FilterPopover = ({
       data-test-subj="indexPattern-filters-existingFilterContainer"
       anchorClassName="eui-fullWidth"
       panelClassName="lnsIndexPatternDimensionEditor__filtersEditor"
-      isOpen={isOpenByCreation || isPopoverOpen}
+      isOpen={isPopoverOpen}
       ownFocus
       closePopover={() => closePopover()}
       button={
         <Button
           onClick={() => {
-            if (isOpenByCreation) {
-              setIsOpenByCreation(false);
-            }
             setIsPopoverOpen((open) => !open);
           }}
         />

diff --git a/...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx b/...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx
@@ -203,7 +203,7 @@ export const FilterList = ({
     <>
       <DragDropBuckets
         onDragEnd={updateFilters}
-        onDragStart={() => setIsOpenByCreation(false)}
+        onDragStart={() => {}}
         droppableId="FILTERS_DROPPABLE_AREA"
         items={localFilters}
       >
@@ -227,8 +227,7 @@ export const FilterList = ({
             >
               <FilterPopover
                 data-test-subj="indexPattern-filters-existingFilterContainer"
-                isOpenByCreation={idx === localFilters.length - 1 && isOpenByCreation}
-                setIsOpenByCreation={setIsOpenByCreation}
+                initiallyOpen={idx === localFilters.length - 1 && isOpenByCreation}
                 indexPattern={indexPattern}
                 filter={filter}
                 setFilter={(f: FilterValue) => {

diff --git a/...ins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx b/...ins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx
@@ -6,7 +6,7 @@
 
 import './advanced_editor.scss';
 
-import React, { useState, MouseEventHandler } from 'react';
+import React, { useState, MouseEventHandler, useEffect } from 'react';
 import { i18n } from '@kbn/i18n';
 import {
   EuiFlexGroup,
@@ -47,18 +47,22 @@ export const RangePopover = ({
   range,
   setRange,
   Button,
-  isOpenByCreation,
-  setIsOpenByCreation,
+  initiallyOpen,
 }: {
   range: LocalRangeType;
   setRange: (newRange: LocalRangeType) => void;
   Button: React.FunctionComponent<{ onClick: MouseEventHandler }>;
-  isOpenByCreation: boolean;
-  setIsOpenByCreation: (open: boolean) => void;
+  initiallyOpen: boolean;
 }) => {
   const [isPopoverOpen, setIsPopoverOpen] = useState(false);
   const [tempRange, setTempRange] = useState(range);
 
+  // set popover open on start to work around EUI bug
+  useEffect(() => {
+    setIsPopoverOpen(initiallyOpen);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
   const saveRangeAndReset = (newRange: LocalRangeType, resetRange = false) => {
     if (resetRange) {
       // reset the temporary range for later use
@@ -86,9 +90,6 @@ export const RangePopover = ({
   });
 
   const onSubmit = () => {
-    if (isOpenByCreation) {
-      setIsOpenByCreation(false);
-    }
     if (isPopoverOpen) {
       setIsPopoverOpen(false);
     }
@@ -98,14 +99,11 @@ export const RangePopover = ({
     <EuiPopover
       display="block"
       ownFocus
-      isOpen={isOpenByCreation || isPopoverOpen}
+      isOpen={isPopoverOpen}
       closePopover={onSubmit}
       button={
         <Button
           onClick={() => {
-            if (isOpenByCreation) {
-              setIsOpenByCreation(false);
-            }
             setIsPopoverOpen((isOpen) => !isOpen);
           }}
         />
@@ -255,11 +253,7 @@ export const AdvancedRangeEditor = ({
       <>
         <DragDropBuckets
           onDragEnd={setLocalRanges}
-          onDragStart={() => {
-            if (isOpenByCreation) {
-              setIsOpenByCreation(false);
-            }
-          }}
+          onDragStart={() => {}}
           droppableId="RANGES_DROPPABLE_AREA"
           items={localRanges}
         >
@@ -283,8 +277,7 @@ export const AdvancedRangeEditor = ({
             >
               <RangePopover
                 range={range}
-                isOpenByCreation={idx === lastIndex && isOpenByCreation}
-                setIsOpenByCreation={setIsOpenByCreation}
+                initiallyOpen={idx === lastIndex && isOpenByCreation}
                 setRange={(newRange: LocalRangeType) => {
                   const newRanges = [...localRanges];
                   if (newRange.id === newRanges[idx].id) {