Skip to content

Commit 4d5fde2

Browse files
authored
fix: Autocomplete context refactor (#8695)
* autoimport.... * replace internal autocomplete context * add FieldInputContext in place of input context and search/textfield context in autocomplete * fix build * removing erroneous autoimports * add ability for user to provide independent filter text * fix lint * fix some more tests * bring back controlled input value at autocomplete level * adding prop to disable virtual focus * another stab at the types * clear autocomplete contexts so that they dont leak to nested collections * add tests for disallowVirtualFocus works with listbox and menu * fix types * refactor CollectionNode to read from static property and properly clone from subclass * naming from reviews and moving contexts out of autocomplete * review comments * properly add all descendants of a cloned node when filtering fixes case where a filtered table keyboard navigation was broken since we had cloned the old collection rather than creating a new one from scratch
1 parent 57e57e0 commit 4d5fde2

28 files changed

+317
-285
lines changed

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

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {AriaLabelingProps, BaseEvent, DOMProps, Node, RefObject} from '@react-types/shared';
13+
import {AriaLabelingProps, BaseEvent, DOMProps, FocusableElement, Node, RefObject} from '@react-types/shared';
1414
import {AriaTextFieldProps} from '@react-aria/textfield';
1515
import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete';
1616
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 {
2828
disallowTypeAhead: boolean
2929
}
3030

31-
// TODO; For now go with Node here, but maybe pare it down to just the essentials? Value, key, and maybe type?
3231
export interface AriaAutocompleteProps<T> extends AutocompleteProps {
3332
/**
3433
* 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 {
3736
filter?: (textValue: string, inputValue: string, node: Node<T>) => boolean,
3837

3938
/**
40-
* Whether or not to focus the first item in the collection after a filter is performed.
39+
* Whether or not to focus the first item in the collection after a filter is performed. Note this is only applicable
40+
* if virtual focus behavior is not turned off via `disableVirtualFocus`.
4141
* @default false
4242
*/
43-
disableAutoFocusFirst?: boolean
43+
disableAutoFocusFirst?: boolean,
44+
45+
/**
46+
* Whether the autocomplete should disable virtual focus, instead making the wrapped collection directly tabbable.
47+
* @default false
48+
*/
49+
disableVirtualFocus?: boolean
4450
}
4551

4652
export interface AriaAutocompleteOptions<T> extends Omit<AriaAutocompleteProps<T>, 'children'> {
@@ -52,7 +58,7 @@ export interface AriaAutocompleteOptions<T> extends Omit<AriaAutocompleteProps<T
5258

5359
export interface AutocompleteAria<T> {
5460
/** Props for the autocomplete textfield/searchfield element. These should be passed to the textfield/searchfield aria hooks respectively. */
55-
textFieldProps: AriaTextFieldProps,
61+
textFieldProps: AriaTextFieldProps<FocusableElement>,
5662
/** Props for the collection, to be passed to collection's respective aria hook (e.g. useMenu). */
5763
collectionProps: CollectionOptions,
5864
/** Ref to attach to the wrapped collection. */
@@ -72,7 +78,8 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
7278
inputRef,
7379
collectionRef,
7480
filter,
75-
disableAutoFocusFirst = false
81+
disableAutoFocusFirst = false,
82+
disableVirtualFocus = false
7683
} = props;
7784

7885
let collectionId = useSlotId();
@@ -83,7 +90,7 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
8390

8491
// For mobile screen readers, we don't want virtual focus, instead opting to disable FocusScope's restoreFocus and manually
8592
// moving focus back to the subtriggers
86-
let shouldUseVirtualFocus = getInteractionModality() !== 'virtual';
93+
let shouldUseVirtualFocus = getInteractionModality() !== 'virtual' && !disableVirtualFocus;
8794

8895
useEffect(() => {
8996
return () => clearTimeout(timeout.current);
@@ -254,15 +261,17 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
254261
}
255262

256263
let shouldPerformDefaultAction = true;
257-
if (focusedNodeId == null) {
258-
shouldPerformDefaultAction = collectionRef.current?.dispatchEvent(
259-
new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
260-
) || false;
261-
} else {
262-
let item = document.getElementById(focusedNodeId);
263-
shouldPerformDefaultAction = item?.dispatchEvent(
264-
new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
265-
) || false;
264+
if (collectionRef.current !== null) {
265+
if (focusedNodeId == null) {
266+
shouldPerformDefaultAction = collectionRef.current?.dispatchEvent(
267+
new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
268+
) || false;
269+
} else {
270+
let item = document.getElementById(focusedNodeId);
271+
shouldPerformDefaultAction = item?.dispatchEvent(
272+
new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
273+
) || false;
274+
}
266275
}
267276

268277
if (shouldPerformDefaultAction) {
@@ -282,6 +291,9 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
282291
}
283292
break;
284293
}
294+
} else {
295+
// TODO: check if we can do this, want to stop textArea from using its default Enter behavior so items are properly triggered
296+
e.preventDefault();
285297
}
286298
};
287299

@@ -359,25 +371,28 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
359371
let textFieldProps = {
360372
value: state.inputValue,
361373
onChange
362-
} as AriaTextFieldProps<HTMLInputElement>;
374+
} as AriaTextFieldProps<FocusableElement>;
375+
376+
let virtualFocusProps = {
377+
onKeyDown,
378+
'aria-activedescendant': state.focusedNodeId ?? undefined,
379+
onBlur,
380+
onFocus
381+
};
363382

