Prevent menu flicker when closing ComboBox via blur (#4060)

* Revert "Prevent ComboBox item selection if menu is closing  (#3999)"

This reverts commit 8c61397.

* Prevent flicker when menu is closed via clicking outside the popover

* move from ref to state tracker for filtered collection

also memos useFilter return so that filteredCollection isnt updated every single render

* adding controlled test

* replace useEffect in favor of updating the lastCollection just before close

* Only updating lastCollection if we are going from open to close

* fixing tests

Author: LFDanLu

packages/@react-aria/i18n/src/useFilter.ts

@@ -10,6 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
+import {useCallback, useMemo} from 'react';
 import {useCollator} from './useCollator';
 
 export interface Filter {
@@ -32,46 +33,51 @@ export function useFilter(options?: Intl.CollatorOptions): Filter {
   });
 
   // TODO(later): these methods don't currently support the ignorePunctuation option.
+  let startsWith = useCallback((string, substring) => {
+    if (substring.length === 0) {
+      return true;
+    }
 
-  return {
-    startsWith(string, substring) {
-      if (substring.length === 0) {
-        return true;
-      }
+    // Normalize both strings so we can slice safely
+    // TODO: take into account the ignorePunctuation option as well...
+    string = string.normalize('NFC');
+    substring = substring.normalize('NFC');
+    return collator.compare(string.slice(0, substring.length), substring) === 0;
+  }, [collator]);
 
-      // Normalize both strings so we can slice safely
-      // TODO: take into account the ignorePunctuation option as well...
-      string = string.normalize('NFC');
-      substring = substring.normalize('NFC');
-      return collator.compare(string.slice(0, substring.length), substring) === 0;
-    },
-    endsWith(string, substring) {
-      if (substring.length === 0) {
-        return true;
-      }
+  let endsWith = useCallback((string, substring) => {
+    if (substring.length === 0) {
+      return true;
+    }
 
-      string = string.normalize('NFC');
-      substring = substring.normalize('NFC');
-      return collator.compare(string.slice(-substring.length), substring) === 0;
-    },
-    contains(string, substring) {
-      if (substring.length === 0) {
-        return true;
-      }
+    string = string.normalize('NFC');
+    substring = substring.normalize('NFC');
+    return collator.compare(string.slice(-substring.length), substring) === 0;
+  }, [collator]);
 
-      string = string.normalize('NFC');
-      substring = substring.normalize('NFC');
+  let contains = useCallback((string, substring) => {
+    if (substring.length === 0) {
+      return true;
+    }
 
-      let scan = 0;
-      let sliceLen = substring.length;
-      for (; scan + sliceLen <= string.length; scan++) {
-        let slice = string.slice(scan, scan + sliceLen);
-        if (collator.compare(substring, slice) === 0) {
-          return true;
-        }
-      }
+    string = string.normalize('NFC');
+    substring = substring.normalize('NFC');
 
-      return false;
+    let scan = 0;
+    let sliceLen = substring.length;
+    for (; scan + sliceLen <= string.length; scan++) {
+      let slice = string.slice(scan, scan + sliceLen);
+      if (collator.compare(substring, slice) === 0) {
+        return true;
+      }
     }
-  };
+
+    return false;
+  }, [collator]);
+
+  return useMemo(() => ({
+    startsWith,
+    endsWith,
+    contains
+  }), [startsWith, endsWith, contains]);
 }

packages/@react-spectrum/autocomplete/test/SearchAutocomplete.test.js

@@ -873,6 +873,11 @@ describe('SearchAutocomplete', function () {
       act(() => {
         fireEvent.change(searchAutocomplete, {target: {value: 'Bulba'}});
         jest.runAllTimers();
+
+      });
+      expect(onOpenChange).toHaveBeenLastCalledWith(true, 'input');
+
+      act(() => {
         searchAutocomplete.blur();
         jest.runAllTimers();
       });

packages/@react-spectrum/combobox/test/ComboBox.test.js

@@ -1532,9 +1532,14 @@ describe('ComboBox', function () {
         userEvent.click(combobox);
         jest.runAllTimers();
       });
+
       act(() => {
         fireEvent.change(combobox, {target: {value: 'Bulba'}});
         jest.runAllTimers();
+      });
+      expect(onOpenChange).toHaveBeenLastCalledWith(true, 'input');
+
+      act(() => {
         combobox.blur();
         jest.runAllTimers();
       });

packages/@react-stately/combobox/src/useComboBoxState.ts

@@ -14,8 +14,8 @@ import {Collection, FocusStrategy, Node} from '@react-types/shared';
 import {ComboBoxProps, MenuTriggerAction} from '@react-types/combobox';
 import {ListCollection, useSingleSelectListState} from '@react-stately/list';
 import {SelectState} from '@react-stately/select';
+import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {useControlledState} from '@react-stately/utils';
-import {useEffect, useMemo, useRef, useState} from 'react';
 import {useMenuTriggerState} from '@react-stately/menu';
 
 export interface ComboBoxState<T> extends SelectState<T> {
@@ -75,7 +75,7 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
     // (scenario: user clicks on already selected option)
     if (key === selectedKey) {
       resetInputValue();
-      triggerState.close();
+      closeMenu();
     }
   };
 
@@ -93,6 +93,7 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
       ? collection
       : filterCollection(collection, inputValue, defaultFilter)
   ), [collection, inputValue, defaultFilter, props.items]);
+  let [lastCollection, setLastCollection] = useState(filteredCollection);
 
   // Track what action is attempting to open the menu
   let menuOpenTrigger = useRef('focus' as MenuTriggerAction);
@@ -136,9 +137,26 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
       menuOpenTrigger.current = trigger;
     }
 
