Merge branch 'autocomplete_audit_for_release' of github.com:adobe/react-spectrum into subdialogs

Author: LFDanLu

packages/@react-aria/autocomplete/src/useAutocomplete.ts

@@ -67,17 +67,24 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
   let collectionId = useId();
   let timeout = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
   let delayNextActiveDescendant = useRef(false);
+  let queuedActiveDescendant = useRef(null);
   let lastCollectionNode = useRef<HTMLElement>(null);
 
   let updateActiveDescendant = useEffectEvent((e) => {
     let {target} = e;
+    if (queuedActiveDescendant.current === target.id) {
+      return;
+    }
+
     clearTimeout(timeout.current);
     e.stopPropagation();
 
     if (target !== collectionRef.current) {
       if (delayNextActiveDescendant.current) {
+        queuedActiveDescendant.current = target.id;
         timeout.current = setTimeout(() => {
           state.setFocusedNodeId(target.id);
+          queuedActiveDescendant.current = null;
         }, 500);
       } else {
         state.setFocusedNodeId(target.id);
@@ -159,7 +166,12 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
         // Early return for Escape here so it doesn't leak the Escape event from the simulated collection event below and
         // close the dialog prematurely. Ideally that should be up to the discretion of the input element hence the check
         // for isPropagationStopped
+        // Also set the inputValue to '' to cover Firefox case where Esc doesn't actually clear searchfields. Normally we already
+        // handle this in useSearchField, but we are directly setting the inputValue on the input element in RAC Autocomplete instead of
+        // passing it to the SearchField via props. This means that a controlled value set on the Autocomplete isn't synced up with the
+        // SearchField until the user makes a change to the field's value via typing
         if (e.isPropagationStopped()) {
+          state.setInputValue('');
           return;
         }
         break;

packages/@react-aria/searchfield/src/useSearchField.ts

@@ -73,7 +73,9 @@ export function useSearchField(
     }
 
     if (key === 'Escape') {
-      if (state.value === '') {
+      // Also check the inputRef value for the case where the value was set directly on the input element instead of going through
+      // the hook
+      if (state.value === '' && (!inputRef.current || inputRef.current.value === '')) {
         e.continuePropagation();
       } else {
         state.setValue('');

packages/@react-aria/selection/src/useSelectableCollection.ts

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, UPDATE_ACTIVEDESCENDANT, useEffectEvent, useEvent, useRouter, useUpdateEffect} from '@react-aria/utils';
+import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, UPDATE_ACTIVEDESCENDANT, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
 import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared';
 import {flushSync} from 'react-dom';
 import {FocusEvent, KeyboardEvent, useEffect, useRef} from 'react';
@@ -429,7 +429,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
     }
   });
 
-  useUpdateEffect(() => {
+  useUpdateLayoutEffect(() => {
     if (shouldVirtualFocusFirst.current) {
       updateActiveDescendant();
     }
@@ -445,7 +445,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
     }
   });
 
-  useUpdateEffect(() => {
+  useUpdateLayoutEffect(() => {
     resetFocusFirstFlag();
   }, [manager.focusedKey, resetFocusFirstFlag]);
 

packages/@react-aria/selection/src/useSelectableItem.ts

@@ -160,6 +160,9 @@ export function useSelectableItem(options: SelectableItemOptions): SelectableIte
   };
 
   // Focus the associated DOM node when this item becomes the focusedKey
+  // TODO: can't make this useLayoutEffect bacause it breaks menus inside dialogs
+  // However, if this is a useEffect, it runs twice and dispatches two UPDATE_ACTIVEDESCENDANT and immediately sets
+  // aria-activeDescendant in useAutocomplete... I've worked around this for now
   useEffect(() => {
     let isFocused = key === manager.focusedKey;
     if (isFocused && manager.isFocused) {

packages/@react-aria/utils/src/index.ts

@@ -24,6 +24,7 @@ export {useGlobalListeners} from './useGlobalListeners';
 export {useLabels} from './useLabels';
 export {useObjectRef} from './useObjectRef';
 export {useUpdateEffect} from './useUpdateEffect';
+export {useUpdateLayoutEffect} from './useUpdateLayoutEffect';
 export {useLayoutEffect} from './useLayoutEffect';
 export {useResizeObserver} from './useResizeObserver';
 export {useSyncRef} from './useSyncRef';

packages/@react-aria/utils/src/useUpdateLayoutEffect.ts

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2020 Adobe. All rights reserved.
+ * This file is licensed to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
+ * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+ * OF ANY KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+
+import {EffectCallback, useRef} from 'react';
+import {useLayoutEffect} from './useLayoutEffect';
+
+// Like useLayoutEffect, but only called for updates after the initial render.
+export function useUpdateLayoutEffect(effect: EffectCallback, dependencies: any[]) {
+  const isInitialMount = useRef(true);
+  const lastDeps = useRef<any[] | null>(null);
+
+  useLayoutEffect(() => {
+    isInitialMount.current = true;
+    return () => {
+      isInitialMount.current = false;
+    };
+  }, []);
+
+  useLayoutEffect(() => {
+    if (isInitialMount.current) {
+      isInitialMount.current = false;
+    } else if (!lastDeps.current || dependencies.some((dep, i) => !Object.is(dep, lastDeps[i]))) {
+      effect();
+    }
+    lastDeps.current = dependencies;
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, dependencies);
+}

packages/react-aria-components/stories/Autocomplete.stories.tsx

@@ -410,7 +410,7 @@ for (let i = 0; i < 50; i++) {
     //     ]
     //   });
     // } else {
-      children.push({name: `Section ${i}, Item ${j}`, id: `item_${i}${j}`});
+      children.push({name: `Section ${i}, Item ${j}`, id: `item_${i}_${j}`});
     // }
   }
 
@@ -534,6 +534,11 @@ export const AutocompleteInPopover = {
         disable: true
       }
     }
+  },
+  parameters: {
+    description: {
+      data: 'Menu is single selection so only the latest selected option will show the selected style'
+    }
   }
 };
 
@@ -578,5 +583,10 @@ export const AutocompleteInPopoverDialogTrigger = {
         disable: true
       }
     }
+  },
+  parameters: {
+    description: {
+      data: 'Menu is single selection so only the latest selected option will show the selected style'
+    }
   }
 };