feat: Automatically detect if wrapped Autocomplete collection supports virtual focus

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

@@ -13,12 +13,12 @@
 import {AriaLabelingProps, BaseEvent, DOMProps, FocusableElement, FocusEvents, KeyboardEvents, Node, RefObject, ValueBase} from '@react-types/shared';
 import {AriaTextFieldProps} from '@react-aria/textfield';
 import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete';
-import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isAndroid, isCtrlKeyPressed, isIOS, mergeProps, mergeRefs, useEffectEvent, useEvent, useLabels, useObjectRef, useSlotId} from '@react-aria/utils';
+import {CLEAR_FOCUS_EVENT, DISALLOW_VIRTUAL_FOCUS, FOCUS_EVENT, getActiveElement, getOwnerDocument, isAndroid, isCtrlKeyPressed, isIOS, mergeProps, mergeRefs, useEffectEvent, useEvent, useId, useLabels, useObjectRef} from '@react-aria/utils';
 import {dispatchVirtualBlur, dispatchVirtualFocus, getVirtuallyFocusedElement, moveVirtualFocus} from '@react-aria/focus';
 import {getInteractionModality} from '@react-aria/interactions';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
-import {FocusEvent as ReactFocusEvent, KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef} from 'react';
+import {FocusEvent as ReactFocusEvent, KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {useLocalizedStringFormatter} from '@react-aria/i18n';
 
 export interface CollectionOptions extends DOMProps, AriaLabelingProps {
@@ -88,7 +88,7 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     disableVirtualFocus = false
   } = props;
 
-  let collectionId = useSlotId();
+  let collectionId = useId();
   let timeout = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
   let delayNextActiveDescendant = useRef(false);
   let queuedActiveDescendant = useRef<string | null>(null);
@@ -97,7 +97,11 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
   // For mobile screen readers, we don't want virtual focus, instead opting to disable FocusScope's restoreFocus and manually
   // moving focus back to the subtriggers
   let isMobileScreenReader = getInteractionModality() === 'virtual' && (isIOS() || isAndroid());
-  let shouldUseVirtualFocus = !isMobileScreenReader && !disableVirtualFocus;
+  let [shouldUseVirtualFocus, setShouldUseVirtualFocus] = useState(!isMobileScreenReader && !disableVirtualFocus);
+  // Tracks if a collection has been connected to the autocomplete. If false, we don't want to add various attributes to the autocomplete input
+  // since it isn't attached to a filterable collection (e.g. Tabs)
+  let [hasCollection, setHasCollection] = useState(false);
+
   useEffect(() => {
     return () => clearTimeout(timeout.current);
   }, []);
@@ -136,6 +140,10 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     delayNextActiveDescendant.current = false;
   });
 
+  let turnOffVirtualFocus = useCallback(() => {
+    setShouldUseVirtualFocus(false);
+  }, []);
+
   let callbackRef = useCallback((collectionNode) => {
     if (collectionNode != null) {
       // When typing forward, we want to delay the setting of active descendant to not interrupt the native screen reader announcement
@@ -145,10 +153,17 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
       lastCollectionNode.current?.removeEventListener('focusin', updateActiveDescendant);
       lastCollectionNode.current = collectionNode;
       collectionNode.addEventListener('focusin', updateActiveDescendant);
+      // If useSelectableCollection isn't passed shouldUseVirtualFocus even when useAutocomplete provides it
+      // that means the collection doesn't support it (e.g. Table). If that is the case, we need to disable it here regardless
+      // of what the user's provided so that the input doesn't recieve the onKeyDown and autocomplete props.
+      collectionNode.addEventListener(DISALLOW_VIRTUAL_FOCUS, turnOffVirtualFocus);
+      setHasCollection(true);
     } else {
       lastCollectionNode.current?.removeEventListener('focusin', updateActiveDescendant);
+      lastCollectionNode.current?.removeEventListener(DISALLOW_VIRTUAL_FOCUS, turnOffVirtualFocus);
+      setHasCollection(false);
     }
-  }, [updateActiveDescendant]);
+  }, [updateActiveDescendant, turnOffVirtualFocus]);
 
   // Make sure to memo so that React doesn't keep registering a new event listeners on every rerender of the wrapped collection
   let mergedCollectionRef = useObjectRef(useMemo(() => mergeRefs(collectionRef, callbackRef), [collectionRef, callbackRef]));
@@ -393,7 +408,7 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     onFocus
   };
 
