[Autocomplete] Better synchronize the highlight with the value (mui#1…

…9923)
EsoterikStare · Mar 30, 2020 · fb17e69 · fb17e69
1 parent e1c123b
commit fb17e69
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 7 deletions.
diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js
@@ -76,6 +76,50 @@ describe('<Autocomplete />', () => {
       fireEvent.change(document.activeElement, { target: { value: 'o' } });
       checkHighlightIs('one');
     });
+
+    it('should set the focus on selected item when dropdown is expanded', () => {
+      const { getByRole, setProps } = render(
+        <Autocomplete
+          {...defaultProps}
+          value="one"
+          options={['one', 'two', 'three']}
+          renderInput={params => <TextField autoFocus {...params} />}
+        />,
+      );
+
+      function checkHighlightIs(expected) {
+        expect(getByRole('listbox').querySelector('li[data-focus]')).to.have.text(expected);
+      }
+
+      checkHighlightIs('one');
+      setProps({ value: 'two' });
+      checkHighlightIs('two');
+    });
+  });
+
+  describe('prop: filterSelectedOptions', () => {
+    it('when the last item is selected, highlights the new last item', () => {
+      const { getByRole } = render(
+        <Autocomplete
+          {...defaultProps}
+          filterSelectedOptions
+          options={['one', 'two', 'three']}
+          renderInput={params => <TextField {...params} autoFocus />}
+        />,
+      );
+
+      function checkHighlightIs(expected) {
+        expect(getByRole('listbox').querySelector('li[data-focus]')).to.have.text(expected);
+      }
+
+      fireEvent.keyDown(document.activeElement, { key: 'ArrowUp' });
+      checkHighlightIs('three');
+      fireEvent.keyDown(document.activeElement, { key: 'Enter' }); // selects the last option
+      const input = getByRole('textbox');
+      input.blur();
+      input.focus(); // opens the listbox again
+      checkHighlightIs('two');
+    });
   });
 
   describe('prop: autoSelect', () => {
@@ -758,7 +802,6 @@ describe('<Autocomplete />', () => {
         />,
       );
 
-      fireEvent.keyDown(document.activeElement, { key: 'ArrowDown' });
       fireEvent.keyDown(document.activeElement, { key: 'ArrowDown' });
       fireEvent.keyDown(document.activeElement, { key: 'Enter' });
 

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -133,7 +133,7 @@ export default function useAutocomplete(props) {
   const defaultHighlighted = autoHighlight ? 0 : -1;
   const highlightedIndexRef = React.useRef(defaultHighlighted);
 
-  function setHighlightedIndex(index, mouse = false) {
+  const setHighlightedIndex = useEventCallback((index, mouse = false) => {
     highlightedIndexRef.current = index;
     // does the index exist?
     if (index === -1) {
@@ -190,7 +190,7 @@ export default function useAutocomplete(props) {
         listboxNode.scrollTop = element.offsetTop - element.offsetHeight * (groupBy ? 1.3 : 0);
       }
     }
-  }
+  });
 
   const [value, setValue] = useControlled({
     controlled: valueProp,
@@ -323,7 +323,7 @@ export default function useAutocomplete(props) {
     }
   }
 
-  const changeHighlightedIndex = (diff, direction) => {
+  const changeHighlightedIndex = useEventCallback((diff, direction) => {
     if (!popupOpen) {
       return;
     }
@@ -390,11 +390,47 @@ export default function useAutocomplete(props) {
         }
       }
     }
-  };
+  });
 
   React.useEffect(() => {
-    changeHighlightedIndex('reset', 'next');
-  }, [inputValue]); // eslint-disable-line react-hooks/exhaustive-deps
+    if (!open) {
+      return;
+    }
+
+    const valueItem = multiple ? value[0] : value;
+
+    // The popup is empty
+    if (filteredOptions.length === 0 || valueItem == null) {
+      changeHighlightedIndex('reset', 'next');
+      return;
+    }
+
+    // Synchronize the value with the highlighted index
+    if (!filterSelectedOptions && valueItem != null) {
+      const itemIndex = findIndex(filteredOptions, optionItem =>
+        getOptionSelected(optionItem, valueItem),
+      );
+
+      setHighlightedIndex(itemIndex);
+      return;
+    }
+
+    // Keep the index in the boundaries
+    if (highlightedIndexRef.current >= filteredOptions.length - 1) {
+      setHighlightedIndex(filteredOptions.length - 1);
+    }
+
+    // Ignore filterOptions => options, getOptionSelected, getOptionLabel)
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [
+    value,
+    open,
+    filterSelectedOptions,
+    changeHighlightedIndex,
+    setHighlightedIndex,
+    inputValue,
+    multiple,
+  ]);
 
   const handleOpen = event => {
     if (open) {