-    triggerState.toggle(focusStrategy);
+    toggleMenu(focusStrategy);
   };
 
+  // If menu is going to close, save the current collection so we can freeze the displayed collection when the
+  // user clicks outside the popover to close the menu. Prevents the menu contents from updating as the menu closes.
+  let toggleMenu = useCallback((focusStrategy) => {
+    if (triggerState.isOpen) {
+      setLastCollection(filteredCollection);
+    }
+
+    triggerState.toggle(focusStrategy);
+  }, [triggerState, filteredCollection]);
+
+  let closeMenu = useCallback(() => {
+    if (triggerState.isOpen) {
+      setLastCollection(filteredCollection);
+      triggerState.close();
+    }
+  }, [triggerState, filteredCollection]);
+
   let lastValue = useRef(inputValue);
   let resetInputValue = () => {
     let itemText = collection.getItem(selectedKey)?.textValue ?? '';
@@ -172,15 +190,15 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
       triggerState.isOpen &&
       filteredCollection.size === 0
     ) {
-      triggerState.close();
+      closeMenu();
     }
 
     // Close when an item is selected.
     if (
       selectedKey != null &&
       selectedKey !== lastSelectedKey.current
     ) {
-      triggerState.close();
+      closeMenu();
     }
 
     // Clear focused key when input value changes and display filtered collection again.
@@ -248,7 +266,7 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
   let commitCustomValue = () => {
     lastSelectedKey.current = null;
     setSelectedKey(null);
-    triggerState.close();
+    closeMenu();
   };
 
   let commitSelection = () => {
@@ -259,11 +277,11 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
       // Stop menu from reopening from useEffect
       let itemText = collection.getItem(selectedKey)?.textValue ?? '';
       lastValue.current = itemText;
-      triggerState.close();
+      closeMenu();
     } else {
       // If only a single aspect of combobox is controlled, reset input value and close menu for the user
       resetInputValue();
-      triggerState.close();
+      closeMenu();
     }
   };
 
@@ -291,7 +309,7 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
     } else {
       commitSelection();
     }
-    triggerState.close();
+    closeMenu();
   };
 
   let setFocused = (isFocused: boolean) => {
@@ -306,6 +324,18 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
     setFocusedState(isFocused);
   };
 
