[Lens] Fix focus on submitting filter popover (#125247)

* fix focus * fix and make consistent * fix test
elastic · Feb 11, 2022 · 2c4e476 · 2c4e476
1 parent b0e534b
commit 2c4e476
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 49 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
@@ -41,7 +41,7 @@ describe('filter popover', () => {
       },
       setFilter: jest.fn(),
       indexPattern: createMockedIndexPattern(),
-      Button: () => <EuiLink onClick={mockOnClick}>trigger</EuiLink>,
+      button: <EuiLink onClick={mockOnClick}>trigger</EuiLink>,
       isOpen: true,
       triggerClose: () => {},
     };

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
@@ -19,14 +19,14 @@ export const FilterPopover = ({
   filter,
   setFilter,
   indexPattern,
-  Button,
+  button,
   isOpen,
   triggerClose,
 }: {
   filter: FilterValue;
   setFilter: Function;
   indexPattern: IndexPattern;
-  Button: React.FunctionComponent;
+  button: React.ReactChild;
   isOpen: boolean;
   triggerClose: () => void;
 }) => {
@@ -78,12 +78,13 @@ export const FilterPopover = ({
       isOpen={isOpen}
       ownFocus
       closePopover={() => triggerClose()}
-      button={<Button />}
+      button={button}
     >
       <QueryInput
         isInvalid={!isQueryValid(filter.input, indexPattern)}
         value={filter.input}
         indexPatternTitle={indexPattern.title}
+        disableAutoFocus
         onChange={setFilterQuery}
         onSubmit={() => {
           if (inputRef.current) inputRef.current.focus();

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
@@ -264,7 +264,7 @@ export const FilterList = ({
                 setFilter={(f: FilterValue) => {
                   onChangeValue(f.id, f.input, f.label);
                 }}
-                Button={() => (
+                button={
                   <EuiLink
                     className="lnsFiltersOperation__popoverButton"
                     data-test-subj="indexPattern-filters-existingFilterTrigger"
@@ -276,7 +276,7 @@ export const FilterList = ({
                   >
                     {filter.label || filter.input.query || defaultLabel}
                   </EuiLink>
-                )}
+                }
               />
             </DraggableBucketContainer>
           );

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
@@ -7,7 +7,7 @@
 
 import './advanced_editor.scss';
 
-import React, { useState, MouseEventHandler, useEffect } from 'react';
+import React, { useState } from 'react';
 import { i18n } from '@kbn/i18n';
 import {
   EuiFlexGroup,
@@ -48,22 +48,19 @@ const getBetterLabel = (range: RangeTypeLens, formatter: IFieldFormat) =>
 export const RangePopover = ({
   range,
   setRange,
-  Button,
-  initiallyOpen,
+  button,
+  triggerClose,
+  isOpen,
 }: {
   range: LocalRangeType;
   setRange: (newRange: LocalRangeType) => void;
-  Button: React.FunctionComponent<{ onClick: MouseEventHandler }>;
-  initiallyOpen: boolean;
+  button: React.ReactChild;
+  triggerClose: () => void;
+  isOpen: 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 labelRef = React.useRef<HTMLInputElement>();
+  const toRef = React.useRef<HTMLInputElement>();
 
   const saveRangeAndReset = (newRange: LocalRangeType, resetRange = false) => {
     if (resetRange) {
@@ -91,25 +88,13 @@ export const RangePopover = ({
     defaultMessage: 'Less than',
   });
 
-  const onSubmit = () => {
-    if (isPopoverOpen) {
-      setIsPopoverOpen(false);
-    }
-  };
-
   return (
     <EuiPopover
       display="block"
       ownFocus
-      isOpen={isPopoverOpen}
-      closePopover={onSubmit}
-      button={
-        <Button
-          onClick={() => {
-            setIsPopoverOpen((isOpen) => !isOpen);
-          }}
-        />
-      }
+      isOpen={isOpen}
+      closePopover={() => triggerClose()}
+      button={button}
       data-test-subj="indexPattern-ranges-popover"
     >
       <EuiFormRow>
@@ -131,6 +116,11 @@ export const RangePopover = ({
                   <EuiText size="s">{lteAppendLabel}</EuiText>
                 </EuiToolTip>
               }
+              onKeyDown={({ key }: React.KeyboardEvent<HTMLInputElement>) => {
+                if (keys.ENTER === key && toRef.current) {
+                  toRef.current.focus();
+                }
+              }}
               compressed
               placeholder={FROM_PLACEHOLDER}
               isInvalid={!isValidRange(tempRange)}
@@ -144,6 +134,11 @@ export const RangePopover = ({
             <EuiFieldNumber
               className="lnsRangesOperation__popoverNumberField"
               value={isValidNumber(to) ? Number(to) : ''}
+              inputRef={(node) => {
+                if (toRef && node) {
+                  toRef.current = node;
+                }
+              }}
               onChange={({ target }) => {
                 const newRange = {
                   ...tempRange,
@@ -161,8 +156,8 @@ export const RangePopover = ({
               placeholder={TO_PLACEHOLDER}
               isInvalid={!isValidRange(tempRange)}
               onKeyDown={({ key }: React.KeyboardEvent<HTMLInputElement>) => {
-                if (keys.ENTER === key && onSubmit) {
-                  onSubmit();
+                if (keys.ENTER === key && labelRef.current) {
+                  labelRef.current.focus();
                 }
               }}
               step={1}
@@ -172,6 +167,7 @@ export const RangePopover = ({
       </EuiFormRow>
       <EuiFormRow>
         <LabelInput
+          inputRef={labelRef}
           value={label || ''}
           onChange={(newLabel) => {
             const newRange = {
@@ -185,7 +181,9 @@ export const RangePopover = ({
             'xpack.lens.indexPattern.ranges.customRangeLabelPlaceholder',
             { defaultMessage: 'Custom label' }
           )}
-          onSubmit={onSubmit}
+          onSubmit={() => {
+            triggerClose();
+          }}
           compressed
           dataTestSubj="indexPattern-ranges-label"
         />
@@ -205,15 +203,11 @@ export const AdvancedRangeEditor = ({
   onToggleEditor: () => void;
   formatter: IFieldFormat;
 }) => {
+  const [activeRangeId, setActiveRangeId] = useState('');
   // use a local state to store ids with range objects
   const [localRanges, setLocalRanges] = useState<LocalRangeType[]>(() =>
     ranges.map((range) => ({ ...range, id: generateId() }))
   );
-  // we need to force the open state of the popover from the outside in some scenarios
-  // so we need an extra state here
-  const [isOpenByCreation, setIsOpenByCreation] = useState(false);
-
-  const lastIndex = localRanges.length - 1;
 
   // Update locally all the time, but bounce the parents prop function to avoid too many requests
   // Avoid to trigger on first render
@@ -227,15 +221,27 @@ export const AdvancedRangeEditor = ({
   );
 
   const addNewRange = () => {
+    const newRangeId = generateId();
+
     setLocalRanges([
       ...localRanges,
       {
-        id: generateId(),
+        id: newRangeId,
         from: localRanges[localRanges.length - 1].to,
         to: Infinity,
         label: '',
       },
     ]);
+
+    setActiveRangeId(newRangeId);
+  };
+
+  const changeActiveRange = (rangeId: string) => {
+    let newActiveRangeId = rangeId;
+    if (activeRangeId === rangeId) {
+      newActiveRangeId = ''; // toggle off
+    }
+    setActiveRangeId(newActiveRangeId);
   };
 
   return (
@@ -281,7 +287,8 @@ export const AdvancedRangeEditor = ({
             >
               <RangePopover
                 range={range}
-                initiallyOpen={idx === lastIndex && isOpenByCreation}
+                isOpen={range.id === activeRangeId}
+                triggerClose={() => changeActiveRange('')}
                 setRange={(newRange: LocalRangeType) => {
                   const newRanges = [...localRanges];
                   if (newRange.id === newRanges[idx].id) {
@@ -291,10 +298,10 @@ export const AdvancedRangeEditor = ({
                   }
                   setLocalRanges(newRanges);
                 }}
-                Button={({ onClick }: { onClick: MouseEventHandler }) => (
+                button={
                   <EuiLink
                     color="text"
-                    onClick={onClick}
+                    onClick={() => changeActiveRange(range.id)}
                     className="lnsRangesOperation__popoverButton"
                     data-test-subj="indexPattern-ranges-popover-trigger"
                   >
@@ -306,15 +313,14 @@ export const AdvancedRangeEditor = ({
                       {getBetterLabel(range, formatter)}
                     </EuiText>
                   </EuiLink>
-                )}
+                }
               />
             </DraggableBucketContainer>
           ))}
         </DragDropBuckets>
         <NewBucketButton
           onClick={() => {
             addNewRange();
-            setIsOpenByCreation(true);
           }}
           label={i18n.translate('xpack.lens.indexPattern.ranges.addRange', {
             defaultMessage: 'Add range',

diff --git a/...s/public/indexpattern_datasource/operations/definitions/shared_components/label_input.tsx b/...s/public/indexpattern_datasource/operations/definitions/shared_components/label_input.tsx
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import React from 'react';
+import React, { useRef } from 'react';
 import { EuiFieldText, keys } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { useDebouncedValue } from '../../../../shared_components';
@@ -28,6 +28,7 @@ export const LabelInput = ({
   compressed?: boolean;
 }) => {
   const { inputValue, handleInputChange } = useDebouncedValue({ value, onChange });
+  const localKeyHold = useRef(false);
 
   return (
     <EuiFieldText
@@ -41,10 +42,15 @@ export const LabelInput = ({
           inputRef.current = node;
         }
       }}
+      onKeyDown={() => {
+        localKeyHold.current = true;
+      }}
       onKeyUp={({ key }: React.KeyboardEvent<HTMLInputElement>) => {
-        if (keys.ENTER === key && onSubmit) {
+        // only submit on key up in case the user started the keypress while the input was focused
+        if (localKeyHold.current && keys.ENTER === key && onSubmit) {
           onSubmit();
         }
+        localKeyHold.current = false;
       }}
       prepend={i18n.translate('xpack.lens.labelInput.label', {
         defaultMessage: 'Label',