Skip to content

feat: Add subdialog support to Menu and Autocomplete #7561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 68 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
db04fec
rough start
LFDanLu Dec 17, 2024
2f7cb82
fix contain
LFDanLu Dec 19, 2024
c3fece1
debug ESC handler
LFDanLu Dec 20, 2024
34ab703
fix rendering of subdialog in autocomplete
LFDanLu Jan 2, 2025
e63c5fd
debug dynamic case and fix context
LFDanLu Jan 3, 2025
5e3fb0b
handle submenutriggers in base collection filtering and fix id issue …
LFDanLu Jan 3, 2025
30e6762
prevent focus from being lost to the body when submenutrigger is virt…
LFDanLu Jan 3, 2025
c4fc772
ensure that the focused subdialogtrigger item remains focused after o…
LFDanLu Jan 15, 2025
19f9eaa
close submenu when virtual focus moves off trigger and properly dispa…
LFDanLu Jan 16, 2025
222807e
Merge branch 'main' of github.com:adobe/react-spectrum into subdialogs
LFDanLu Jan 16, 2025
b3f3ecd
fix hover for now so I can debug the focus issues
LFDanLu Jan 17, 2025
931c28e
fix tab handling in subdialogs and opening submenus on mobile when us…
LFDanLu Jan 18, 2025
fdc40c5
Fix to close submenus/dialogs when in a Autocomplete that isnt in a p…
LFDanLu Jan 22, 2025
a1bd7f5
fix issue where you cant shift tab from the Autocompletes text field
LFDanLu Jan 22, 2025
cc954c2
fix shift tabbing from closing the subdialogs when rendered by Autoco…
LFDanLu Jan 22, 2025
016bb1a
Merge branch 'main' into subdialogs
LFDanLu Jan 22, 2025
2e581cf
make sure expanded triggers dont have visible focus ring
LFDanLu Jan 23, 2025
f27a766
fix all subdialog being closed when using ESC on a subMenuTrigger and…
LFDanLu Jan 23, 2025
5cb78be
update mobile screen reader behavior so focus is restored to the subt…
LFDanLu Jan 24, 2025
a2c87ed
only close outer most submenu/subdialog when using ESC
LFDanLu Jan 24, 2025
43ab42f
partial fix to only persist a single focus ring on item or input for …
LFDanLu Jan 24, 2025
7f4a779
Making focus ring appear on virtually focused items and on input fiel…
LFDanLu Jan 24, 2025
19251e1
Merge branch 'main' into subdialogs
LFDanLu Jan 24, 2025
7af516b
simplifiying useSelectableItem change in favor of proper manager.isFo…
LFDanLu Jan 24, 2025
61d812a
Merge branch 'main' into subdialogs
LFDanLu Jan 27, 2025
9922dd7
saving point for tests
LFDanLu Jan 28, 2025
fec56ff
Fix focusscope for subdialogs and add test
LFDanLu Jan 28, 2025
d1e9eef
Fix S2 autocomplete double focus ring
LFDanLu Jan 28, 2025
eab66a8
adding tests for subdialog and testing useInteractOutside for subdial…
LFDanLu Jan 28, 2025
0d2f41d
add submenu tests
LFDanLu Jan 29, 2025
5a32a08
fix ESC closing multiple levels of subdialogs/menus due to collection…
LFDanLu Jan 29, 2025
04a1782
hack around react 16 failures for now
LFDanLu Jan 29, 2025
dbee21e
fix lint
LFDanLu Jan 29, 2025
0198298
Merge branch 'main' of github.com:adobe/react-spectrum into subdialogs
LFDanLu Jan 29, 2025
14e7403
fix react 16, close menu on hovering different item
LFDanLu Jan 29, 2025
b81d9dd
fixing lint
LFDanLu Jan 29, 2025
9b81a2c
wip: refactor to fire fake focus/blur events
devongovett Jan 31, 2025
ca935d7
fix more
devongovett Feb 6, 2025
237b30e
implement keepVisible for non-modal popovers
devongovett Feb 7, 2025
426681e
refactor
devongovett Feb 10, 2025
0dc6d84
update lock
devongovett Feb 10, 2025
4b3bc0c
lint
devongovett Feb 10, 2025
6d5cec9
fix 16
devongovett Feb 10, 2025
e66a8e2
fix test
devongovett Feb 10, 2025
5b49889
Clear MenuContext in SubDialog
devongovett Feb 11, 2025
f6b7272
Fix dismiss on interact outside
devongovett Feb 11, 2025
0b75f85
Use focusedNodeId ref for keyboard handlers to avoid race condition
devongovett Feb 11, 2025
a5573cd
refactor listbox filtering to support usage inside Select
devongovett Feb 11, 2025
65b105e
Make sure props are sent to the right elements
devongovett Feb 12, 2025
77148f5
fix RSP
devongovett Feb 12, 2025
1e394d2
cleanup
devongovett Feb 12, 2025
e03daa4
fix react 16/17
devongovett Feb 12, 2025
30c517c
Revert "fix react 16/17"
devongovett Feb 12, 2025
46c90a0
Revert "fix RSP"
devongovett Feb 12, 2025
057f60c
Revert "Fix dismiss on interact outside"
devongovett Feb 12, 2025
506312f
Fix interact outside for RAC submenu/subdialogs specifically
devongovett Feb 12, 2025
7862d51
Merge branch 'subdialogs-devon' into subdialogs
devongovett Feb 12, 2025
1eaa17c
revert dialog change
devongovett Feb 13, 2025
3fae6be
more cleanup
devongovett Feb 13, 2025
43b3595
Merge branch 'main' of github.com:adobe/react-spectrum into subdialogs
devongovett Feb 13, 2025
3929cef
Use getActiveElement utility
devongovett Feb 13, 2025
7fd803a
focus input when clicking on collection
devongovett Feb 14, 2025
08b8c1a
fix merge
devongovett Feb 14, 2025
dc2dbb4
Refactor left/right arrow key behavior to be less menu specific
devongovett Feb 14, 2025
6d3d377
Remove onDismissButtonPress for now
devongovett Feb 14, 2025
7be3d10
lint
devongovett Feb 14, 2025
3dee094
Merge branch 'main' of github.com:adobe/react-spectrum into subdialogs
devongovett Feb 19, 2025
8f6724f
Merge branch 'main' of github.com:adobe/react-spectrum into subdialogs
devongovett Feb 20, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
diff --git a/dist/cjs/document/prepareDocument.js b/dist/cjs/document/prepareDocument.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi i opened a pr against userEvent to add this patch

