feat: Automatically detect if wrapped Autocomplete collection supports virtual focus (#8862)

* exporting contexts for quarry

* refactor autocomplete hook so it detects if it is attached to a filterable collection and if said collection supports virtual focus

* initial attempt to get rid of Dialog from S2 Popover

* fix tests

* add inner div with overflow so that popover arrow shows up

* fix items overflowing when partially scrolled into view

see https://github.com/adobe/react-spectrum/pull/7672/files

* update Popover so it accepts more values in styles instead

* fix lint

* export colorSchemeContext

* clean up extra comment

* fix chromatic

* review comments

* fix calc

* substitute custom event in favor of collection tabindex check

collection should only have a tab index if it isnt using virtual focus

* update width calculations

turns out the border of the popover shouldnt be included in the total width calculation, it is considered outside of the popover so no need to adjust. The border will be removed in favor of a box shadow to conform with update designs later

* forgot to remove commented out code

* move popover styling to user provided inner div

as per discussion with team, if someone wants to modify their popover internals, they are expected to add a inner wrapping div themselves and turn off padding like custom dialog.

* forgot to bring back styles

---------

Co-authored-by: Robert Snow <[email protected]>

Author: LFDanLu

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, 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);
   }, []);
@@ -145,8 +149,16 @@ 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.
+      if (collectionNode.getAttribute('tabindex') != null) {
+        setShouldUseVirtualFocus(false);
+      }
+      setHasCollection(true);
     } else {
       lastCollectionNode.current?.removeEventListener('focusin', updateActiveDescendant);
+      setHasCollection(false);
     }
   }, [updateActiveDescendant]);
 
@@ -393,7 +405,7 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     onFocus
   };
 
