fix: Autocomplete context refactor

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

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {AriaLabelingProps, BaseEvent, DOMProps, Node, RefObject} from '@react-types/shared';
+import {AriaLabelingProps, BaseEvent, DOMProps, FocusableElement, Node, RefObject} 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, isCtrlKeyPressed, mergeProps, mergeRefs, useEffectEvent, useEvent, useLabels, useObjectRef, useSlotId} from '@react-aria/utils';
@@ -28,7 +28,6 @@ export interface CollectionOptions extends DOMProps, AriaLabelingProps {
   disallowTypeAhead: boolean
 }
 
-// TODO; For now go with Node here, but maybe pare it down to just the essentials? Value, key, and maybe type?
 export interface AriaAutocompleteProps<T> extends AutocompleteProps {
   /**
    * An optional filter function used to determine if a option should be included in the autocomplete list.
@@ -37,10 +36,17 @@ export interface AriaAutocompleteProps<T> extends AutocompleteProps {
   filter?: (textValue: string, inputValue: string, node: Node<T>) => boolean,
 
   /**
-   * Whether or not to focus the first item in the collection after a filter is performed.
+   * Whether or not to focus the first item in the collection after a filter is performed. Note this is only applicable
+   * if virtual focus behavior is not turned off via `disableVirtualFocus`.
    * @default false
    */
-  disableAutoFocusFirst?: boolean
+  disableAutoFocusFirst?: boolean,
+
+  /**
+   * Whether the autocomplete should disable virtual focus, instead making the wrapped collection directly tabbable.
+   * @default false
+   */
+  disableVirtualFocus?: boolean
 }
 
 export interface AriaAutocompleteOptions<T> extends Omit<AriaAutocompleteProps<T>, 'children'> {
@@ -52,7 +58,7 @@ export interface AriaAutocompleteOptions<T> extends Omit<AriaAutocompleteProps<T
 
 export interface AutocompleteAria<T> {
   /** Props for the autocomplete textfield/searchfield element. These should be passed to the textfield/searchfield aria hooks respectively. */
-  textFieldProps: AriaTextFieldProps,
+  textFieldProps: AriaTextFieldProps<FocusableElement>,
   /** Props for the collection, to be passed to collection's respective aria hook (e.g. useMenu). */
   collectionProps: CollectionOptions,
   /** Ref to attach to the wrapped collection. */
@@ -72,7 +78,8 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     inputRef,
     collectionRef,
     filter,
-    disableAutoFocusFirst = false
+    disableAutoFocusFirst = false,
+    disableVirtualFocus = false
   } = props;
 
   let collectionId = useSlotId();
@@ -83,7 +90,7 @@ 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 shouldUseVirtualFocus = getInteractionModality() !== 'virtual';
+  let shouldUseVirtualFocus = getInteractionModality() !== 'virtual' && !disableVirtualFocus;
 
   useEffect(() => {
     return () => clearTimeout(timeout.current);
@@ -254,15 +261,17 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
     }
 
     let shouldPerformDefaultAction = true;
-    if (focusedNodeId == null) {
-      shouldPerformDefaultAction = collectionRef.current?.dispatchEvent(
-        new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
-      ) || false;
-    } else {
-      let item = document.getElementById(focusedNodeId);
-      shouldPerformDefaultAction = item?.dispatchEvent(
-        new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
-      ) || false;
+    if (collectionRef.current !== null) {
+      if (focusedNodeId == null) {
+        shouldPerformDefaultAction = collectionRef.current?.dispatchEvent(
+          new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
+        ) || false;
+      } else {
+        let item = document.getElementById(focusedNodeId);
+        shouldPerformDefaultAction = item?.dispatchEvent(
+          new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
+        ) || false;
+      }
     }
 
     if (shouldPerformDefaultAction) {
@@ -282,6 +291,9 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
           }
           break;
       }
+    } else {
+      // TODO: check if we can do this, want to stop textArea from using its default Enter behavior so items are properly triggered
+      e.preventDefault();
     }
   };
 