-  if (collectionId) {
+  if (hasCollection) {
     inputProps = {
       ...inputProps,
       ...(shouldUseVirtualFocus && virtualFocusProps),
@@ -413,7 +428,7 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     inputProps,
     collectionProps: mergeProps(collectionProps, {
       shouldUseVirtualFocus,
-      disallowTypeAhead: true
+      disallowTypeAhead: shouldUseVirtualFocus
     }),
     collectionRef: mergedCollectionRef,
     filter: filter != null ? filterFn : undefined

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, getActiveElement, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
+import {CLEAR_FOCUS_EVENT, DISALLOW_VIRTUAL_FOCUS, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils';
 import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus';
 import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared';
 import {flushSync} from 'react-dom';
@@ -396,6 +396,17 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
     }
   };
 
+  useEffect(() => {
+    if (!shouldUseVirtualFocus) {
+      let disableVirtualFocus = new CustomEvent(DISALLOW_VIRTUAL_FOCUS, {
+        cancelable: true,
+        bubbles: true
+      });
+
+      ref.current?.dispatchEvent(disableVirtualFocus);
+    }
+  }, [shouldUseVirtualFocus, ref]);
+
   // Ref to track whether the first item in the collection should be automatically focused. Specifically used for autocomplete when user types
   // to focus the first key AFTER the collection updates.
   // TODO: potentially expand the usage of this

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

@@ -13,3 +13,5 @@
 // Custom event names for updating the autocomplete's aria-activedecendant.
 export const CLEAR_FOCUS_EVENT = 'react-aria-clear-focus';
 export const FOCUS_EVENT = 'react-aria-focus';
+// Custom event to tell autocomplete that virtual focus isn't supported for this component (e.g. Table)
+export const DISALLOW_VIRTUAL_FOCUS = 'react-aria-disallow-virtual-focus';

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

@@ -47,7 +47,7 @@ export {useFormReset} from './useFormReset';
 export {useLoadMore} from './useLoadMore';
 export {useLoadMoreSentinel, useLoadMoreSentinel as UNSTABLE_useLoadMoreSentinel} from './useLoadMoreSentinel';
 export {inertValue} from './inertValue';
-export {CLEAR_FOCUS_EVENT, FOCUS_EVENT} from './constants';
+export {CLEAR_FOCUS_EVENT, DISALLOW_VIRTUAL_FOCUS, FOCUS_EVENT} from './constants';
 export {isCtrlKeyPressed} from './keyboard';
 export {useEnterAnimation, useExitAnimation} from './animation';
 export {isFocusable, isTabbable} from './isFocusable';

packages/@react-spectrum/s2/src/ComboBox.tsx

@@ -59,7 +59,7 @@ import intlMessages from '../intl/*.json';
 import {mergeRefs, useResizeObserver, useSlotId} from '@react-aria/utils';
 import {Node} from 'react-stately';
 import {Placement} from 'react-aria';
-import {PopoverBase} from './Popover';
+import {Popover} from './Popover';
 import {pressScale} from './pressScale';
 import {ProgressCircle} from './ProgressCircle';
 import {TextFieldRef} from '@react-types/textfield';
@@ -643,7 +643,7 @@ const ComboboxInner = forwardRef(function ComboboxInner(props: ComboBoxProps<any
           description={descriptionMessage}>
           {errorMessage}
         </HelpText>
-        <PopoverBase
+        <Popover
           hideArrow
           triggerRef={triggerRef}
           offset={menuOffset}
@@ -652,7 +652,11 @@ const ComboboxInner = forwardRef(function ComboboxInner(props: ComboBoxProps<any
           UNSAFE_style={{
             width: menuWidth ? `${menuWidth}px` : undefined,
             // manually subtract border as we can't set Popover to border-box, it causes the contents to spill out
-            '--trigger-width': `calc(${triggerWidth} - 2px)`
+            '--trigger-width': `calc(${triggerWidth} - 2px)`,
+            // TODO: Unfortunately can't override via styles prop
+            // need to unset the overflow otherwise we get two scroll bars
+            padding: 0,
+            overflow: 'unset'
           } as CSSProperties}
           styles={style({
             minWidth: '--trigger-width',
@@ -693,7 +697,7 @@ const ComboboxInner = forwardRef(function ComboboxInner(props: ComboBoxProps<any
               </ListBox>
             </Virtualizer>
           </Provider>
-        </PopoverBase>
+        </Popover>
       </InternalComboboxContext.Provider>
     </>
   );

packages/@react-spectrum/s2/src/ContextualHelp.tsx

@@ -103,6 +103,8 @@ export const ContextualHelp = forwardRef(function ContextualHelp(props: Contextu
         offset={offset}
         crossOffset={crossOffset}
         hideArrow
+        // TODO: Unfortunately, we can't pass these styles via the styles prop
+        // since we don't want to actually allow modifying width and padding for the popover...
         styles={popover}>
         <RACDialog className={mergeStyles(dialogInner, style({borderRadius: 'none', margin: 'calc(self(paddingTop) * -1)', padding: 24}))}>
           <Provider

packages/@react-spectrum/s2/src/DatePicker.tsx

@@ -239,6 +239,8 @@ export function CalendarPopover(props: PropsWithChildren): ReactElement {
   return (
     <PopoverBase
       hideArrow
+      // TODO: another case where the below styles aren't allowed as overrides via styles
+      // and thus we can't get away with replacing it with Popover...
       styles={style({
         paddingX: 16,
         paddingY: 32,

packages/@react-spectrum/s2/src/Menu.tsx

@@ -41,7 +41,7 @@ import {forwardRefType} from './types';
 import {HeaderContext, HeadingContext, KeyboardContext, Text, TextContext} from './Content';
 import {IconContext} from './Icon'; // chevron right removed??
 import {ImageContext} from './Image';
-import {InPopoverContext, PopoverBase, PopoverContext} from './Popover';
+import {InPopoverContext, Popover, PopoverContext} from './Popover';
 import LinkOutIcon from '../ui-icons/LinkOut';
 import {mergeStyles} from '../style/runtime';
 import {Placement, useLocale} from 'react-aria';
@@ -366,14 +366,19 @@ export const Menu = /*#__PURE__*/ (forwardRef as forwardRefType)(function Menu<T
 
   if (isPopover) {
     return (
-      <PopoverBase
+      <Popover
         ref={ref}
         hideArrow
-        UNSAFE_style={UNSAFE_style}
+        UNSAFE_style={{
+          ...UNSAFE_style,
+          // TODO: similar to Combobox, can't override via styles props
+          padding: 0,
+          overflow: 'unset'
+        }}
         UNSAFE_className={UNSAFE_className}
         styles={styles}>
         {content}
-      </PopoverBase>
+      </Popover>
     );
   }
 

packages/@react-spectrum/s2/src/Picker.tsx

@@ -69,7 +69,7 @@ import {IconContext} from './Icon';
 import intlMessages from '../intl/*.json';
 import {mergeStyles} from '../style/runtime';
 import {Placement} from 'react-aria';
-import {PopoverBase} from './Popover';
+import {Popover} from './Popover';
 import {PressResponder} from '@react-aria/interactions';
 import {pressScale} from './pressScale';
 import {ProgressCircle} from './ProgressCircle';
@@ -394,13 +394,16 @@ export const Picker = /*#__PURE__*/ (forwardRef as forwardRefType)(function Pick
                 estimatedHeadingHeight: 50,
                 padding: 8,
                 loaderHeight: LOADER_ROW_HEIGHTS[size][scale]}}>
-              <PopoverBase
+              <Popover
                 hideArrow
                 offset={menuOffset}
                 placement={`${direction} ${align}` as Placement}
                 shouldFlip={shouldFlip}
                 UNSAFE_style={{
-                  width: menuWidth && !isQuiet ? `${menuWidth}px` : undefined
+                  width: menuWidth && !isQuiet ? `${menuWidth}px` : undefined,
+                  // TODO: similar to Combobox, can't override via styles props
+                  padding: 0,
+                  overflow: 'unset'
                 }}
                 styles={style({
                   marginStart: {
@@ -436,7 +439,7 @@ export const Picker = /*#__PURE__*/ (forwardRef as forwardRefType)(function Pick
                     {renderer}
                   </ListBox>
                 </Provider>
-              </PopoverBase>
+              </Popover>
             </Virtualizer>
           </InternalPickerContext.Provider>
         </>

packages/@react-spectrum/s2/src/Popover.tsx

@@ -15,16 +15,16 @@ import {
   PopoverProps as AriaPopoverProps,
   composeRenderProps,
   ContextValue,
-  Dialog,
   DialogProps,
   OverlayArrow,
   OverlayTriggerStateContext,
   useLocale
 } from 'react-aria-components';
 import {colorScheme, getAllowedOverrides, StyleProps, UnsafeStyles} from './style-utils' with {type: 'macro'};
 import {ColorSchemeContext} from './Provider';
-import {createContext, forwardRef, MutableRefObject, useCallback, useContext} from 'react';
+import {createContext, ForwardedRef, forwardRef, useCallback, useContext, useMemo} from 'react';
 import {DOMRef, DOMRefValue, GlobalDOMAttributes} from '@react-types/shared';
+import {mergeRefs} from '@react-aria/utils';
 import {mergeStyles} from '../style/runtime';
 import {style} from '../style' with {type: 'macro'};
 import {StyleString} from '../style/types' with {type: 'macro'};
@@ -155,27 +155,26 @@ let arrow = style({
 export const PopoverContext = createContext<ContextValue<PopoverProps, DOMRefValue<HTMLDivElement>>>(null);
 export const InPopoverContext = createContext(false);
 
-export const PopoverBase = forwardRef(function PopoverBase(props: PopoverProps, ref: DOMRef<HTMLDivElement>) {
-  [props, ref] = useSpectrumContextProps(props, ref, PopoverContext);
+export const PopoverBase = forwardRef(function PopoverBase(props: PopoverProps, ref: ForwardedRef<HTMLDivElement | null>) {
   let {
     hideArrow = false,
     UNSAFE_className = '',
     UNSAFE_style,
     styles,
     size
   } = props;
-  let domRef = useDOMRef(ref);
   let colorScheme = useContext(ColorSchemeContext);
   let {locale, direction} = useLocale();
 
   // TODO: should we pass through lang and dir props in RAC?
   let popoverRef = useCallback((el: HTMLDivElement) => {
-    (domRef as MutableRefObject<HTMLDivElement>).current = el;
     if (el) {
       el.lang = locale;
       el.dir = direction;
     }
-  }, [locale, direction, domRef]);
+  }, [locale, direction]);
+  // Memoed so it doesn't break ComboBox/Picker scrolling
+  let mergedRef = useMemo(() => mergeRefs(popoverRef, ref), [ref, popoverRef]);
 
   // On small devices, show a modal (or eventually a tray) instead of a popover.
   // TODO: reverted this until we have trays.
@@ -204,7 +203,7 @@ export const PopoverBase = forwardRef(function PopoverBase(props: PopoverProps,
     <AriaPopover
       {...props}
       offset={(props.offset ?? 8) + (hideArrow ? 0 : 8)}
-      ref={popoverRef}
+      ref={mergedRef}
       style={{
         ...UNSAFE_style,
         // Override default z-index from useOverlayPosition. We use isolation: isolate instead.
@@ -229,43 +228,33 @@ export const PopoverBase = forwardRef(function PopoverBase(props: PopoverProps,
   );
 });
 
-export interface PopoverDialogProps extends Pick<PopoverProps, 'size' | 'hideArrow'| 'placement' | 'shouldFlip' | 'containerPadding' | 'offset' | 'crossOffset' | 'triggerRef' | 'isOpen' | 'onOpenChange'>, Omit<DialogProps, 'className' | 'style' | keyof GlobalDOMAttributes>, StyleProps {
+export interface PopoverDialogProps extends Pick<PopoverProps, 'children' | 'size' | 'hideArrow'| 'placement' | 'shouldFlip' | 'containerPadding' | 'offset' | 'crossOffset' | 'triggerRef' | 'isOpen' | 'onOpenChange'>, Omit<DialogProps, 'children' | 'className' | 'style' | keyof GlobalDOMAttributes>, StyleProps {
 }
 
-
+// TODO this now goes on the Popover itself, do we want to allow users to override the height?
+// That made sense for the inner dialog when that existed, but probably is undesirable for the popover itself
+// Will move padding into the popover styles instead when this is decided
 const dialogStyle = style({
   padding: 8,
-  boxSizing: 'border-box',
-  outlineStyle: 'none',
-  borderRadius: 'inherit',
-  overflow: 'auto',
-  position: 'relative',
-  width: 'full',
-  maxSize: 'inherit'
+  display: 'block',
+  overflow: 'auto'
 }, getAllowedOverrides({height: true}));
 
 /**
  * A popover is an overlay element positioned relative to a trigger.
  */
-export const Popover = forwardRef(function Popover(props: PopoverDialogProps, ref: DOMRef) {
+export const Popover = forwardRef(function Popover(props: PopoverDialogProps, ref: DOMRef<HTMLDivElement>) {
+  [props, ref] = useSpectrumContextProps(props, ref, PopoverContext);
   let domRef = useDOMRef(ref);
-  const {triggerRef, isOpen, onOpenChange, ...otherProps} = props;
-
 
   return (
-    <PopoverBase  isOpen={isOpen} onOpenChange={onOpenChange} triggerRef={triggerRef} size={props.size} hideArrow={props.hideArrow} placement={props.placement} shouldFlip={props.shouldFlip} containerPadding={props.containerPadding} offset={props.offset} crossOffset={props.crossOffset}>
-      <Dialog
-        {...otherProps}
-        ref={domRef}
-        style={props.UNSAFE_style}
-        className={(props.UNSAFE_className || '') + dialogStyle(null, props.styles)}>
-        {composeRenderProps(props.children, (children) => (
-          // Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state.
-          <OverlayTriggerStateContext.Provider value={null}>
-            {children}
-          </OverlayTriggerStateContext.Provider>
-        ))}
-      </Dialog>
+    <PopoverBase {...props} ref={domRef} styles={dialogStyle(null, props.styles)}>
+      {composeRenderProps(props.children, (children) => (
+        // Reset OverlayTriggerStateContext so the buttons inside the dialog don't retain their hover state.
+        <OverlayTriggerStateContext.Provider value={null}>
+          {children}
+        </OverlayTriggerStateContext.Provider>
+      ))}
     </PopoverBase>
   );
 });