index 39a24b8f2ccdc52739d130480ab18975073616cb..0c3f5199401c15b90230c25a02de364eeef3e297 100644
--- a/dist/cjs/document/prepareDocument.js
+++ b/dist/cjs/document/prepareDocument.js
@@ -30,7 +30,7 @@ function prepareDocument(document) {
const initialValue = UI.getInitialValue(el);
if (initialValue !== undefined) {
if (el.value !== initialValue) {
- dispatchEvent.dispatchDOMEvent(el, 'change');
+ el.dispatchEvent(new Event('change'));
}
UI.clearInitialValue(el);
}
diff --git a/dist/cjs/utils/focus/getActiveElement.js b/dist/cjs/utils/focus/getActiveElement.js
index d25f3a8ef67e856e43614559f73012899c0b53d7..4ed9ee45565ed438ee9284d8d3043c0bd50463eb 100644
--- a/dist/cjs/utils/focus/getActiveElement.js
+++ b/dist/cjs/utils/focus/getActiveElement.js
@@ -6,6 +6,8 @@ function getActiveElement(document) {
const activeElement = document.activeElement;
if (activeElement === null || activeElement === undefined ? undefined : activeElement.shadowRoot) {
return getActiveElement(activeElement.shadowRoot);
+ } else if (activeElement && activeElement.tagName === 'IFRAME') {
+ return getActiveElement(activeElement.contentWindow.document);
} else {
// Browser does not yield disabled elements as document.activeElement - jsdom does
if (isDisabled.isDisabled(activeElement)) {
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
"@testing-library/dom": "^10.1.0",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^15.0.7",
"@testing-library/user-event": "^14.6.1",
"@testing-library/user-event": "patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch",
"@types/react": "npm:[email protected]",
"@types/react-dom": "npm:[email protected]",
"@types/storybook__react": "^4.0.2",
Expand Down Expand Up @@ -234,7 +234,8 @@
"@types/react-dom": "npm:[email protected]",
"recast": "0.23.6",
"ast-types": "0.16.1",
"svgo": "^3"
"svgo": "^3",
"@testing-library/user-event@npm:^14.4.0": "patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch"
},
"@parcel/transformer-css": {
"cssModules": {
Expand Down
1 change: 1 addition & 0 deletions packages/@react-aria/autocomplete/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
},
"dependencies": {
"@react-aria/combobox": "^3.11.1",
"@react-aria/focus": "^3.19.1",
"@react-aria/i18n": "^3.12.5",
"@react-aria/interactions": "^3.23.0",
"@react-aria/listbox": "^3.14.0",
Expand Down
133 changes: 97 additions & 36 deletions packages/@react-aria/autocomplete/src/useAutocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
import {AriaLabelingProps, BaseEvent, DOMProps, RefObject} from '@react-types/shared';
import {AriaTextFieldProps} from '@react-aria/textfield';
import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete';
import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, isCtrlKeyPressed, mergeProps, mergeRefs, UPDATE_ACTIVEDESCENDANT, useEffectEvent, useId, useLabels, useObjectRef} from '@react-aria/utils';
import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isCtrlKeyPressed, mergeProps, mergeRefs, useEffectEvent, useId, useLabels, useObjectRef} from '@react-aria/utils';
import {dispatchVirtualBlur, dispatchVirtualFocus, moveVirtualFocus} from '@react-aria/focus';
import {getInteractionModality} from '@react-aria/interactions';
// @ts-ignore
import intlMessages from '../intl/*.json';
import {KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef} from 'react';
import React, {FocusEvent as ReactFocusEvent, KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef} from 'react';
import {useLocalizedStringFormatter} from '@react-aria/i18n';

export interface CollectionOptions extends DOMProps, AriaLabelingProps {
Expand All @@ -34,6 +36,8 @@ export interface AriaAutocompleteProps extends AutocompleteProps {
}

export interface AriaAutocompleteOptions extends Omit<AriaAutocompleteProps, 'children'> {
/** The ref for the wrapped collection element. */
inputRef: RefObject<HTMLInputElement | null>,
/** The ref for the wrapped collection element. */
collectionRef: RefObject<HTMLElement | null>
}
Expand All @@ -57,36 +61,49 @@ export interface AutocompleteAria {
*/
export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state: AutocompleteState): AutocompleteAria {
let {
inputRef,
collectionRef,
filter
} = props;

let collectionId = useId();
let timeout = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
let delayNextActiveDescendant = useRef(false);
let queuedActiveDescendant = useRef(null);
let queuedActiveDescendant = useRef<string | null>(null);
let lastCollectionNode = useRef<HTMLElement>(null);

let updateActiveDescendant = useEffectEvent((e) => {
let {target} = e;
if (queuedActiveDescendant.current === target.id) {
return;
// 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';

useEffect(() => {
return () => clearTimeout(timeout.current);
}, []);

let updateActiveDescendant = useEffectEvent((e: Event) => {
// Ensure input is focused if the user clicks on the collection directly.
if (!e.isTrusted && shouldUseVirtualFocus && inputRef.current && getActiveElement(getOwnerDocument(inputRef.current)) !== inputRef.current) {
inputRef.current.focus();
}

let target = e.target as Element | null;
if (e.isTrusted || !target || 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 {
queuedActiveDescendant.current = target.id;
state.setFocusedNodeId(target.id);
}
} else {
queuedActiveDescendant.current = null;
state.setFocusedNodeId(null);
}

Expand All @@ -96,14 +113,14 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
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
// of the letter you just typed. If we recieve another UPDATE_ACTIVEDESCENDANT call then we clear the queued update
// of the letter you just typed. If we recieve another focus event then we clear the queued update
// We track lastCollectionNode to do proper cleanup since callbackRefs just pass null when unmounting. This also handles
// React 19's extra call of the callback ref in strict mode
lastCollectionNode.current?.removeEventListener(UPDATE_ACTIVEDESCENDANT, updateActiveDescendant);
lastCollectionNode.current?.removeEventListener('focusin', updateActiveDescendant);
lastCollectionNode.current = collectionNode;
collectionNode.addEventListener(UPDATE_ACTIVEDESCENDANT, updateActiveDescendant);
collectionNode.addEventListener('focusin', updateActiveDescendant);
} else {
lastCollectionNode.current?.removeEventListener(UPDATE_ACTIVEDESCENDANT, updateActiveDescendant);
lastCollectionNode.current?.removeEventListener('focusin', updateActiveDescendant);
}
}, [updateActiveDescendant]);

Expand All @@ -123,11 +140,16 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
);
});

let clearVirtualFocus = useEffectEvent(() => {
let clearVirtualFocus = useEffectEvent((clearFocusKey?: boolean) => {
moveVirtualFocus(getActiveElement());
queuedActiveDescendant.current = null;
state.setFocusedNodeId(null);
let clearFocusEvent = new CustomEvent(CLEAR_FOCUS_EVENT, {
cancelable: true,
bubbles: true
bubbles: true,
detail: {
clearFocusKey
}
});
clearTimeout(timeout.current);
delayNextActiveDescendant.current = false;
Expand All @@ -141,7 +163,8 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
if (state.inputValue !== value && state.inputValue.length <= value.length) {
focusFirstItem();
} else {
clearVirtualFocus();
// Fully clear focused key when backspacing since the list may change and thus we'd want to start fresh again
clearVirtualFocus(true);
}

state.setInputValue(value);
Expand All @@ -155,6 +178,7 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
return;
}

let focusedNodeId = queuedActiveDescendant.current;
switch (e.key) {
case 'a':
if (isCtrlKeyPressed(e)) {
Expand Down Expand Up @@ -185,7 +209,7 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
case 'PageUp':
case 'ArrowUp':
case 'ArrowDown': {
if ((e.key === 'Home' || e.key === 'End') && state.focusedNodeId == null && e.shiftKey) {
if ((e.key === 'Home' || e.key === 'End') && focusedNodeId == null && e.shiftKey) {
return;
}

Expand All @@ -200,13 +224,6 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
collectionRef.current?.dispatchEvent(focusCollection);
break;
}
case 'ArrowLeft':
case 'ArrowRight':
// TODO: will need to special case this so it doesn't clear the focused key if we are currently
// focused on a submenutrigger? May not need to since focus would
// But what about wrapped grids where ArrowLeft and ArrowRight should navigate left/right
clearVirtualFocus();
break;
}

// Emulate the keyboard events that happen in the input field in the wrapped collection. This is for triggering things like onAction via Enter
Expand All @@ -217,15 +234,28 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
e.stopPropagation();
}

if (state.focusedNodeId == null) {
collectionRef.current?.dispatchEvent(
let shouldPerformDefaultAction = true;
if (focusedNodeId == null) {
shouldPerformDefaultAction = collectionRef.current?.dispatchEvent(
new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
);
) || false;
} else {
let item = document.getElementById(state.focusedNodeId);
item?.dispatchEvent(
let item = document.getElementById(focusedNodeId);
shouldPerformDefaultAction = item?.dispatchEvent(
new KeyboardEvent(e.nativeEvent.type, e.nativeEvent)
);
) || false;
}

if (shouldPerformDefaultAction) {
switch (e.key) {
case 'ArrowLeft':
case 'ArrowRight': {
// Clear the activedescendant so NVDA announcements aren't interrupted but retain the focused key in the collection so the
// user's keyboard navigation restarts from where they left off
clearVirtualFocus();
break;
}
}
}
};

Expand All @@ -235,12 +265,13 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
// is detected by usePress instead of the original keyup originating from the input
if (e.target === keyDownTarget.current) {
e.stopImmediatePropagation();
if (state.focusedNodeId == null) {
let focusedNodeId = queuedActiveDescendant.current;
if (focusedNodeId == null) {
collectionRef.current?.dispatchEvent(
new KeyboardEvent(e.type, e)
);
} else {
let item = document.getElementById(state.focusedNodeId);
let item = document.getElementById(focusedNodeId);
item?.dispatchEvent(
new KeyboardEvent(e.type, e)
);
Expand Down Expand Up @@ -269,6 +300,34 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
return true;
}, [state.inputValue, filter]);

// Be sure to clear/restore the virtual + collection focus when blurring/refocusing the field so we only show the
// focus ring on the virtually focused collection when are actually interacting with the Autocomplete
let onBlur = (e: ReactFocusEvent) => {
if (!e.isTrusted) {
return;
}

let lastFocusedNode = queuedActiveDescendant.current ? document.getElementById(queuedActiveDescendant.current) : null;
if (lastFocusedNode) {
dispatchVirtualBlur(lastFocusedNode, e.relatedTarget);
}
};

let onFocus = (e: ReactFocusEvent) => {
if (!e.isTrusted) {
return;
}

let curFocusedNode = queuedActiveDescendant.current ? document.getElementById(queuedActiveDescendant.current) : null;
if (curFocusedNode) {
let target = e.target;
queueMicrotask(() => {
dispatchVirtualBlur(target, curFocusedNode);
dispatchVirtualFocus(curFocusedNode, target);
});
}
};

return {
textFieldProps: {
value: state.inputValue,
Expand All @@ -283,11 +342,13 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
// 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'
spellCheck: 'false',
[parseInt(React.version, 10) >= 17 ? 'enterKeyHint' : 'enterkeyhint']: 'enter',
onBlur,
onFocus
},
collectionProps: mergeProps(collectionProps, {
// TODO: shouldFocusOnHover? shouldFocusWrap? Should it be up to the wrapped collection?
shouldUseVirtualFocus: true,
shouldUseVirtualFocus,
disallowTypeAhead: true
}),
collectionRef: mergedCollectionRef,
Expand Down
50 changes: 36 additions & 14 deletions packages/@react-aria/collections/src/BaseCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
let clonedSection: Mutable<CollectionNode<T>> = (node as CollectionNode<T>).clone();
let lastChildInSection: Mutable<CollectionNode<T>> | null = null;
for (let child of this.getChildren(node.key)) {
if (filterFn(child.textValue) || child.type === 'header') {
if (shouldKeepNode(child, filterFn, this, newCollection)) {
let clonedChild: Mutable<CollectionNode<T>> = (child as CollectionNode<T>).clone();
// eslint-disable-next-line max-depth
if (lastChildInSection == null) {
Expand Down Expand Up @@ -288,22 +288,25 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
lastNode = clonedSeparator;
newCollection.addNode(clonedSeparator);
}
} else if (filterFn(node.textValue)) {
} else {
// At this point, the node is either a subdialogtrigger node or a standard row/item
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weirdly specific comment about subdialogtriggers, can we generalize a little more of what is going on?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this whole method has had a TODO comment above it for a while.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can refactor this to move a lot of this logic into Menu/ListBox, but I want to do that separately and not block this PR any further.

let clonedNode: Mutable<CollectionNode<T>> = (node as CollectionNode<T>).clone();
if (newCollection.firstKey == null) {
newCollection.firstKey = clonedNode.key;
}
if (shouldKeepNode(clonedNode, filterFn, this, newCollection)) {
if (newCollection.firstKey == null) {
newCollection.firstKey = clonedNode.key;
}

if (lastNode != null && (lastNode.type !== 'section' && lastNode.type !== 'separator') && lastNode.parentKey === clonedNode.parentKey) {
lastNode.nextKey = clonedNode.key;
clonedNode.prevKey = lastNode.key;
} else {
clonedNode.prevKey = null;
}
if (lastNode != null && (lastNode.type !== 'section' && lastNode.type !== 'separator') && lastNode.parentKey === clonedNode.parentKey) {
lastNode.nextKey = clonedNode.key;
clonedNode.prevKey = lastNode.key;
} else {
clonedNode.prevKey = null;
}

clonedNode.nextKey = null;
newCollection.addNode(clonedNode);
lastNode = clonedNode;
clonedNode.nextKey = null;
newCollection.addNode(clonedNode);
lastNode = clonedNode;
}
}
}

Expand All @@ -322,3 +325,22 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
return newCollection;
}
}

function shouldKeepNode<T>(node: Node<T>, filterFn: (nodeValue: string) => boolean, oldCollection: BaseCollection<T>, newCollection: BaseCollection<T>): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really follow what this function is for, can we get some comments

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely can do when we settle on the form of the collection node filtering. For now, this function basically checks if a given node in the collection should be retained based on its text value versus the filterFn that the user has provided (aka typical case is that the filterFn should return true if the current autocomplete string is a part of the node's text value).

There is extra logic in here specifically for the subdialogtrigger/submenutrigger case where we need to look up the first child of those nodes (aka the wrapped menu item) to extract the text value associated with the trigger. Said child also then needs to be added to the newCollection (aka the filtered collection), otherwise we'd only be adding the wrapper node

if (node.type === 'subdialogtrigger' || node.type === 'submenutrigger') {
// Subdialog wrapper should only have one child, if it passes the filter add it to the new collection since we don't need to
// do any extra handling for its first/next key
let triggerChild = [...oldCollection.getChildren(node.key)][0];
if (triggerChild && filterFn(triggerChild.textValue)) {
let clonedChild: Mutable<CollectionNode<T>> = (triggerChild as CollectionNode<T>).clone();
newCollection.addNode(clonedChild);
return true;
} else {
return false;
}
} else if (node.type === 'header') {
return true;
} else {
return filterFn(node.textValue);
}
}
4 changes: 3 additions & 1 deletion packages/@react-aria/dialog/src/useDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ export interface DialogAria {
* A dialog is an overlay shown above other content in an application.
*/
export function useDialog(props: AriaDialogProps, ref: RefObject<FocusableElement | null>): DialogAria {
let {role = 'dialog'} = props;
let {
role = 'dialog'
} = props;
let titleId: string | undefined = useSlotId();
titleId = props['aria-label'] ? undefined : titleId;

Expand Down
Loading