@@ -359,25 +371,28 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
   let textFieldProps = {
     value: state.inputValue,
     onChange
-  } as AriaTextFieldProps<HTMLInputElement>;
+  } as AriaTextFieldProps<FocusableElement>;
+
+  let virtualFocusProps = {
+    onKeyDown,
+    'aria-activedescendant': state.focusedNodeId ?? undefined,
+    onBlur,
+    onFocus
+  };
 
   if (collectionId) {
     textFieldProps = {
       ...textFieldProps,
-      onKeyDown,
-      autoComplete: 'off',
-      'aria-haspopup': collectionId ? 'listbox' : undefined,
+      ...(shouldUseVirtualFocus && virtualFocusProps),
+      enterKeyHint: 'go',
       'aria-controls': collectionId,
       // TODO: readd proper logic for completionMode = complete (aria-autocomplete: both)
       'aria-autocomplete': 'list',
-      'aria-activedescendant': state.focusedNodeId ?? undefined,
       // This disable's iOS's autocorrect suggestions, since the autocomplete provides its own suggestions.
       autoCorrect: 'off',
       // This disable's the macOS Safari spell check auto corrections.
       spellCheck: 'false',
-      enterKeyHint: 'go',
-      onBlur,
-      onFocus
+      autoComplete: 'off'
     };
   }
 

packages/@react-aria/collections/src/BaseCollection.ts