+  let displayedCollection = useMemo(() => {
+    if (triggerState.isOpen) {
+      if (showAllItems) {
+        return originalCollection;
+      } else {
+        return filteredCollection;
+      }
+    } else {
+      return lastCollection;
+    }
+  }, [triggerState.isOpen, originalCollection, filteredCollection, showAllItems, lastCollection]);
+
   return {
     ...triggerState,
     toggle,
@@ -318,7 +348,7 @@ export function useComboBoxState<T extends object>(props: ComboBoxStateOptions<T
     isFocused,
     setFocused,
     selectedItem,
-    collection: showAllItems ? originalCollection : filteredCollection,
+    collection: displayedCollection,
     inputValue,
     setInputValue,
     commit,

packages/@react-stately/combobox/test/useComboBoxState.test.js

@@ -14,6 +14,7 @@ import {actHook as act, renderHook} from '@react-spectrum/test-utils';
 import {Item} from '@react-stately/collections';
 import React from 'react';
 import {useComboBoxState} from '../';
+import {useFilter} from 'react-aria';
 
 describe('useComboBoxState tests', function () {
   describe('open state', function () {
@@ -196,7 +197,7 @@ describe('useComboBoxState tests', function () {
       expect(onSelectionChange).toHaveBeenCalledWith('1');
     });
 
-    it('supports sdefault no selection', function () {
+    it('supports default no selection', function () {
       let initialProps = {...defaultProps};
       let {result} = renderHook((props) => useComboBoxState(props), {initialProps});
       expect(result.current.selectionManager.selectionMode).toBe('single');
@@ -207,5 +208,84 @@ describe('useComboBoxState tests', function () {
       expect(result.current.selectionManager.selectedKeys).not.toContain('0');
       expect(onSelectionChange).toHaveBeenCalledWith('1');
     });
+
+    it('won\'t update the returned collection if the combobox is closed (uncontrolled items)', function () {
+      let filter = renderHook((props) => useFilter(props), {sensitivity: 'base'});
+      let initialProps = {...defaultProps, items: null, defaultItems: [{key: '0', name: 'one'}, {key: '1', name: 'onomatopoeia'}], defaultInputValue: '', defaultFilter: filter.result.current.contains};
+      let {result} = renderHook((props) => useComboBoxState(props), {initialProps});
+      expect(result.current.collection.size).toEqual(2);
+      expect(result.current.inputValue).toBe('');
+
+      act(() => {result.current.open();});
+      act(() => result.current.setInputValue('onom'));
+      expect(result.current.inputValue).toBe('onom');
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      // The input value updates, but the returned collection still only contains onomatopoeia
+      act(() => {result.current.setFocused(false);});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      // Subsequent calls that would close the menu don't update the tracked lastCollection
+      act(() => {result.current.commit();});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      act(() => {result.current.close();});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      act(() => {result.current.revert();});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      act(() => {result.current.open();});
+      expect(result.current.collection.size).toEqual(2);
+    });
+
+    it('won\'t update the returned collection if the combobox is closed (controlled items)', function () {
+      let initialProps = {...defaultProps};
+      let {result, rerender} = renderHook((props) => useComboBoxState(props), {initialProps});
+      expect(result.current.collection.size).toEqual(2);
+
+      act(() => {result.current.open();});
+      rerender({...initialProps, items: [{key: '1', name: 'onomatopoeia'}]});
+      // Returned collection reflects the items provided by the user since the combobox is open
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      act(() => {result.current.setFocused(false);});
+      // Returned collection reflects the old items provided by the user when the combobox is closed
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+      rerender(initialProps);
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      // Subsequent calls that would close the menu don't update the tracked lastCollection
+      act(() => {result.current.commit();});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      act(() => {result.current.close();});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      act(() => {result.current.revert();});
+      expect(result.current.collection.size).toEqual(1);
+      expect(result.current.inputValue).toBe('');
+      expect(result.current.collection.getItem('1').rendered).toBe('onomatopoeia');
+
+      // When the combobox is opened again, the returned collection of items updates to reflect the items provided by the user
+      act(() => {result.current.open();});
+      expect(result.current.collection.size).toEqual(2);
+    });
   });
 });