364383
if (collectionId) {
365384
textFieldProps = {
366385
...textFieldProps,
367-
onKeyDown,
368-
autoComplete: 'off',
369-
'aria-haspopup': collectionId ? 'listbox' : undefined,
386+
...(shouldUseVirtualFocus && virtualFocusProps),
387+
enterKeyHint: 'go',
370388
'aria-controls': collectionId,
371389
// TODO: readd proper logic for completionMode = complete (aria-autocomplete: both)
372390
'aria-autocomplete': 'list',
373-
'aria-activedescendant': state.focusedNodeId ?? undefined,
374391
// This disable's iOS's autocorrect suggestions, since the autocomplete provides its own suggestions.
375392
autoCorrect: 'off',
376393
// This disable's the macOS Safari spell check auto corrections.
377394
spellCheck: 'false',
378-
enterKeyHint: 'go',
379-
onBlur,
380-
onFocus
395+
autoComplete: 'off'
381396
};
382397
}
383398

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

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type FilterFn<T> = (textValue: string, node: Node<T>) => boolean;
2121

2222
/** An immutable object representing a Node in a Collection. */
2323
export class CollectionNode<T> implements Node<T> {
24+
static readonly type;
2425
readonly type: string;
2526
readonly key: Key;
2627
readonly value: T | null = null;
@@ -40,17 +41,17 @@ export class CollectionNode<T> implements Node<T> {
4041
readonly colSpan: number | null = null;
4142
readonly colIndex: number | null = null;
4243

43-
constructor(type: string, key: Key) {
44-
this.type = type;
44+
constructor(key: Key) {
45+
this.type = (this.constructor as typeof CollectionNode).type;
4546
this.key = key;
4647
}
4748

4849
get childNodes(): Iterable<Node<T>> {
4950
throw new Error('childNodes is not supported');
5051
}
5152

52-
clone(): CollectionNode<T> {
53-
let node: Mutable<CollectionNode<T>> = new CollectionNode(this.type, this.key);
53+
clone(): this {
54+
let node: Mutable<this> = new (this.constructor as any)(this.key);
5455
node.value = this.value;
5556
node.level = this.level;
5657
node.hasChildNodes = this.hasChildNodes;
@@ -67,7 +68,6 @@ export class CollectionNode<T> implements Node<T> {
6768
node.render = this.render;
6869
node.colSpan = this.colSpan;
6970
node.colIndex = this.colIndex;
70-
node.filter = this.filter;
7171
return node;
7272
}
7373

@@ -85,20 +85,24 @@ export class CollectionNode<T> implements Node<T> {
8585
export class FilterLessNode<T> extends CollectionNode<T> {
8686
// eslint-disable-next-line @typescript-eslint/no-unused-vars
8787
filter(collection: BaseCollection<T>, newCollection: BaseCollection<T>, filterFn: FilterFn<T>): FilterLessNode<T> | null {
88-
return this.clone();
88+
let clone = this.clone();
89+
newCollection.addDescendants(clone, collection);
90+
return clone;
8991
}
9092
}
9193

94+
export class LoaderNode extends FilterLessNode<any> {
95+
static readonly type = 'loader';
96+
}
97+
9298
export class ItemNode<T> extends CollectionNode<T> {
9399
static readonly type = 'item';
94100

95-
constructor(key: Key) {
96-
super(ItemNode.type, key);
97-
}
98-
99101
filter(collection: BaseCollection<T>, newCollection: BaseCollection<T>, filterFn: FilterFn<T>): ItemNode<T> | null {
100102
if (filterFn(this.textValue, this)) {
101-
return this.clone();
103+
let clone = this.clone();
104+
newCollection.addDescendants(clone, collection);
105+
return clone;
102106
}
103107

104108
return null;
@@ -108,10 +112,6 @@ export class ItemNode<T> extends CollectionNode<T> {
108112
export class SectionNode<T> extends CollectionNode<T> {
109113
static readonly type = 'section';
110114

111-
constructor(key: Key) {
112-
super(SectionNode.type, key);
113-
}
114-
115115
filter(collection: BaseCollection<T>, newCollection: BaseCollection<T>, filterFn: FilterFn<T>): SectionNode<T> | null {
116116
let filteredSection = super.filter(collection, newCollection, filterFn);
117117
if (filteredSection) {
@@ -259,6 +259,15 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
259259
this.keyMap.set(node.key, node);
260260
}
261261

262+
// Deeply add a node and its children to the collection from another collection, primarily used when filtering a collection
263+
addDescendants(node: CollectionNode<T>, oldCollection: BaseCollection<T>): void {
264+
this.addNode(node);
265+
let children = oldCollection.getChildren(node.key);
266+
for (let child of children) {
267+
this.addDescendants(child as CollectionNode<T>, oldCollection);
268+
}
269+
}
270+
262271
removeNode(key: Key): void {
263272
if (this.frozen) {
264273
throw new Error('Cannot remove a node to a frozen collection');
@@ -282,14 +291,10 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
282291
this.frozen = !isSSR;
283292
}
284293

285-
filter(filterFn: FilterFn<T>, newCollection?: BaseCollection<T>): BaseCollection<T> {
286-
if (newCollection == null) {
287-
newCollection = new BaseCollection<T>();
288-
}
289-
294+
filter(filterFn: FilterFn<T>): this {
295+
let newCollection = new (this.constructor as any)();
290296
let [firstKey, lastKey] = filterChildren(this, newCollection, this.firstKey, filterFn);
291-
newCollection.firstKey = firstKey;
292-
newCollection.lastKey = lastKey;
297+
newCollection?.commit(firstKey, lastKey);
293298
return newCollection;
294299
}
295300
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,9 @@ export type CollectionNodeClass<T> = {
133133
};
134134

135135
function createCollectionNodeClass(type: string): CollectionNodeClass<any> {
136-
let NodeClass = function (key: Key) {
137-
return new CollectionNode(type, key);
138-
} as any;
139-
NodeClass.type = type;
136+
let NodeClass = class extends CollectionNode<any> {
137+
static readonly type = type;
138+
};
140139
return NodeClass;
141140
}
142141

@@ -172,7 +171,6 @@ function useSSRCollectionNode<T extends Element>(CollectionNodeClass: Collection
172171
}
173172

174173
// @ts-ignore
175-
// TODO: could just make this a div perhaps, but keep it in line with how it used to work
176174
return <CollectionNodeClass.type ref={itemRef}>{children}</CollectionNodeClass.type>;
177175
}
178176

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,6 @@ export class ElementNode<T> extends BaseNode<T> {
260260
private _node: CollectionNode<T> | null;
261261
isMutated = true;
262262
private _index: number = 0;
263-
hasSetProps = false;
264263
isHidden = false;
265264

266265
constructor(type: string, ownerDocument: Document<T, any>) {
@@ -285,10 +284,11 @@ export class ElementNode<T> extends BaseNode<T> {
285284
return 0;
286285
}
287286

288-
get node(): CollectionNode<T> | null {
289-
if (this._node == null && process.env.NODE_ENV !== 'production') {
290-
console.error('Attempted to access node before it was defined. Check if setProps wasn\'t called before attempting to access the node.');
287+
get node(): CollectionNode<T> {
288+
if (this._node == null) {
289+
throw Error('Attempted to access node before it was defined. Check if setProps wasn\'t called before attempting to access the node.');
291290
}
291+
292292
return this._node;
293293
}
294294

@@ -302,31 +302,31 @@ export class ElementNode<T> extends BaseNode<T> {
302302
*/
303303
private getMutableNode(): Mutable<CollectionNode<T>> {
304304
if (!this.isMutated) {
305-
this.node = this.node!.clone();
305+
this.node = this.node.clone();
306306
this.isMutated = true;
307307
}
308308

309309
this.ownerDocument.markDirty(this);
310-
return this.node!;
310+
return this.node;
311311
}
312312

313313
updateNode(): void {
314314
let nextSibling = this.nextVisibleSibling;
315315
let node = this.getMutableNode();
316316
node.index = this.index;
317317
node.level = this.level;
318-
node.parentKey = this.parentNode instanceof ElementNode ? this.parentNode.node!.key : null;
319-
node.prevKey = this.previousVisibleSibling?.node!.key ?? null;
320-
node.nextKey = nextSibling?.node!.key ?? null;
318+
node.parentKey = this.parentNode instanceof ElementNode ? this.parentNode.node.key : null;
319+
node.prevKey = this.previousVisibleSibling?.node.key ?? null;
320+
node.nextKey = nextSibling?.node.key ?? null;
321321
node.hasChildNodes = !!this.firstChild;
322-
node.firstChildKey = this.firstVisibleChild?.node!.key ?? null;
323-
node.lastChildKey = this.lastVisibleChild?.node!.key ?? null;
322+
node.firstChildKey = this.firstVisibleChild?.node.key ?? null;
323+
node.lastChildKey = this.lastVisibleChild?.node.key ?? null;
324324

325325
// Update the colIndex of sibling nodes if this node has a colSpan.
326326
if ((node.colSpan != null || node.colIndex != null) && nextSibling) {
327327
// This queues the next sibling for update, which means this happens recursively.
328328
let nextColIndex = (node.colIndex ?? node.index) + (node.colSpan ?? 1);
329-
if (nextColIndex !== nextSibling.node!.colIndex) {
329+
if (nextColIndex !== nextSibling.node.colIndex) {
330330
let siblingNode = nextSibling.getMutableNode();
331331
siblingNode.colIndex = nextColIndex;
332332
}
@@ -455,7 +455,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
455455
}
456456

457457
let collection = this.getMutableCollection();
458-
if (!collection.getItem(element.node!.key)) {
458+
if (!collection.getItem(element.node.key)) {
459459
for (let child of element) {
460460
this.addNode(child);
461461
}
@@ -470,7 +470,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
470470
}
471471

472472
let collection = this.getMutableCollection();
473-
collection.removeNode(node.node!.key);
473+
collection.removeNode(node.node.key);
474474
}
475475

476476
/** Finalizes the collection update, updating all nodes and freezing the collection. */
@@ -516,7 +516,7 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
516516

517517
// Finally, update the collection.
518518
if (this.nextCollection) {
519-
this.nextCollection.commit(this.firstVisibleChild?.node!.key ?? null, this.lastVisibleChild?.node!.key ?? null, this.isSSR);
519+
this.nextCollection.commit(this.firstVisibleChild?.node.key ?? null, this.lastVisibleChild?.node.key ?? null, this.isSSR);
520520
if (!this.isSSR) {
521521
this.collection = this.nextCollection;
522522
this.nextCollection = null;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
export {CollectionBuilder, Collection, createLeafComponent, createBranchComponent} from './CollectionBuilder';
1414
export {createHideableComponent, useIsHidden} from './Hidden';
1515
export {useCachedChildren} from './useCachedChildren';
16-
export {BaseCollection, CollectionNode, ItemNode, SectionNode, FilterLessNode} from './BaseCollection';
16+
export {BaseCollection, CollectionNode, ItemNode, SectionNode, FilterLessNode, LoaderNode} from './BaseCollection';
1717

1818
export type {CollectionBuilderProps, CollectionProps} from './CollectionBuilder';
1919
export type {CachedChildrenOptions} from './useCachedChildren';

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,6 @@ import {render} from '@testing-library/react';
44

55
class ItemNode extends CollectionNode {
66
static type = 'item';
7-
8-
constructor(key) {
9-
super(ItemNode.type, key);
10-
}
117
}
128

139
const Item = createLeafComponent(ItemNode, () => {

0 commit comments

Comments
 (0)