-  if (collectionId) {
+  if (hasCollection) {
     inputProps = {
       ...inputProps,
       ...(shouldUseVirtualFocus && virtualFocusProps),
@@ -413,7 +425,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-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';
@@ -478,7 +478,7 @@ const ComboboxInner = forwardRef(function ComboboxInner(props: ComboBoxProps<any
   }
 
   let triggerRef = useRef<HTMLDivElement>(null);
-    // Make menu width match input + button
+  // Make menu width match input + button
   let [triggerWidth, setTriggerWidth] = useState<string | null>(null);
   let onResize = useCallback(() => {
     if (triggerRef.current) {
@@ -643,57 +643,62 @@ const ComboboxInner = forwardRef(function ComboboxInner(props: ComboBoxProps<any
           description={descriptionMessage}>
           {errorMessage}
         </HelpText>
-        <PopoverBase
+        <Popover
           hideArrow
           triggerRef={triggerRef}
           offset={menuOffset}
           placement={`${direction} ${align}` as Placement}
           shouldFlip={shouldFlip}
           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': (menuWidth ? menuWidth + 'px' : triggerWidth)
           } as CSSProperties}
+          padding="none"
           styles={style({
             minWidth: '--trigger-width',
             width: '--trigger-width'
           })}>
-          <Provider
-            values={[
-              [HeaderContext, {styles: listboxHeader({size})}],
-              [HeadingContext, {
-                // @ts-ignore
-                role: 'presentation',
-                styles: sectionHeading
-              }],
-              [TextContext, {
-                slots: {
-                  'description': {styles: description({size})}
-                }
-              }]
-            ]}>
-            <Virtualizer
-              layout={ListLayout}
-              layoutOptions={{
-                estimatedRowHeight: 32,
-                padding: 8,
-                estimatedHeadingHeight: 50,
-                loaderHeight: LOADER_ROW_HEIGHTS[size][scale]
-              }}>
-              <ListBox
-                dependencies={props.dependencies}
-                renderEmptyState={() => (
-                  <span className={emptyStateText({size})}>
-                    {loadingState === 'loading' ? stringFormatter.format('table.loading') : stringFormatter.format('combobox.noResults')}
-                  </span>
-                )}
-                items={items}
-                className={listbox({size})}>
-                {renderer}
-              </ListBox>
-            </Virtualizer>
-          </Provider>
-        </PopoverBase>
+          <div
+            className={style({
+              display: 'flex',
+              size: 'full'
+            })}>
+            <Provider
+              values={[
+                [HeaderContext, {styles: listboxHeader({size})}],
+                [HeadingContext, {
+                  // @ts-ignore
+                  role: 'presentation',
+                  styles: sectionHeading
+                }],
+                [TextContext, {
+                  slots: {
+                    'description': {styles: description({size})}
+                  }
+                }]
+              ]}>
+              <Virtualizer
+                layout={ListLayout}
+                layoutOptions={{
+                  estimatedRowHeight: 32,
+                  padding: 8,
+                  estimatedHeadingHeight: 50,
+                  loaderHeight: LOADER_ROW_HEIGHTS[size][scale]
+                }}>
+                <ListBox
+                  dependencies={props.dependencies}
+                  renderEmptyState={() => (
+                    <span className={emptyStateText({size})}>
+                      {loadingState === 'loading' ? stringFormatter.format('table.loading') : stringFormatter.format('combobox.noResults')}
+                    </span>
+                  )}
+                  items={items}
+                  className={listbox({size})}>
+                  {renderer}
+                </ListBox>
+              </Virtualizer>
+            </Provider>
+          </div>
+        </Popover>
       </InternalComboboxContext.Provider>
     </>
   );

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

@@ -11,7 +11,7 @@ import InfoIcon from '../s2wf-icons/S2_Icon_InfoCircle_20_N.svg';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {mergeStyles} from '../style/runtime';
-import {PopoverBase, PopoverDialogProps} from './Popover';
+import {Popover, PopoverDialogProps} from './Popover';
 import {space, style} from '../style' with {type: 'macro'};
 import {StyleProps} from './style-utils' with { type: 'macro' };
 import {useLocalizedStringFormatter} from '@react-aria/i18n';
@@ -39,11 +39,12 @@ export interface ContextualHelpProps extends
   size?: 'XS' | 'S'
 }
 
-const popover = style({
-  fontFamily: 'sans',
+const wrappingDiv = style({
   minWidth: 268,
   width: 268,
-  padding: 24
+  padding: 24,
+  boxSizing: 'border-box',
+  height: 'full'
 });
 
 export const ContextualHelpContext = createContext<ContextValue<Partial<ContextualHelpProps>, FocusableRefValue<HTMLButtonElement>>>(null);
@@ -96,39 +97,42 @@ export const ContextualHelp = forwardRef(function ContextualHelp(props: Contextu
         isQuiet>
         {variant === 'info' ? <InfoIcon /> : <HelpIcon />}
       </ActionButton>
-      <PopoverBase
+      <Popover
+        padding="none"
         placement={placement}
         shouldFlip={shouldFlip}
         // not working => containerPadding={containerPadding}
         offset={offset}
         crossOffset={crossOffset}
-        hideArrow
-        styles={popover}>
-        <RACDialog className={mergeStyles(dialogInner, style({borderRadius: 'none', margin: 'calc(self(paddingTop) * -1)', padding: 24}))}>
-          <Provider
-            values={[
-              [TextContext, {
-                slots: {
-                  [DEFAULT_SLOT]: {}
-                }
-              }],
-              [HeadingContext, {styles: style({
-                font: 'heading-xs',
-                margin: 0,
-                marginBottom: space(8) // This only makes it 10px on mobile and should be 12px
-              })}],
-              [ContentContext, {styles: style({
-                font: 'body-sm'
-              })}],
-              [FooterContext, {styles: style({
-                font: 'body-sm',
-                marginTop: 16
-              })}]
-            ]}>
-            {children}
-          </Provider>
-        </RACDialog>
-      </PopoverBase>
+        hideArrow>
+        <div
+          className={wrappingDiv}>
+          <RACDialog className={mergeStyles(dialogInner, style({borderRadius: 'none', margin: 'calc(self(paddingTop) * -1)', padding: 24}))}>
+            <Provider
+              values={[
+                [TextContext, {
+                  slots: {
+                    [DEFAULT_SLOT]: {}
+                  }
+                }],
+                [HeadingContext, {styles: style({
+                  font: 'heading-xs',
+                  margin: 0,
+                  marginBottom: space(8) // This only makes it 10px on mobile and should be 12px
+                })}],
+                [ContentContext, {styles: style({
+                  font: 'body-sm'
+                })}],
+                [FooterContext, {styles: style({
+                  font: 'body-sm',
+                  marginTop: 16
+                })}]
+              ]}>
+              {children}
+            </Provider>
+          </RACDialog>
+        </div>
+      </Popover>
     </DialogTrigger>
   );
 });

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

@@ -33,7 +33,7 @@ import {FieldGroup, FieldLabel, HelpText} from './Field';
 import {forwardRefType, GlobalDOMAttributes, HelpTextProps, SpectrumLabelableProps} from '@react-types/shared';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
-import {PopoverBase} from './Popover';
+import {Popover} from './Popover';
 import {pressScale} from './pressScale';
 import {useLocalizedStringFormatter} from '@react-aria/i18n';
 import {useSpectrumContextProps} from './useSpectrumContextProps';
@@ -237,25 +237,30 @@ export const DatePicker = /*#__PURE__*/ (forwardRef as forwardRefType)(function
 
 export function CalendarPopover(props: PropsWithChildren): ReactElement {
   return (
-    <PopoverBase
+    <Popover
       hideArrow
-      styles={style({
-        paddingX: 16,
-        paddingY: 32,
-        overflow: 'auto',
-        display: 'flex',
-        flexDirection: 'column',
-        gap: 16
-      })}>
-      <Dialog>
-        <Provider
-          values={[
-            [OverlayTriggerStateContext, null]
-          ]}>
-          {props.children}
-        </Provider>
-      </Dialog>
-    </PopoverBase>
+      padding="none">
+      <div
+        className={style({
+          paddingX: 16,
+          paddingY: 32,
+          overflow: 'auto',
+          display: 'flex',
+          flexDirection: 'column',
+          gap: 16,
+          boxSizing: 'border-box',
+          size: 'full'
+        })}>
+        <Dialog>
+          <Provider
+            values={[
+              [OverlayTriggerStateContext, null]
+            ]}>
+            {props.children}
+          </Provider>
+        </Dialog>
+      </div>
+    </Popover>
   );
 }
 

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';
@@ -320,6 +320,11 @@ let InternalMenuContext = createContext<{size: 'S' | 'M' | 'L' | 'XL', isSubmenu
 
 let InternalMenuTriggerContext = createContext<Omit<MenuTriggerProps, 'children'> | null>(null);
 
+let wrappingDiv = style({
+  display: 'flex',
+  size: 'full'
+});
+
 /**
  * Menus display a list of actions or options that a user can choose.
  */
@@ -366,14 +371,16 @@ export const Menu = /*#__PURE__*/ (forwardRef as forwardRefType)(function Menu<T
 
   if (isPopover) {
     return (
-      <PopoverBase
+      <Popover
         ref={ref}
-        hideArrow
-        UNSAFE_style={UNSAFE_style}
-        UNSAFE_className={UNSAFE_className}
-        styles={styles}>
-        {content}
-      </PopoverBase>
+        padding="none"
+        hideArrow>
+        <div
+          style={UNSAFE_style}
+          className={(UNSAFE_className || '') + wrappingDiv}>
+          {content}
+        </div>
+      </Popover>
     );
   }