@@ -21,6 +21,7 @@ type FilterFn<T> = (textValue: string, node: Node<T>) => boolean;
 
 /** An immutable object representing a Node in a Collection. */
 export class CollectionNode<T> implements Node<T> {
+  static readonly type;
   readonly type: string;
   readonly key: Key;
   readonly value: T | null = null;
@@ -40,8 +41,8 @@ export class CollectionNode<T> implements Node<T> {
   readonly colSpan: number | null = null;
   readonly colIndex: number | null = null;
 
-  constructor(type: string, key: Key) {
-    this.type = type;
+  constructor(key: Key) {
+    this.type = (this.constructor as typeof CollectionNode).type;
     this.key = key;
   }
 
@@ -50,7 +51,7 @@ export class CollectionNode<T> implements Node<T> {
   }
 
   clone(): CollectionNode<T> {
-    let node: Mutable<CollectionNode<T>> = new CollectionNode(this.type, this.key);
+    let node: Mutable<CollectionNode<T>> = new (this.constructor as typeof CollectionNode)(this.key);
     node.value = this.value;
     node.level = this.level;
     node.hasChildNodes = this.hasChildNodes;
@@ -67,7 +68,6 @@ export class CollectionNode<T> implements Node<T> {
     node.render = this.render;
     node.colSpan = this.colSpan;
     node.colIndex = this.colIndex;
-    node.filter = this.filter;
     return node;
   }
 
@@ -89,15 +89,16 @@ export class FilterLessNode<T> extends CollectionNode<T> {
   }
 }
 
+export class LoaderNode extends FilterLessNode<any> {
+  static readonly type = 'loader';
+}
+
 export class ItemNode<T> extends CollectionNode<T> {
   static readonly type = 'item';
 
-  constructor(key: Key) {
-    super(ItemNode.type, key);
-  }
-
   filter(collection: BaseCollection<T>, newCollection: BaseCollection<T>, filterFn: FilterFn<T>): ItemNode<T> | null {
     if (filterFn(this.textValue, this)) {
+      // TODO: returning just this instead of cloning broke filtering, investigate
       return this.clone();
     }
 
@@ -108,10 +109,6 @@ export class ItemNode<T> extends CollectionNode<T> {
 export class SectionNode<T> extends CollectionNode<T> {
   static readonly type = 'section';
 
-  constructor(key: Key) {
-    super(SectionNode.type, key);
-  }
-
   filter(collection: BaseCollection<T>, newCollection: BaseCollection<T>, filterFn: FilterFn<T>): SectionNode<T> | null {
     let filteredSection = super.filter(collection, newCollection, filterFn);
     if (filteredSection) {

packages/@react-aria/collections/src/CollectionBuilder.tsx

@@ -133,10 +133,9 @@ export type CollectionNodeClass<T> = {
 };
 
 function createCollectionNodeClass(type: string): CollectionNodeClass<any> {
-  let NodeClass = function (key: Key) {
-    return new CollectionNode(type, key);
-  } as any;
-  NodeClass.type = type;
+  let NodeClass = class extends CollectionNode<any> {
+    static readonly type = type;
+  };
   return NodeClass;
 }
 
@@ -172,7 +171,6 @@ function useSSRCollectionNode<T extends Element>(CollectionNodeClass: Collection
   }
 
   // @ts-ignore
-  // TODO: could just make this a div perhaps, but keep it in line with how it used to work
   return <CollectionNodeClass.type ref={itemRef}>{children}</CollectionNodeClass.type>;
 }
 

packages/@react-aria/collections/src/Document.ts

@@ -285,10 +285,11 @@ export class ElementNode<T> extends BaseNode<T> {
     return 0;
   }
 
-  get node(): CollectionNode<T> | null {
-    if (this._node == null && process.env.NODE_ENV !== 'production') {
-      console.error('Attempted to access node before it was defined. Check if setProps wasn\'t called before attempting to access the node.');
+  get node(): CollectionNode<T> {
+    if (this._node == null) {
+      throw Error('Attempted to access node before it was defined. Check if setProps wasn\'t called before attempting to access the node.');
     }
+
     return this._node;
   }
 
@@ -302,31 +303,31 @@ export class ElementNode<T> extends BaseNode<T> {
    */
   private getMutableNode(): Mutable<CollectionNode<T>> {
     if (!this.isMutated) {
-      this.node = this.node!.clone();
+      this.node = this.node.clone();
       this.isMutated = true;
     }
 
     this.ownerDocument.markDirty(this);
-    return this.node!;
+    return this.node;
   }
 
   updateNode(): void {
     let nextSibling = this.nextVisibleSibling;
     let node = this.getMutableNode();
     node.index = this.index;
     node.level = this.level;
-    node.parentKey = this.parentNode instanceof ElementNode ? this.parentNode.node!.key : null;
-    node.prevKey = this.previousVisibleSibling?.node!.key ?? null;
-    node.nextKey = nextSibling?.node!.key ?? null;
+    node.parentKey = this.parentNode instanceof ElementNode ? this.parentNode.node.key : null;
+    node.prevKey = this.previousVisibleSibling?.node.key ?? null;
+    node.nextKey = nextSibling?.node.key ?? null;
     node.hasChildNodes = !!this.firstChild;
-    node.firstChildKey = this.firstVisibleChild?.node!.key ?? null;
-    node.lastChildKey = this.lastVisibleChild?.node!.key ?? null;
+    node.firstChildKey = this.firstVisibleChild?.node.key ?? null;
+    node.lastChildKey = this.lastVisibleChild?.node.key ?? null;
 
     // Update the colIndex of sibling nodes if this node has a colSpan.
     if ((node.colSpan != null || node.colIndex != null) && nextSibling) {
       // This queues the next sibling for update, which means this happens recursively.
       let nextColIndex = (node.colIndex ?? node.index) + (node.colSpan ?? 1);
-      if (nextColIndex !== nextSibling.node!.colIndex) {
+      if (nextColIndex !== nextSibling.node.colIndex) {
         let siblingNode = nextSibling.getMutableNode();
         siblingNode.colIndex = nextColIndex;
       }
@@ -455,7 +456,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
     }
 
     let collection = this.getMutableCollection();
-    if (!collection.getItem(element.node!.key)) {
+    if (!collection.getItem(element.node.key)) {
       for (let child of element) {
         this.addNode(child);
       }
@@ -470,7 +471,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
     }
 
     let collection = this.getMutableCollection();
-    collection.removeNode(node.node!.key);
+    collection.removeNode(node.node.key);
   }
 
   /** Finalizes the collection update, updating all nodes and freezing the collection. */
@@ -516,7 +517,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
 
     // Finally, update the collection.
     if (this.nextCollection) {
-      this.nextCollection.commit(this.firstVisibleChild?.node!.key ?? null, this.lastVisibleChild?.node!.key ?? null, this.isSSR);
+      this.nextCollection.commit(this.firstVisibleChild?.node.key ?? null, this.lastVisibleChild?.node.key ?? null, this.isSSR);
       if (!this.isSSR) {
         this.collection = this.nextCollection;
         this.nextCollection = null;

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

@@ -13,7 +13,7 @@
 export {CollectionBuilder, Collection, createLeafComponent, createBranchComponent} from './CollectionBuilder';
 export {createHideableComponent, useIsHidden} from './Hidden';
 export {useCachedChildren} from './useCachedChildren';
-export {BaseCollection, CollectionNode, ItemNode, SectionNode, FilterLessNode} from './BaseCollection';
+export {BaseCollection, CollectionNode, ItemNode, SectionNode, FilterLessNode, LoaderNode} from './BaseCollection';
 
 export type {CollectionBuilderProps, CollectionProps} from './CollectionBuilder';
 export type {CachedChildrenOptions} from './useCachedChildren';

packages/@react-aria/collections/test/CollectionBuilder.test.js

@@ -4,10 +4,6 @@ import {render} from '@testing-library/react';
 
 class ItemNode extends CollectionNode {
   static type = 'item';
-
-  constructor(key) {
-    super(ItemNode.type, key);
-  }
 }
 
 const Item = createLeafComponent(ItemNode, () => {

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

@@ -30,10 +30,10 @@ import {
   ListStateContext,
   Provider,
   SectionProps,
-  SeparatorNode,
   Virtualizer
 } from 'react-aria-components';
 import {AsyncLoadable, GlobalDOMAttributes, HelpTextProps, LoadingState, SpectrumLabelableProps} from '@react-types/shared';
+import {BaseCollection, CollectionNode, createLeafComponent} from '@react-aria/collections';
 import {baseColor, edgeToText, focusRing, space, style} from '../style' with {type: 'macro'};
 import {centerBaseline} from './CenterBaseline';
 import {centerPadding, control, controlBorderRadius, controlFont, controlSize, field, fieldInput, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'};
@@ -49,7 +49,6 @@ import CheckmarkIcon from '../ui-icons/Checkmark';
 import ChevronIcon from '../ui-icons/Chevron';
 import {createContext, CSSProperties, ForwardedRef, forwardRef, ReactNode, Ref, useCallback, useContext, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
 import {createFocusableRef} from '@react-spectrum/utils';
-import {createLeafComponent} from '@react-aria/collections';
 import {FieldErrorIcon, FieldGroup, FieldLabel, HelpText, Input} from './Field';
 import {FormContext, useFormProps} from './Form';
 import {forwardRefType} from './types';
@@ -700,6 +699,19 @@ const ComboboxInner = forwardRef(function ComboboxInner(props: ComboBoxProps<any
   );
 });
 
+class SeparatorNode extends CollectionNode<any> {
+  static readonly type = 'separator';
+
+  filter(collection: BaseCollection<any>, newCollection: BaseCollection<any>): CollectionNode<any> | null {
+    let prevItem = newCollection.getItem(this.prevKey!);
+    if (prevItem && prevItem.type !== 'separator') {
+      return this.clone();
+    }
+
+    return null;
+  }
+}
+
 export const Divider = /*#__PURE__*/ createLeafComponent(SeparatorNode, function Divider({size}: {size?: 'S' | 'M' | 'L' | 'XL'}, ref: ForwardedRef<HTMLDivElement>, node: Node<unknown>) {
   let listState = useContext(ListStateContext)!;
 

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

@@ -11,7 +11,6 @@
  */
 
 import {createLeafComponent, FilterLessNode} from '@react-aria/collections';
-import {Key} from '@react-types/shared';
 import {ReactNode} from 'react';
 import {Skeleton} from './Skeleton';
 
@@ -23,10 +22,6 @@ let cache = new WeakMap();
 
 class SkeletonNode extends FilterLessNode<unknown> {
   static readonly type = 'skeleton';
-
-  constructor(key: Key) {
-    super(SkeletonNode.type, key);
-  }
 }
 
 /**

packages/react-aria-components/package.json

@@ -51,6 +51,7 @@
     "@react-aria/live-announcer": "^3.4.4",
     "@react-aria/overlays": "^3.28.0",
     "@react-aria/ssr": "^3.9.10",
+    "@react-aria/textfield": "^3.18.0",
     "@react-aria/toolbar": "3.0.0-beta.19",
     "@react-aria/utils": "^3.30.0",
     "@react-aria/virtualizer": "^4.1.8",