From 27eae61f76732be341d5902cf87907e81ffed5f8 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 4 Jun 2026 11:09:10 -0700 Subject: [PATCH 01/12] initial changes to support textfields in gridlists --- .../stories/GridList.stories.tsx | 120 ++++++++++++++- .../stories/Tree.stories.tsx | 139 +++++++++++++++++- .../test/GridList.test.js | 54 +++++++ .../src/gridlist/useGridListItem.ts | 64 +++++++- packages/react-aria/src/select/useSelect.ts | 2 - .../react-aria/src/selection/useTypeSelect.ts | 6 +- 6 files changed, 372 insertions(+), 13 deletions(-) diff --git a/packages/react-aria-components/stories/GridList.stories.tsx b/packages/react-aria-components/stories/GridList.stories.tsx index 7c42b76baf4..5a5b8629843 100644 --- a/packages/react-aria-components/stories/GridList.stories.tsx +++ b/packages/react-aria-components/stories/GridList.stories.tsx @@ -28,10 +28,12 @@ import { GridListSection } from '../src/GridList'; import {Heading} from '../src/Heading'; +import {Input} from '../src/Input'; import {Key} from '@react-types/shared'; import {ListLayout, Size, WaterfallLayout} from 'react-stately/useVirtualizerState'; import {LoadingSpinner} from './utils'; import {LoadingState} from '@react-types/shared'; +import {Menu, MenuItem, MenuTrigger} from '../src/Menu'; import {Meta, StoryFn, StoryObj} from '@storybook/react'; import {Modal, ModalOverlay, ModalOverlayProps} from '../src/Modal'; import {Popover} from '../src/Popover'; @@ -39,6 +41,8 @@ import React, {JSX, useState} from 'react'; import styles from '../example/index.css'; import {Tag, TagGroup, TagList} from '../src/TagGroup'; import {Text} from '../src/Text'; +import {TextField} from '../src/TextField'; +import {Toolbar} from '../src/Toolbar'; import {useAsyncList} from 'react-stately/useAsyncList'; import {useListData} from 'react-stately/useListData'; import {Virtualizer} from '../src/Virtualizer'; @@ -312,13 +316,21 @@ GridListSectionExample.story = { }; export function VirtualizedGridListSection() { - let sections: {id: string; name: string; children: {id: string; name: string}[]}[] = []; + let sections: { + id: string; + name: string; + children: {id: string; name: string}[]; + }[] = []; for (let s = 0; s < 10; s++) { let items: {id: string; name: string}[] = []; for (let i = 0; i < 3; i++) { items.push({id: `item_${s}_${i}`, name: `Section ${s}, Item ${i}`}); } - sections.push({id: `section_${s}`, name: `Section ${s}`, children: items}); + sections.push({ + id: `section_${s}`, + name: `Section ${s}`, + children: items + }); } return ( @@ -360,7 +372,9 @@ const VirtualizedGridListRender = (args: GridListProps & {isLoading: boolea let {dragAndDropHooks} = useDragAndDrop({ getItems: keys => { - return [...keys].map(key => ({'text/plain': list.getItem(key)?.name ?? ''})); + return [...keys].map(key => ({ + 'text/plain': list.getItem(key)?.name ?? '' + })); }, onReorder(e) { if (e.target.dropPosition === 'before') { @@ -953,3 +967,103 @@ export const AsyncGridListGridVirtualized: StoryObj { + let isHorizontalStack = args.orientation === 'horizontal' && args.layout !== 'grid'; + return ( + <> + + + + RAC TextField + + + + + + Raw input + + + TextField + Button + + + {' '} + + + + Toolbar + + + + + + + + Menu + {/* TODO: hitting escape to close the menu, returns focus to the row. + Tabbing back from the external input also focuses the trggerbutton rather than the row. Tabbing back into the textfield row focuses the row */} + + + + + Cut + Copy + Paste + + + + + + + + ); +}; + +GridListWithTextfield.story = { + args: { + layout: 'stack', + orientation: 'vertical', + escapeKeyBehavior: 'clearSelection', + shouldSelectOnPressUp: false, + disallowTypeAhead: false + }, + argTypes: { + layout: { + control: 'radio', + options: ['stack', 'grid'] + }, + orientation: { + control: 'radio', + options: ['vertical', 'horizontal'] + }, + keyboardNavigationBehavior: { + control: 'radio', + options: ['arrow', 'tab'] + }, + selectionMode: { + control: 'radio', + options: ['none', 'single', 'multiple'] + }, + selectionBehavior: { + control: 'radio', + options: ['toggle', 'replace'] + }, + escapeKeyBehavior: { + control: 'radio', + options: ['clearSelection', 'none'] + } + } +}; diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 4e305216973..4899aaac152 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -16,17 +16,18 @@ import {Checkbox, CheckboxProps} from '../src/Checkbox'; import {classNames} from '@adobe/react-spectrum/private/utils/classNames'; import {Collection} from 'react-aria/Collection'; import {DroppableCollectionReorderEvent, Key} from '@react-types/shared'; +import {Input} from '../src/Input'; import {isTextDropItem, useDragAndDrop} from '../exports/useDragAndDrop'; import {ListLayout} from 'react-stately/useVirtualizerState'; -import {Menu, MenuTrigger} from '../src/Menu'; - +import {Menu, MenuItem, MenuTrigger} from '../src/Menu'; import {Meta, StoryFn, StoryObj} from '@storybook/react'; - import {MyMenuItem} from './utils'; import {Popover} from '../src/Popover'; import React, {JSX, ReactNode, useCallback, useState} from 'react'; import styles from '../example/index.css'; import {Text} from '../src/Text'; +import {TextField} from '../src/TextField'; +import {Toolbar} from '../src/Toolbar'; import { Tree, TreeHeader, @@ -54,6 +55,7 @@ export type TreeStory = StoryFn; interface StaticTreeItemProps extends TreeItemProps { title?: string; children: ReactNode; + interactive?: ReactNode; } interface MyCheckboxProps extends CheckboxProps { @@ -117,6 +119,7 @@ const StaticTreeItem = (props: StaticTreeItemProps) => { )} {props.title || props.children} + {props.interactive} @@ -1801,3 +1804,133 @@ export const HugeVirtualizedTree: StoryObj = { }, render: args => }; + +// TODO: bugs to investigate +// clicking on the textfield and hitting space in the textfield when selection is enabled causes selection to be toggled +// cant add spaces in the parent rows textfield? +function TreeWithTextField(props: TreeProps) { + return ( + <> + + + + + + }> + RAC TextField + + }> + Raw input + + + + + }> + + + + + + + }> + TextField + Button + + + + + + + }> + Toolbar + + }> + }> + Nested child 1 + + + + + + Cut + Copy + Paste + + + + }> + Nested child 2 + + + + + + + ); +} + +export const TreeWithTextFieldStory: StoryObj = { + render: args => , + args: { + selectionMode: 'none', + selectionBehavior: 'toggle', + disabledBehavior: 'selection' + }, + argTypes: { + // TODO: add later + // keyboardNavigationBehavior: { + // control: 'radio', + // options: ['arrow', 'tab'] + // }, + selectionMode: { + control: 'radio', + options: ['none', 'single', 'multiple'] + }, + selectionBehavior: { + control: 'radio', + options: ['toggle', 'replace'] + }, + disabledBehavior: { + control: 'radio', + options: ['selection', 'all'] + } + }, + name: 'Tree with Textfield' +}; diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 5875fe1bd51..8fb5a7707c4 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -1145,6 +1145,60 @@ describe('GridList', () => { expect(document.activeElement).toBe(items[0]); }); + it('should not navigate rows when arrow keys are pressed while a text input child has focus', async () => { + let {getAllByRole, getByRole} = render( + + + Apple + + Banana + + ); + + let rows = getAllByRole('row'); + let input = getByRole('textbox'); + + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard('{ArrowDown}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowUp}'); + expect(document.activeElement).toBe(input); + }); + + it('should not trigger typeahead when typing in a text input child', async () => { + let {getAllByRole, getByRole} = render( + + + Apple + + + Banana + + + ); + + let rows = getAllByRole('row'); + let input = getByRole('textbox'); + + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard('b'); + expect(document.activeElement).toBe(input); + expect(input).toHaveValue('b'); + }); + + it('should not trigger selection when typing in the text input child', async () => { + // TODO: implement, right now it will select + }); + it('should not propagate the checkbox context from selection into other cells', async () => { let tree = render( diff --git a/packages/react-aria/src/gridlist/useGridListItem.ts b/packages/react-aria/src/gridlist/useGridListItem.ts index d70ea835356..194a8cbcf10 100644 --- a/packages/react-aria/src/gridlist/useGridListItem.ts +++ b/packages/react-aria/src/gridlist/useGridListItem.ts @@ -310,7 +310,7 @@ export function useGridListItem( } }; - let onKeyDown = e => { + let onKeyDown = (e: ReactKeyboardEvent) => { let activeElement = getActiveElement(); if ( !nodeContains(e.currentTarget, getEventTarget(e) as Element) || @@ -320,6 +320,57 @@ export function useGridListItem( return; } + if (keyboardNavigationBehavior === 'tab') { + // TODO: try out Rob's useTypeSelect change in place of this stop propagation for character keys? + // will still need to stop arrow key propagation otherwise useSelectableCollection recieves the event and moves focus + // should it just stop propagation for all events? + if (activeElement !== ref.current) { + if ( + isArrowKey(e.key) || + isCharacterKey(e.key) || + (e.key === '' && state.selectionManager.selectionMode !== 'none') + ) { + e.stopPropagation(); + return; + } + } + + // TODO: for tree expansion since we turn off the capturing listener if keyboardNavigationBehavior = tab + // copied from above, extract into helper later + // need to support tab keyboard navigation in tree + if ('expandedKeys' in state && activeElement === ref.current) { + if ( + e.key === EXPANSION_KEYS['expand'][direction] && + state.selectionManager.focusedKey === node.key && + hasChildRows && + !state.expandedKeys.has(node.key) + ) { + state.toggleKey(node.key); + e.stopPropagation(); + return; + } else if ( + e.key === EXPANSION_KEYS['collapse'][direction] && + state.selectionManager.focusedKey === node.key + ) { + // If item is collapsible, collapse it; else move to parent + if (hasChildRows && state.expandedKeys.has(node.key)) { + state.toggleKey(node.key); + e.stopPropagation(); + return; + } else if ( + !state.expandedKeys.has(node.key) && + node.parentKey && + state.collection.getItem(node.parentKey)?.type === 'item' + ) { + // Item is a leaf or already collapsed, move focus to parent + state.selectionManager.setFocusedKey(node.parentKey); + e.stopPropagation(); + return; + } + } + } + } + switch (e.key) { case 'Tab': { if (keyboardNavigationBehavior === 'tab') { @@ -351,7 +402,7 @@ export function useGridListItem( let rowProps: DOMAttributes = mergeProps(itemProps, linkProps, { role: 'row', - onKeyDownCapture, + onKeyDownCapture: keyboardNavigationBehavior === 'arrow' ? onKeyDownCapture : undefined, onKeyDown, onFocus, // 'aria-label': [(node.textValue || undefined), rowAnnouncement].filter(Boolean).join(', '), @@ -419,3 +470,12 @@ function getDirectChildren(parent: RSNode, collection: Collection( errorMessage: props.errorMessage || validationErrors }); - typeSelectProps.onKeyDown = typeSelectProps.onKeyDownCapture; - delete typeSelectProps.onKeyDownCapture; if (state.selectionManager.selectionMode === 'multiple') { typeSelectProps = {}; } diff --git a/packages/react-aria/src/selection/useTypeSelect.ts b/packages/react-aria/src/selection/useTypeSelect.ts index eae1215dad4..21334c58f92 100644 --- a/packages/react-aria/src/selection/useTypeSelect.ts +++ b/packages/react-aria/src/selection/useTypeSelect.ts @@ -103,9 +103,9 @@ export function useTypeSelect(options: AriaTypeSelectOptions): TypeSelectAria { return { typeSelectProps: { - // Using a capturing listener to catch the keydown event before - // other hooks in order to handle the Spacebar event. - onKeyDownCapture: keyboardDelegate.getKeyForSearch ? onKeyDown : undefined + // TODO: now that this is not capturing, will need to make sure other collection components/hooks + // work properly with it (aka now space for selection will alway take priority) + onKeyDown: keyboardDelegate.getKeyForSearch ? onKeyDown : undefined } }; } From 5ab7678a633680629bae7a03b1d61f1e25498830 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 4 Jun 2026 15:51:27 -0700 Subject: [PATCH 02/12] use robs event leak type select and prevent space from triggering selection when in input field --- .../stories/GridList.stories.tsx | 12 +-- .../stories/Tree.stories.tsx | 3 +- .../test/GridList.test.js | 36 ++++++- .../test/TagGroup.test.js | 5 + .../src/gridlist/useGridListItem.ts | 34 ++++--- .../react-aria/src/selection/useTypeSelect.ts | 98 ++++++++++++++----- 6 files changed, 140 insertions(+), 48 deletions(-) diff --git a/packages/react-aria-components/stories/GridList.stories.tsx b/packages/react-aria-components/stories/GridList.stories.tsx index 5a5b8629843..9dc48496b6e 100644 --- a/packages/react-aria-components/stories/GridList.stories.tsx +++ b/packages/react-aria-components/stories/GridList.stories.tsx @@ -969,7 +969,7 @@ export const AsyncGridListGridVirtualized: StoryObj { let isHorizontalStack = args.orientation === 'horizontal' && args.layout !== 'grid'; return ( @@ -987,23 +987,23 @@ export const GridListWithTextfield: GridListStory = args => { gridAutoFlow: args.orientation === 'horizontal' ? 'column' : 'row' }} {...args}> - + RAC TextField - + Raw input - + TextField + Button {' '} - + Toolbar @@ -1011,7 +1011,7 @@ export const GridListWithTextfield: GridListStory = args => { - + Menu {/* TODO: hitting escape to close the menu, returns focus to the row. Tabbing back from the external input also focuses the trggerbutton rather than the row. Tabbing back into the textfield row focuses the row */} diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 4899aaac152..5ca6401c28b 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -1806,8 +1806,7 @@ export const HugeVirtualizedTree: StoryObj = { }; // TODO: bugs to investigate -// clicking on the textfield and hitting space in the textfield when selection is enabled causes selection to be toggled -// cant add spaces in the parent rows textfield? +// clicking on the textfield when selection is enabled causes selection to be toggled function TreeWithTextField(props: TreeProps) { return ( <> diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 8fb5a7707c4..367445d83fa 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -1148,10 +1148,12 @@ describe('GridList', () => { it('should not navigate rows when arrow keys are pressed while a text input child has focus', async () => { let {getAllByRole, getByRole} = render( - + Apple - Banana + + Banana + ); @@ -1195,8 +1197,34 @@ describe('GridList', () => { expect(input).toHaveValue('b'); }); - it('should not trigger selection when typing in the text input child', async () => { - // TODO: implement, right now it will select + it('should not trigger selection when pressing Space in a text input child', async () => { + using onSelectionChange = jest.fn(); + let {getAllByRole, getByRole} = render( + + + Apple + + + Banana + + + ); + + let rows = getAllByRole('row'); + let input = getByRole('textbox'); + + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard(' '); + expect(input).toHaveValue(' '); + expect(onSelectionChange).not.toHaveBeenCalled(); }); it('should not propagate the checkbox context from selection into other cells', async () => { diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index e94c84b7686..8f615682656 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -660,6 +660,11 @@ describe('TagGroup', () => { expect(onRemove).toHaveBeenCalledTimes(2); expect(onRemove).toHaveBeenLastCalledWith(new Set(['cat'])); + // TODO: a change in behavior since taggroup is a gridlist with "tab" keyboard navigation behavior + // previously you could go to the next tab via arrow keys when you were focused on the close button + await user.keyboard('{Shift>}{Tab}{/Shift}'); + expect(tags[0]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); expect(tags[1]).toHaveFocus(); diff --git a/packages/react-aria/src/gridlist/useGridListItem.ts b/packages/react-aria/src/gridlist/useGridListItem.ts index 194a8cbcf10..48df2ee6550 100644 --- a/packages/react-aria/src/gridlist/useGridListItem.ts +++ b/packages/react-aria/src/gridlist/useGridListItem.ts @@ -321,18 +321,17 @@ export function useGridListItem( } if (keyboardNavigationBehavior === 'tab') { - // TODO: try out Rob's useTypeSelect change in place of this stop propagation for character keys? - // will still need to stop arrow key propagation otherwise useSelectableCollection recieves the event and moves focus - // should it just stop propagation for all events? - if (activeElement !== ref.current) { - if ( - isArrowKey(e.key) || - isCharacterKey(e.key) || - (e.key === '' && state.selectionManager.selectionMode !== 'none') - ) { - e.stopPropagation(); - return; - } + // TODO: Added Rob's useTypeSelect changes, but that only stops if type select is in progress + // This will stop arrow key navigation and typeselect from bubbling up + // (note that this breaks TagGroup's old behavior of using arrow keys to move from "x" button to next tag and typeselect when inside a card/row) + // should it just stop propagation for all events since we can't rely on non-RAC components stopping propagation even they handled the event + // Will need to do something similar for click? + if ( + activeElement !== ref.current && + (isArrowKey(e.key) || isCharacterKey(e.key) || e.key === 'Enter') + ) { + e.stopPropagation(); + return; } // TODO: for tree expansion since we turn off the capturing listener if keyboardNavigationBehavior = tab @@ -403,7 +402,6 @@ export function useGridListItem( let rowProps: DOMAttributes = mergeProps(itemProps, linkProps, { role: 'row', onKeyDownCapture: keyboardNavigationBehavior === 'arrow' ? onKeyDownCapture : undefined, - onKeyDown, onFocus, // 'aria-label': [(node.textValue || undefined), rowAnnouncement].filter(Boolean).join(', '), 'aria-label': node['aria-label'] || node.textValue || undefined, @@ -418,6 +416,16 @@ export function useGridListItem( id: getRowId(state, node.key) }); + // TODO we need to guard against space/enter triggering selection/row link via usePress (from itemProps) so check if propagation + // is stopped. this also fixes space not working in a textfield in a tree parent row + let baseOnKeyDown = rowProps.onKeyDown; + rowProps.onKeyDown = (e: ReactKeyboardEvent) => { + onKeyDown(e as ReactKeyboardEvent); + if (!e.isPropagationStopped()) { + baseOnKeyDown?.(e); + } + }; + if (isVirtualized) { let {collection} = state; let nodes = [...collection]; diff --git a/packages/react-aria/src/selection/useTypeSelect.ts b/packages/react-aria/src/selection/useTypeSelect.ts index 21334c58f92..ad7e00d4304 100644 --- a/packages/react-aria/src/selection/useTypeSelect.ts +++ b/packages/react-aria/src/selection/useTypeSelect.ts @@ -12,7 +12,7 @@ import {DOMAttributes, Key, KeyboardDelegate} from '@react-types/shared'; import {getEventTarget, nodeContains} from '../utils/shadowdom/DOMFunctions'; -import {KeyboardEvent, useRef} from 'react'; +import {KeyboardEvent, useEffect, useRef} from 'react'; import {MultipleSelectionManager} from 'react-stately/useMultipleSelectionState'; /** @@ -50,41 +50,75 @@ export function useTypeSelect(options: AriaTypeSelectOptions): TypeSelectAria { let state = useRef<{search: string; timeout: ReturnType | undefined}>({ search: '', timeout: undefined - }).current; + }); + + let onKeyDownCapture = (e: KeyboardEvent) => { + // if we're in the middle of a search, then a spacebar should be treated as a search and we should not propagate the event + // since we handle this one in a capture phase, we should ignore it in the bubble phase + if (state.current.search.length > 0 && e.key === ' ') { + e.preventDefault(); + if ( + !('continuePropagation' in e) || + ('continuePropagation' in e && !e.isPropagationStopped()) + ) { + e.stopPropagation(); + } + state.current.search += ' '; + + if (keyboardDelegate.getKeyForSearch != null) { + // Use the delegate to find a key to focus. + // Prioritize items after the currently focused item, falling back to searching the whole list. + let key = keyboardDelegate.getKeyForSearch( + state.current.search, + selectionManager.focusedKey + ); + + // If no key found, search from the top. + if (key == null) { + key = keyboardDelegate.getKeyForSearch(state.current.search); + } + + if (key != null) { + selectionManager.setFocusedKey(key); + if (onTypeSelect) { + onTypeSelect(key); + } + } + } + + clearTimeout(state.current.timeout); + state.current.timeout = setTimeout(() => { + state.current.search = ''; + }, TYPEAHEAD_DEBOUNCE_WAIT_MS); + } + }; let onKeyDown = (e: KeyboardEvent) => { + if (e.altKey) { + return; + } + let character = getStringForKey(e.key); if ( !character || e.ctrlKey || e.metaKey || + e.altKey || !nodeContains(e.currentTarget, getEventTarget(e) as HTMLElement) || - (state.search.length === 0 && character === ' ') + (state.current.search.length === 0 && character === ' ') ) { return; } - // Do not propagate the Spacebar event if it's meant to be part of the search. - // When we time out, the search term becomes empty, hence the check on length. - // Trimming is to account for the case of pressing the Spacebar more than once, - // which should cycle through the selection/deselection of the focused item. - if (character === ' ' && state.search.trim().length > 0) { - e.preventDefault(); - if (!('continuePropagation' in e)) { - e.stopPropagation(); - } - } - - state.search += character; + state.current.search += character; if (keyboardDelegate.getKeyForSearch != null) { // Use the delegate to find a key to focus. // Prioritize items after the currently focused item, falling back to searching the whole list. - let key = keyboardDelegate.getKeyForSearch(state.search, selectionManager.focusedKey); + let key = keyboardDelegate.getKeyForSearch(state.current.search, selectionManager.focusedKey); - // If no key found, search from the top. if (key == null) { - key = keyboardDelegate.getKeyForSearch(state.search); + key = keyboardDelegate.getKeyForSearch(state.current.search); } if (key != null) { @@ -92,19 +126,37 @@ export function useTypeSelect(options: AriaTypeSelectOptions): TypeSelectAria { if (onTypeSelect) { onTypeSelect(key); } + e.preventDefault(); + if (!('continuePropagation' in e)) { + e.stopPropagation(); + } + } else { + // if still nothing then the type to select is done and everything is reset + state.current.search = ''; + clearTimeout(state.current.timeout); + state.current.timeout = undefined; + return; } } - clearTimeout(state.timeout); - state.timeout = setTimeout(() => { - state.search = ''; + clearTimeout(state.current.timeout); + state.current.timeout = setTimeout(() => { + state.current.search = ''; }, TYPEAHEAD_DEBOUNCE_WAIT_MS); }; + useEffect(() => { + let timeout = state.current.timeout; + return () => { + clearTimeout(timeout); + }; + }, [state]); + return { typeSelectProps: { - // TODO: now that this is not capturing, will need to make sure other collection components/hooks - // work properly with it (aka now space for selection will alway take priority) + // Using a capturing listener to catch the keydown event before + // other hooks in order to handle the Spacebar event. + onKeyDownCapture: keyboardDelegate.getKeyForSearch ? onKeyDownCapture : undefined, onKeyDown: keyboardDelegate.getKeyForSearch ? onKeyDown : undefined } }; From 9227d33c2c3ca228bec022912bfbc1972b7e7fa9 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 4 Jun 2026 16:28:49 -0700 Subject: [PATCH 03/12] fix tests, but also use a more robust check instead of active element the failing tests didnt focus the element when triggering a keypress --- packages/react-aria/src/gridlist/useGridListItem.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-aria/src/gridlist/useGridListItem.ts b/packages/react-aria/src/gridlist/useGridListItem.ts index 48df2ee6550..d27bac3fcad 100644 --- a/packages/react-aria/src/gridlist/useGridListItem.ts +++ b/packages/react-aria/src/gridlist/useGridListItem.ts @@ -327,7 +327,7 @@ export function useGridListItem( // should it just stop propagation for all events since we can't rely on non-RAC components stopping propagation even they handled the event // Will need to do something similar for click? if ( - activeElement !== ref.current && + getEventTarget(e) !== ref.current && (isArrowKey(e.key) || isCharacterKey(e.key) || e.key === 'Enter') ) { e.stopPropagation(); From d1ccfa6328a9d11db5ff2731740db6c4a3cb6fe3 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 5 Jun 2026 10:33:37 -0700 Subject: [PATCH 04/12] add S2 Card story and tests for tree and gridlist --- .../s2/stories/CardView.stories.tsx | 42 +++- .../stories/Tree.stories.tsx | 2 +- .../test/GridList.test.js | 187 ++++++++++-------- .../react-aria-components/test/Tree.test.tsx | 89 ++++++++- 4 files changed, 230 insertions(+), 90 deletions(-) diff --git a/packages/@react-spectrum/s2/stories/CardView.stories.tsx b/packages/@react-spectrum/s2/stories/CardView.stories.tsx index e5efd0212a4..2f00aba5d14 100644 --- a/packages/@react-spectrum/s2/stories/CardView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/CardView.stories.tsx @@ -27,6 +27,7 @@ import {MenuItem} from '../src/Menu'; import type {Meta, StoryObj} from '@storybook/react'; import {SkeletonCollection} from '../src/SkeletonCollection'; import {style} from '../style/spectrum-theme' with {type: 'macro'}; +import {TextField} from '../src/TextField'; import {useAsyncList} from 'react-stately/useAsyncList'; const meta: Meta = { @@ -72,7 +73,15 @@ const avatarSize = { XL: 32 } as const; -export function PhotoCard({item, layout}: {item: Item; layout: string}) { +export function PhotoCard({ + item, + layout, + interactive +}: { + item: Item; + layout: string; + interactive?: React.ReactNode; +}) { return ( {({size}) => ( @@ -112,12 +121,20 @@ export function PhotoCard({item, layout}: {item: Item; layout: string}) {
- - {item.user.name} +
+ + {item.user.name} +
+ {interactive}
@@ -126,7 +143,7 @@ export function PhotoCard({item, layout}: {item: Item; layout: string}) { ); } -export const ExampleRender = (args: CardViewProps) => { +export const ExampleRender = (args: CardViewProps & {interactive?: React.ReactNode}) => { let list = useAsyncList({ async load({signal, cursor, items}) { let page = cursor || 1; @@ -155,7 +172,9 @@ export const ExampleRender = (args: CardViewProps) => { onLoadMore={args.loadingState === 'idle' ? list.loadMore : undefined} styles={cardViewStyles}> - {item => } + {item => ( + + )} {(loadingState === 'loading' || loadingState === 'loadingMore') && ( @@ -288,3 +307,14 @@ export const CollectionCards: Story = { onAction: undefined } }; + +export const CardViewWithTextField: Story = { + render: args => ( + } /> + ), + args: { + loadingState: 'idle', + onAction: undefined, + selectionMode: 'multiple' + } +}; diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 5ca6401c28b..3d192f14c54 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -1807,7 +1807,7 @@ export const HugeVirtualizedTree: StoryObj = { // TODO: bugs to investigate // clicking on the textfield when selection is enabled causes selection to be toggled -function TreeWithTextField(props: TreeProps) { +export function TreeWithTextField(props: TreeProps) { return ( <> diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 367445d83fa..2ec0df5d91a 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -1145,88 +1145,6 @@ describe('GridList', () => { expect(document.activeElement).toBe(items[0]); }); - it('should not navigate rows when arrow keys are pressed while a text input child has focus', async () => { - let {getAllByRole, getByRole} = render( - - - Apple - - - Banana - - - ); - - let rows = getAllByRole('row'); - let input = getByRole('textbox'); - - await user.tab(); - expect(document.activeElement).toBe(rows[0]); - - await user.tab(); - expect(document.activeElement).toBe(input); - - await user.keyboard('{ArrowDown}'); - expect(document.activeElement).toBe(input); - await user.keyboard('{ArrowUp}'); - expect(document.activeElement).toBe(input); - }); - - it('should not trigger typeahead when typing in a text input child', async () => { - let {getAllByRole, getByRole} = render( - - - Apple - - - Banana - - - ); - - let rows = getAllByRole('row'); - let input = getByRole('textbox'); - - await user.tab(); - expect(document.activeElement).toBe(rows[0]); - await user.tab(); - expect(document.activeElement).toBe(input); - - await user.keyboard('b'); - expect(document.activeElement).toBe(input); - expect(input).toHaveValue('b'); - }); - - it('should not trigger selection when pressing Space in a text input child', async () => { - using onSelectionChange = jest.fn(); - let {getAllByRole, getByRole} = render( - - - Apple - - - Banana - - - ); - - let rows = getAllByRole('row'); - let input = getByRole('textbox'); - - await user.tab(); - expect(document.activeElement).toBe(rows[0]); - await user.tab(); - expect(document.activeElement).toBe(input); - - await user.keyboard(' '); - expect(input).toHaveValue(' '); - expect(onSelectionChange).not.toHaveBeenCalled(); - }); - it('should not propagate the checkbox context from selection into other cells', async () => { let tree = render( @@ -1915,4 +1833,109 @@ describe('GridList', () => { } ); }); + + describe('tab navigation and textfields', () => { + it.each([ + ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], + ['layout="grid"', {layout: 'grid'}] + ])( + 'should not navigate rows when arrow keys are pressed while a text input child has focus (%s)', + async (_, listProps) => { + let {getAllByRole, getByRole} = render( + + + Apple + + + Banana + + + ); + + let rows = getAllByRole('row'); + let input = getByRole('textbox'); + + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard('{ArrowDown}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowUp}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowRight}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowLeft}'); + expect(document.activeElement).toBe(input); + } + ); + + it.each([ + ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], + ['layout="grid"', {layout: 'grid'}] + ])( + 'should not trigger typeahead when typing in a text input child (%s)', + async (_, listProps) => { + let {getAllByRole, getByRole} = render( + + + Apple + + + Banana + + + ); + + let rows = getAllByRole('row'); + let input = getByRole('textbox'); + + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard('b'); + expect(document.activeElement).toBe(input); + expect(input).toHaveValue('b'); + } + ); + + it.each([ + ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], + ['layout="grid"', {layout: 'grid'}] + ])( + 'should not trigger selection when pressing Space in a text input child (%s)', + async (_, listProps) => { + let onSelectionChange = jest.fn(); + let {getAllByRole, getByRole} = render( + + + Apple + + + Banana + + + ); + + let rows = getAllByRole('row'); + let input = getByRole('textbox'); + + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard(' '); + expect(input).toHaveValue(' '); + expect(onSelectionChange).not.toHaveBeenCalled(); + } + ); + }); }); diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index d9072b35a17..2e0f634ad75 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -47,7 +47,8 @@ import {Virtualizer} from '../src/Virtualizer'; let { EmptyTreeStaticStory: EmptyLoadingTree, LoadingStoryDepOnTopStory: LoadingMoreTree, - TreeWithDragAndDrop + TreeWithDragAndDrop, + TreeWithTextFieldStory } = composeStories(stories); let onSelectionChange = jest.fn(); @@ -2838,6 +2839,92 @@ describe('Tree', () => { expect(rows[18]).toHaveAttribute('aria-posinset', '1'); expect(rows[18]).toHaveAttribute('aria-setsize', '1'); }); + + describe('tab navigation and textfields', () => { + it('should not navigate rows when arrow keys are pressed while a text input child has focus', async () => { + let {getByRole} = render(); + let treeTester = testUtilUser.createTester('Tree', {root: getByRole('treegrid')}); + let rows = treeTester.getRows(); + let input = getByRole('textbox', {name: 'Name'}); + + // tab past the before tree input + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard('{ArrowDown}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowUp}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowRight}'); + expect(document.activeElement).toBe(input); + await user.keyboard('{ArrowLeft}'); + expect(document.activeElement).toBe(input); + }); + + it('should not trigger typeahead when typing in a text input child', async () => { + let {getByRole} = render(); + let treeTester = testUtilUser.createTester('Tree', {root: getByRole('treegrid')}); + let rows = treeTester.getRows(); + let input = getByRole('textbox', {name: 'Name'}); + + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard('row'); + expect(document.activeElement).toBe(input); + expect(input).toHaveValue('row'); + }); + + it('should not trigger selection when pressing Space in a text input child of a leaf row', async () => { + let onSelectionChange = jest.fn(); + let {getByRole} = render( + + ); + let treeTester = testUtilUser.createTester('Tree', {root: getByRole('treegrid')}); + let rows = treeTester.getRows(); + let input = getByRole('textbox', {name: 'Name'}); + + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(input); + + await user.keyboard(' '); + expect(input).toHaveValue(' '); + expect(onSelectionChange).not.toHaveBeenCalled(); + }); + + it('should allow typing space in the text input child of a parent row', async () => { + let onSelectionChange = jest.fn(); + let {getByRole} = render( + + ); + let treeTester = testUtilUser.createTester('Tree', {root: getByRole('treegrid')}); + let rows = treeTester.getRows(); + + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(rows[0]); + await user.keyboard('{ArrowDown}'); + await user.keyboard('{ArrowDown}'); + await user.tab(); + await user.tab(); + let parentInput = getByRole('textbox', {name: 'row 1 input'}); + expect(document.activeElement).toBe(parentInput); + + await user.keyboard(' '); + expect(parentInput).toHaveValue(' '); + expect(onSelectionChange).not.toHaveBeenCalled(); + }); + }); }); AriaTreeTests({ From cc8e6b1fb81d7b73b8316ebc256c32e6ec2c2953 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 5 Jun 2026 11:42:40 -0700 Subject: [PATCH 05/12] make presses on components in rows not trigger selection --- .../stories/GridList.stories.tsx | 92 +++++++++++++++++-- .../stories/Tree.stories.tsx | 47 ++++++++-- .../test/GridList.test.js | 74 +++++++++++++-- .../src/gridlist/useGridListItem.ts | 35 ++++++- 4 files changed, 224 insertions(+), 24 deletions(-) diff --git a/packages/react-aria-components/stories/GridList.stories.tsx b/packages/react-aria-components/stories/GridList.stories.tsx index 9dc48496b6e..5adc1814509 100644 --- a/packages/react-aria-components/stories/GridList.stories.tsx +++ b/packages/react-aria-components/stories/GridList.stories.tsx @@ -12,9 +12,10 @@ import {action} from 'storybook/actions'; import {Button} from '../src/Button'; -import {Checkbox, CheckboxProps} from '../src/Checkbox'; +import {Checkbox, CheckboxGroup, CheckboxProps} from '../src/Checkbox'; import {classNames} from '@adobe/react-spectrum/private/utils/classNames'; import {Collection} from 'react-aria/Collection'; +import {ComboBox} from '../src/ComboBox'; import {Dialog, DialogTrigger} from '../src/Dialog'; import {DropIndicator, useDragAndDrop} from '../src/useDragAndDrop'; import {GridLayout} from '../src/GridLayout'; @@ -30,8 +31,9 @@ import { import {Heading} from '../src/Heading'; import {Input} from '../src/Input'; import {Key} from '@react-types/shared'; +import {ListBox} from '../src/ListBox'; import {ListLayout, Size, WaterfallLayout} from 'react-stately/useVirtualizerState'; -import {LoadingSpinner} from './utils'; +import {LoadingSpinner, MyListBoxItem} from './utils'; import {LoadingState} from '@react-types/shared'; import {Menu, MenuItem, MenuTrigger} from '../src/Menu'; import {Meta, StoryFn, StoryObj} from '@storybook/react'; @@ -47,6 +49,7 @@ import {useAsyncList} from 'react-stately/useAsyncList'; import {useListData} from 'react-stately/useListData'; import {Virtualizer} from '../src/Virtualizer'; import './styles.css'; +import {Radio, RadioGroup} from '../src/RadioGroup'; export default { title: 'React Aria Components/GridList', @@ -968,12 +971,16 @@ export const AsyncGridListGridVirtualized: StoryObj { + return
No results
; +}; + // TODO: bugs to investigate // clicking on the textfield when selection is enabled causes selection to be toggled export const GridListWithTextfield: GridListStory = args => { let isHorizontalStack = args.orientation === 'horizontal' && args.layout !== 'grid'; return ( - <> +
{ Toolbar - - - + + + @@ -1026,9 +1033,80 @@ export const GridListWithTextfield: GridListStory = args => { + + RadioGroup + + + Dog + + + Cat + + + Dragon + + + + + CheckboxGroup + + + + Soccer + + + + Baseball + + + + Basketball + + + + + ComboBox + +
+ + +
+ + + Foo + Bar + Baz + Google + + +
+
- +
); }; diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 3d192f14c54..9313fb48120 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -15,13 +15,15 @@ import {Button} from '../src/Button'; import {Checkbox, CheckboxProps} from '../src/Checkbox'; import {classNames} from '@adobe/react-spectrum/private/utils/classNames'; import {Collection} from 'react-aria/Collection'; +import {ComboBox} from '../src/ComboBox'; import {DroppableCollectionReorderEvent, Key} from '@react-types/shared'; import {Input} from '../src/Input'; import {isTextDropItem, useDragAndDrop} from '../exports/useDragAndDrop'; +import {ListBox} from '../src/ListBox'; import {ListLayout} from 'react-stately/useVirtualizerState'; import {Menu, MenuItem, MenuTrigger} from '../src/Menu'; import {Meta, StoryFn, StoryObj} from '@storybook/react'; -import {MyMenuItem} from './utils'; +import {MyListBoxItem, MyMenuItem} from './utils'; import {Popover} from '../src/Popover'; import React, {JSX, ReactNode, useCallback, useState} from 'react'; import styles from '../example/index.css'; @@ -47,7 +49,7 @@ import './styles.css'; export default { title: 'React Aria Components/Tree', component: Tree, - excludeStories: ['TreeExampleStaticRender'] + excludeStories: ['TreeExampleStaticRender', 'TreeWithTextField'] } as Meta; export type TreeStory = StoryFn; @@ -1805,15 +1807,19 @@ export const HugeVirtualizedTree: StoryObj = { render: args => }; +let comboboxEmptyState = () => { + return
No results
; +}; + // TODO: bugs to investigate // clicking on the textfield when selection is enabled causes selection to be toggled export function TreeWithTextField(props: TreeProps) { return ( - <> +
(props: TreeProps) { textValue="Toolbar" interactive={ - - - + + + }> Toolbar @@ -1877,7 +1883,30 @@ export function TreeWithTextField(props: TreeProps) { }> + interactive={ + +
+ + +
+ + + Foo + Bar + Baz + Google + + +
+ }> Nested child 1
(props: TreeProps) {
- +
); } diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 2ec0df5d91a..8772dd3720e 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -1841,7 +1841,7 @@ describe('GridList', () => { ])( 'should not navigate rows when arrow keys are pressed while a text input child has focus (%s)', async (_, listProps) => { - let {getAllByRole, getByRole} = render( + let {getByRole} = render( Apple @@ -1852,7 +1852,8 @@ describe('GridList', () => { ); - let rows = getAllByRole('row'); + let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); + let rows = gridListTester.getRows(); let input = getByRole('textbox'); await user.tab(); @@ -1877,7 +1878,7 @@ describe('GridList', () => { ])( 'should not trigger typeahead when typing in a text input child (%s)', async (_, listProps) => { - let {getAllByRole, getByRole} = render( + let {getByRole} = render( Apple @@ -1888,7 +1889,8 @@ describe('GridList', () => { ); - let rows = getAllByRole('row'); + let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); + let rows = gridListTester.getRows(); let input = getByRole('textbox'); await user.tab(); @@ -1909,7 +1911,7 @@ describe('GridList', () => { 'should not trigger selection when pressing Space in a text input child (%s)', async (_, listProps) => { let onSelectionChange = jest.fn(); - let {getAllByRole, getByRole} = render( + let {getByRole} = render( { ); - let rows = getAllByRole('row'); + let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); + let rows = gridListTester.getRows(); let input = getByRole('textbox'); await user.tab(); @@ -1937,5 +1940,64 @@ describe('GridList', () => { expect(onSelectionChange).not.toHaveBeenCalled(); } ); + + it.each([ + ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], + ['layout="grid"', {layout: 'grid'}] + ])( + 'should not trigger selection when clicking on a tabbable child element (%s)', + async (_, listProps) => { + let onSelectionChange = jest.fn(); + let {getByRole} = render( + + + Apple + + + Banana + + + ); + + let input = getByRole('textbox'); + await user.click(input); + expect(document.activeElement).toBe(input); + expect(onSelectionChange).not.toHaveBeenCalled(); + } + ); + + it.each([ + ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], + ['layout="grid"', {layout: 'grid'}] + ])( + 'should still trigger selection when clicking on a row with no tabbable children (%s)', + async (_, listProps) => { + let onSelectionChange = jest.fn(); + let {getByRole} = render( + + + Apple + + + Banana + + + ); + + let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); + let rows = gridListTester.getRows(); + await user.click(rows[0]); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['item1'])); + } + ); }); }); diff --git a/packages/react-aria/src/gridlist/useGridListItem.ts b/packages/react-aria/src/gridlist/useGridListItem.ts index d27bac3fcad..7c17e58fb86 100644 --- a/packages/react-aria/src/gridlist/useGridListItem.ts +++ b/packages/react-aria/src/gridlist/useGridListItem.ts @@ -30,8 +30,15 @@ import { import {getFocusableTreeWalker} from '../focus/FocusScope'; import {getRowId, listMap} from './utils'; import {getScrollParent} from '../utils/getScrollParent'; -import {HTMLAttributes, KeyboardEvent as ReactKeyboardEvent, useRef} from 'react'; +import { + HTMLAttributes, + KeyboardEvent as ReactKeyboardEvent, + MouseEvent as ReactMouseEvent, + PointerEvent as ReactPointerEvent, + useRef +} from 'react'; import {isFocusVisible} from '../interactions/useFocusVisible'; +import {isTabbable} from '../utils/isFocusable'; import type {ListState} from 'react-stately/useListState'; import {mergeProps} from '../utils/mergeProps'; import {scrollIntoViewport} from '../utils/scrollIntoView'; @@ -416,7 +423,9 @@ export function useGridListItem( id: getRowId(state, node.key) }); - // TODO we need to guard against space/enter triggering selection/row link via usePress (from itemProps) so check if propagation + // TODO: guarding against selection when firing space/enter/click on a element in a row is technically not only limited to textfields so I + // am not making it specific to keyboardNavigationBehavior = tab, but maybe we should still? + // we need to guard against space/enter triggering selection/row link via usePress (from itemProps) so check if propagation // is stopped. this also fixes space not working in a textfield in a tree parent row let baseOnKeyDown = rowProps.onKeyDown; rowProps.onKeyDown = (e: ReactKeyboardEvent) => { @@ -426,6 +435,28 @@ export function useGridListItem( } }; + // guard against presses triggering row selecition when they happen on elements within the row + // am currently assuming if it is tabbable it is interactive, but maybe can use a different kind of check + let baseOnPointerDown = rowProps.onPointerDown; + rowProps.onPointerDown = (e: ReactPointerEvent) => { + let target = getEventTarget(e) as Element | null; + if (target && target !== ref.current && isTabbable(target)) { + e.stopPropagation(); + return; + } + baseOnPointerDown?.(e); + }; + + let baseOnMouseDown = rowProps.onMouseDown; + rowProps.onMouseDown = (e: ReactMouseEvent) => { + let target = getEventTarget(e) as Element | null; + if (target && target !== ref.current && isTabbable(target)) { + e.stopPropagation(); + return; + } + baseOnMouseDown?.(e); + }; + if (isVirtualized) { let {collection} = state; let nodes = [...collection]; From 158d3ce78a8350741fbce796347c84f65a3e9075 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 5 Jun 2026 15:35:28 -0700 Subject: [PATCH 06/12] add docs for rac --- packages/@react-spectrum/s2/src/TreeView.tsx | 1 + .../dev/s2-docs/pages/react-aria/GridList.mdx | 55 ++++++++++++++++++- .../dev/s2-docs/pages/react-aria/Tree.mdx | 55 ++++++++++++++++++- .../stories/GridList.stories.tsx | 2 - .../stories/Tree.stories.tsx | 14 ++--- packages/react-aria/src/tree/useTree.ts | 5 +- starters/docs/src/ComboBox.css | 2 + starters/docs/src/ComboBox.tsx | 2 +- starters/docs/src/Tree.tsx | 12 +++- 9 files changed, 127 insertions(+), 21 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 4573d2d5331..73abdd4bb11 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -104,6 +104,7 @@ export interface TreeViewProps | 'selectionBehavior' | 'onScroll' | 'onCellAction' + | 'keyboardNavigationBehavior' | keyof GlobalDOMAttributes >, UnsafeStyles, diff --git a/packages/dev/s2-docs/pages/react-aria/GridList.mdx b/packages/dev/s2-docs/pages/react-aria/GridList.mdx index c9511c40470..90eee7f2a2f 100644 --- a/packages/dev/s2-docs/pages/react-aria/GridList.mdx +++ b/packages/dev/s2-docs/pages/react-aria/GridList.mdx @@ -672,7 +672,7 @@ function Example(props) { Use the `layout` and `orientation` props to arrange items in horizontal and vertical stacks and grids. This affects keyboard navigation and drag and drop behavior. -```tsx render docs={docs.exports.GridList} links={docs.links} props={['layout', 'orientation', 'keyboardNavigationBehavior']} initialProps={{layout: 'grid', orientation: 'horizontal', keyboardNavigationBehavior: 'tab'}} wide +```tsx render docs={docs.exports.GridList} links={docs.links} props={['layout', 'orientation']} initialProps={{layout: 'grid', orientation: 'horizontal'}} wide "use client"; import {GridList, GridListItem, Text} from 'vanilla-starter/GridList'; @@ -704,6 +704,59 @@ let photos = [
``` +## Keyboard navigation + +By default, GridList uses arrow key navigation to move focus into rows. Set `keyboardNavigationBehavior="tab"` to have Tab move focus in and out of a row. + +```tsx render +"use client"; +import {GridList, GridListItem, Text} from 'vanilla-starter/GridList'; +import {ComboBox, ComboBoxItem} from 'vanilla-starter/ComboBox'; + +///- begin collapse -/// +function PermissionPicker({label}) { + return ( + + Can view + Can comment + Can edit + + ); +} +///- end collapse -/// + + + + + Desert Sunset + PNG • 2/3/2024 + + + + + Hiking Trail + JPEG • 1/10/2022 + + + + + Lion + JPEG • 8/28/2021 + + + + + Mountain Sunrise + PNG • 3/15/2015 + + + +``` + ## Drag and drop GridList supports drag and drop interactions when the `dragAndDropHooks` prop is provided using the hook. Users can drop data on the list as a whole, on individual items, insert new items between existing ones, or reorder items. React Aria supports drag and drop via mouse, touch, keyboard, and screen reader interactions. See the [drag and drop guide](dnd?component=GridList) to learn more. diff --git a/packages/dev/s2-docs/pages/react-aria/Tree.mdx b/packages/dev/s2-docs/pages/react-aria/Tree.mdx index 9094137b4c2..3803c657d8e 100644 --- a/packages/dev/s2-docs/pages/react-aria/Tree.mdx +++ b/packages/dev/s2-docs/pages/react-aria/Tree.mdx @@ -257,7 +257,7 @@ import {Tree, TreeHeader, TreeItem, TreeSection} from 'vanilla-starter/Tree'; - + Documents @@ -322,6 +322,59 @@ function Example(props) { } ``` +## Keyboard navigation + +By default, Tree uses arrow key navigation to move focus into rows. Set `keyboardNavigationBehavior="tab"` to have Option move focus in and out of a row. + +```tsx render +"use client"; +import {Tree, TreeItem, TreeItemContent} from 'vanilla-starter/Tree'; +import {ComboBox, ComboBoxItem} from 'vanilla-starter/ComboBox'; + +///- begin collapse -/// +function PermissionPicker({label}) { + return ( + + Can view + Can comment + Can edit + + ); +} +///- end collapse -/// + + + + + + Weekly Report.pdf + + + + + + Budget.xlsx + + + + + + + + Sunset.jpg + + + + + +``` + ## Drag and drop Tree supports drag and drop interactions when the `dragAndDropHooks` prop is provided using the hook. Users can drop data on the list as a whole, on individual items, insert new items between existing ones, or reorder items. React Aria supports drag and drop via mouse, touch, keyboard, and screen reader interactions. See the [drag and drop guide](dnd?component=Tree) to learn more. diff --git a/packages/react-aria-components/stories/GridList.stories.tsx b/packages/react-aria-components/stories/GridList.stories.tsx index 5adc1814509..5b2e8b07582 100644 --- a/packages/react-aria-components/stories/GridList.stories.tsx +++ b/packages/react-aria-components/stories/GridList.stories.tsx @@ -975,8 +975,6 @@ let comboboxEmptyState = () => { return
No results
; }; -// TODO: bugs to investigate -// clicking on the textfield when selection is enabled causes selection to be toggled export const GridListWithTextfield: GridListStory = args => { let isHorizontalStack = args.orientation === 'horizontal' && args.layout !== 'grid'; return ( diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 9313fb48120..b5920c692f6 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -1811,8 +1811,6 @@ let comboboxEmptyState = () => { return
No results
; }; -// TODO: bugs to investigate -// clicking on the textfield when selection is enabled causes selection to be toggled export function TreeWithTextField(props: TreeProps) { return (
@@ -1822,8 +1820,7 @@ export function TreeWithTextField(props: TreeProps) { style={{width: 400}} aria-label="tree with textfields" disabledKeys={['rawinput']} - // TODO: cast for now, tab behavior to be added to Tree later (will need tests/docs and whatnot)? - {...({keyboardNavigationBehavior: 'tab'} as any)} + keyboardNavigationBehavior="tab" {...props}> = { disabledBehavior: 'selection' }, argTypes: { - // TODO: add later - // keyboardNavigationBehavior: { - // control: 'radio', - // options: ['arrow', 'tab'] - // }, + keyboardNavigationBehavior: { + control: 'radio', + options: ['arrow', 'tab'] + }, selectionMode: { control: 'radio', options: ['none', 'single', 'multiple'] diff --git a/packages/react-aria/src/tree/useTree.ts b/packages/react-aria/src/tree/useTree.ts index 6a2f38793aa..dd98bd85c9e 100644 --- a/packages/react-aria/src/tree/useTree.ts +++ b/packages/react-aria/src/tree/useTree.ts @@ -21,10 +21,7 @@ import {TreeState} from 'react-stately/useTreeState'; export interface TreeProps extends GridListProps {} -export interface AriaTreeProps extends Omit< - AriaGridListProps, - 'keyboardNavigationBehavior' -> {} +export interface AriaTreeProps extends AriaGridListProps {} export interface AriaTreeOptions extends Omit< AriaGridListOptions, 'children' | 'shouldFocusWrap' diff --git a/starters/docs/src/ComboBox.css b/starters/docs/src/ComboBox.css index 12f8d59edef..6a9f4a33013 100644 --- a/starters/docs/src/ComboBox.css +++ b/starters/docs/src/ComboBox.css @@ -2,6 +2,8 @@ @import './TextField.css'; .react-aria-ComboBox { + display: flex; + flex-direction: column; color: var(--text-color); width: calc(var(--spacing) * 50); diff --git a/starters/docs/src/ComboBox.tsx b/starters/docs/src/ComboBox.tsx index e0ec8c027a6..80ef180ebb9 100644 --- a/starters/docs/src/ComboBox.tsx +++ b/starters/docs/src/ComboBox.tsx @@ -35,7 +35,7 @@ export function ComboBox({ }: ComboBoxProps) { return ( - + {label && }
diff --git a/starters/docs/src/Tree.tsx b/starters/docs/src/Tree.tsx index 3928dfd5a4e..425e7ea2481 100644 --- a/starters/docs/src/Tree.tsx +++ b/starters/docs/src/Tree.tsx @@ -48,15 +48,21 @@ export function TreeItemContent( } export interface TreeItemProps extends Partial { - title: React.ReactNode; + title?: React.ReactNode; } export function TreeItem(props: TreeItemProps) { let textValue = typeof props.title === 'string' ? props.title : ''; return ( - {props.title} - {props.children} + {props.title != null ? ( + <> + {props.title} + {props.children} + + ) : ( + props.children + )} ); } From e66db0bf05794c3e1836d39f9632ef7c097e148c Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 8 Jun 2026 11:19:49 -0700 Subject: [PATCH 07/12] update docs and dedupe tree logic in useGridListItem keyboard handlers --- .../dev/s2-docs/pages/react-aria/GridList.mdx | 44 +++---- .../dev/s2-docs/pages/react-aria/Tree.mdx | 1 + .../src/gridlist/useGridListItem.ts | 115 ++++++++---------- 3 files changed, 73 insertions(+), 87 deletions(-) diff --git a/packages/dev/s2-docs/pages/react-aria/GridList.mdx b/packages/dev/s2-docs/pages/react-aria/GridList.mdx index 90eee7f2a2f..dd8fb401865 100644 --- a/packages/dev/s2-docs/pages/react-aria/GridList.mdx +++ b/packages/dev/s2-docs/pages/react-aria/GridList.mdx @@ -714,6 +714,16 @@ import {GridList, GridListItem, Text} from 'vanilla-starter/GridList'; import {ComboBox, ComboBoxItem} from 'vanilla-starter/ComboBox'; ///- begin collapse -/// +///- begin collapse -/// +let photos = [ + {id: 1, title: 'Desert Sunset', description: 'PNG • 2/3/2024', src: 'https://images.unsplash.com/photo-1705034598432-1694e203cdf3?q=80&w=600&auto=format&fit=crop'}, + {id: 2, title: 'Hiking Trail', description: 'JPEG • 1/10/2022', src: 'https://images.unsplash.com/photo-1722233987129-61dc344db8b6?q=80&w=600&auto=format&fit=crop'}, + {id: 3, title: 'Lion', description: 'JPEG • 8/28/2021', src: 'https://images.unsplash.com/photo-1629812456605-4a044aa38fbc?q=80&w=600&auto=format&fit=crop'}, + {id: 4, title: 'Mountain Sunrise', description: 'PNG • 3/15/2015', src: 'https://images.unsplash.com/photo-1722172118908-1a97c312ce8c?q=80&w=600&auto=format&fit=crop'}, + {id: 5, title: 'Giraffe tongue', description: 'PNG • 11/27/2019', src: 'https://images.unsplash.com/photo-1574870111867-089730e5a72b?q=80&w=600&auto=format&fit=crop'}, + {id: 6, title: 'Golden Hour', description: 'WEBP • 7/24/2024', src: 'https://images.unsplash.com/photo-1718378037953-ab21bf2cf771?q=80&w=600&auto=format&fit=crop'}, +]; + function PermissionPicker({label}) { return ( @@ -729,31 +739,17 @@ function PermissionPicker({label}) { /*- begin highlight -*/ keyboardNavigationBehavior="tab" /*- end highlight -*/ + items={photos} + selectionMode="multiple" aria-label="Shared files"> - - - Desert Sunset - PNG • 2/3/2024 - - - - - Hiking Trail - JPEG • 1/10/2022 - - - - - Lion - JPEG • 8/28/2021 - - - - - Mountain Sunrise - PNG • 3/15/2015 - - + {item => ( + + + {item.title} + {item.description} + + + )} ``` diff --git a/packages/dev/s2-docs/pages/react-aria/Tree.mdx b/packages/dev/s2-docs/pages/react-aria/Tree.mdx index 3803c657d8e..7ede3dd972e 100644 --- a/packages/dev/s2-docs/pages/react-aria/Tree.mdx +++ b/packages/dev/s2-docs/pages/react-aria/Tree.mdx @@ -347,6 +347,7 @@ function PermissionPicker({label}) { /*- begin highlight -*/ keyboardNavigationBehavior="tab" /*- end highlight -*/ + selectionMode="multiple" defaultExpandedKeys={['documents', 'photos']} aria-label="Shared files" style={{width: 420}}> diff --git a/packages/react-aria/src/gridlist/useGridListItem.ts b/packages/react-aria/src/gridlist/useGridListItem.ts index 7c17e58fb86..d92412132c5 100644 --- a/packages/react-aria/src/gridlist/useGridListItem.ts +++ b/packages/react-aria/src/gridlist/useGridListItem.ts @@ -188,36 +188,10 @@ export function useGridListItem( let walker = getFocusableTreeWalker(ref.current); walker.currentNode = activeElement; - if ('expandedKeys' in state && activeElement === ref.current) { - if ( - e.key === EXPANSION_KEYS['expand'][direction] && - state.selectionManager.focusedKey === node.key && - hasChildRows && - !state.expandedKeys.has(node.key) - ) { - state.toggleKey(node.key); - e.stopPropagation(); - return; - } else if ( - e.key === EXPANSION_KEYS['collapse'][direction] && - state.selectionManager.focusedKey === node.key - ) { - // If item is collapsible, collapse it; else move to parent - if (hasChildRows && state.expandedKeys.has(node.key)) { - state.toggleKey(node.key); - e.stopPropagation(); - return; - } else if ( - !state.expandedKeys.has(node.key) && - node.parentKey && - state.collection.getItem(node.parentKey)?.type === 'item' - ) { - // Item is a leaf or already collapsed, move focus to parent - state.selectionManager.setFocusedKey(node.parentKey); - e.stopPropagation(); - return; - } - } + if ( + handleTreeExpansionKeys(e, state, node, hasChildRows, direction, activeElement, ref.current) + ) { + return; } switch (e.key) { @@ -341,39 +315,10 @@ export function useGridListItem( return; } - // TODO: for tree expansion since we turn off the capturing listener if keyboardNavigationBehavior = tab - // copied from above, extract into helper later - // need to support tab keyboard navigation in tree - if ('expandedKeys' in state && activeElement === ref.current) { - if ( - e.key === EXPANSION_KEYS['expand'][direction] && - state.selectionManager.focusedKey === node.key && - hasChildRows && - !state.expandedKeys.has(node.key) - ) { - state.toggleKey(node.key); - e.stopPropagation(); - return; - } else if ( - e.key === EXPANSION_KEYS['collapse'][direction] && - state.selectionManager.focusedKey === node.key - ) { - // If item is collapsible, collapse it; else move to parent - if (hasChildRows && state.expandedKeys.has(node.key)) { - state.toggleKey(node.key); - e.stopPropagation(); - return; - } else if ( - !state.expandedKeys.has(node.key) && - node.parentKey && - state.collection.getItem(node.parentKey)?.type === 'item' - ) { - // Item is a leaf or already collapsed, move focus to parent - state.selectionManager.setFocusedKey(node.parentKey); - e.stopPropagation(); - return; - } - } + if ( + handleTreeExpansionKeys(e, state, node, hasChildRows, direction, activeElement, ref.current) + ) { + return; } } @@ -484,6 +429,50 @@ export function useGridListItem( }; } +function handleTreeExpansionKeys( + e: ReactKeyboardEvent, + state: ListState | TreeState, + node: RSNode, + hasChildRows: boolean | undefined, + direction: string, + activeElement: Element | null, + rowRef: FocusableElement | null +): boolean { + if (!('expandedKeys' in state) || activeElement !== rowRef) { + return false; + } + if ( + e.key === EXPANSION_KEYS['expand'][direction] && + state.selectionManager.focusedKey === node.key && + hasChildRows && + !state.expandedKeys.has(node.key) + ) { + state.toggleKey(node.key); + e.stopPropagation(); + return true; + } else if ( + e.key === EXPANSION_KEYS['collapse'][direction] && + state.selectionManager.focusedKey === node.key + ) { + // If item is collapsible, collapse it; else move to parent + if (hasChildRows && state.expandedKeys.has(node.key)) { + state.toggleKey(node.key); + e.stopPropagation(); + return true; + } else if ( + !state.expandedKeys.has(node.key) && + node.parentKey && + state.collection.getItem(node.parentKey)?.type === 'item' + ) { + // Item is a leaf or already collapsed, move focus to parent + state.selectionManager.setFocusedKey(node.parentKey); + e.stopPropagation(); + return true; + } + } + return false; +} + function last(walker: TreeWalker) { let next: FocusableElement | null = null; let last: FocusableElement | null = null; From f350944b033a6b2ab3691a2e1534b30ab5343ce3 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 12 Jun 2026 10:42:40 -0700 Subject: [PATCH 08/12] pull other updates from other branch and change to stop all other events other than tab --- .../dev/s2-docs/pages/react-aria/GridList.mdx | 1 + .../dev/s2-docs/pages/react-aria/Tree.mdx | 1 + .../stories/GridList.stories.tsx | 8 +++---- .../test/GridList.test.js | 5 +++- .../test/TagGroup.test.js | 2 +- .../src/gridlist/useGridListItem.ts | 23 ++++--------------- 6 files changed, 14 insertions(+), 26 deletions(-) diff --git a/packages/dev/s2-docs/pages/react-aria/GridList.mdx b/packages/dev/s2-docs/pages/react-aria/GridList.mdx index dd8fb401865..74464237e46 100644 --- a/packages/dev/s2-docs/pages/react-aria/GridList.mdx +++ b/packages/dev/s2-docs/pages/react-aria/GridList.mdx @@ -707,6 +707,7 @@ let photos = [ ## Keyboard navigation By default, GridList uses arrow key navigation to move focus into rows. Set `keyboardNavigationBehavior="tab"` to have Tab move focus in and out of a row. +Use this when rows contain interactive elements such as text fields, where arrow keys and typing in the field should not trigger grid navigation or selection. ```tsx render "use client"; diff --git a/packages/dev/s2-docs/pages/react-aria/Tree.mdx b/packages/dev/s2-docs/pages/react-aria/Tree.mdx index 7ede3dd972e..2f626756fba 100644 --- a/packages/dev/s2-docs/pages/react-aria/Tree.mdx +++ b/packages/dev/s2-docs/pages/react-aria/Tree.mdx @@ -325,6 +325,7 @@ function Example(props) { ## Keyboard navigation By default, Tree uses arrow key navigation to move focus into rows. Set `keyboardNavigationBehavior="tab"` to have Option move focus in and out of a row. +Use this when rows contain interactive elements such as text fields, where arrow keys and typing in the field should not trigger grid navigation or selection. ```tsx render "use client"; diff --git a/packages/react-aria-components/stories/GridList.stories.tsx b/packages/react-aria-components/stories/GridList.stories.tsx index 5b2e8b07582..55adce21e64 100644 --- a/packages/react-aria-components/stories/GridList.stories.tsx +++ b/packages/react-aria-components/stories/GridList.stories.tsx @@ -979,7 +979,7 @@ export const GridListWithTextfield: GridListStory = args => { let isHorizontalStack = args.orientation === 'horizontal' && args.layout !== 'grid'; return (
- + { - +
); }; @@ -1112,9 +1112,7 @@ GridListWithTextfield.story = { args: { layout: 'stack', orientation: 'vertical', - escapeKeyBehavior: 'clearSelection', - shouldSelectOnPressUp: false, - disallowTypeAhead: false + escapeKeyBehavior: 'clearSelection' }, argTypes: { layout: { diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 8772dd3720e..7d835f8c94c 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -1908,7 +1908,7 @@ describe('GridList', () => { ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], ['layout="grid"', {layout: 'grid'}] ])( - 'should not trigger selection when pressing Space in a text input child (%s)', + 'should not trigger selection when pressing Space or Enter in a text input child (%s)', async (_, listProps) => { let onSelectionChange = jest.fn(); let {getByRole} = render( @@ -1938,6 +1938,9 @@ describe('GridList', () => { await user.keyboard(' '); expect(input).toHaveValue(' '); expect(onSelectionChange).not.toHaveBeenCalled(); + + await user.keyboard('{Enter}'); + expect(onSelectionChange).not.toHaveBeenCalled(); } ); diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index 8f615682656..ca686a9bd64 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -662,7 +662,7 @@ describe('TagGroup', () => { // TODO: a change in behavior since taggroup is a gridlist with "tab" keyboard navigation behavior // previously you could go to the next tab via arrow keys when you were focused on the close button - await user.keyboard('{Shift>}{Tab}{/Shift}'); + await user.tab({shift: true}); expect(tags[0]).toHaveFocus(); await user.keyboard('{ArrowRight}'); diff --git a/packages/react-aria/src/gridlist/useGridListItem.ts b/packages/react-aria/src/gridlist/useGridListItem.ts index d92412132c5..f352fc824f4 100644 --- a/packages/react-aria/src/gridlist/useGridListItem.ts +++ b/packages/react-aria/src/gridlist/useGridListItem.ts @@ -11,7 +11,6 @@ */ import {chain} from '../utils/chain'; - import { Collection, DOMAttributes, @@ -302,15 +301,10 @@ export function useGridListItem( } if (keyboardNavigationBehavior === 'tab') { - // TODO: Added Rob's useTypeSelect changes, but that only stops if type select is in progress - // This will stop arrow key navigation and typeselect from bubbling up - // (note that this breaks TagGroup's old behavior of using arrow keys to move from "x" button to next tag and typeselect when inside a card/row) - // should it just stop propagation for all events since we can't rely on non-RAC components stopping propagation even they handled the event - // Will need to do something similar for click? - if ( - getEventTarget(e) !== ref.current && - (isArrowKey(e.key) || isCharacterKey(e.key) || e.key === 'Enter') - ) { + // Stop propagation for all events that originate from the children of the gridlist item since we don't want to trigger + // grid level interactions (row navigation/typeselect/etc) + // exception made for Tab since that needs to propagate to useSelectableCollection to tab out of the gridlist, might be others? + if (getEventTarget(e) !== ref.current && e.key !== 'Tab') { e.stopPropagation(); return; } @@ -498,12 +492,3 @@ function getDirectChildren(parent: RSNode, collection: Collection Date: Fri, 12 Jun 2026 10:47:28 -0700 Subject: [PATCH 09/12] clean up outdated test comment --- packages/react-aria-components/test/TagGroup.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index ca686a9bd64..88c057d0abd 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -660,8 +660,6 @@ describe('TagGroup', () => { expect(onRemove).toHaveBeenCalledTimes(2); expect(onRemove).toHaveBeenLastCalledWith(new Set(['cat'])); - // TODO: a change in behavior since taggroup is a gridlist with "tab" keyboard navigation behavior - // previously you could go to the next tab via arrow keys when you were focused on the close button await user.tab({shift: true}); expect(tags[0]).toHaveFocus(); From 7a88537598dc1a7613fe4e315dee6a8b7f51c3c1 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 12 Jun 2026 14:10:24 -0700 Subject: [PATCH 10/12] fix so that Tab from combobox and toolbar propagate upwards so that you tab out of gridlist --- .../stories/GridList.stories.tsx | 50 ++++++------ .../test/GridList.test.js | 79 +++++++++++++++++++ .../react-aria/src/combobox/useComboBox.ts | 2 +- .../src/interactions/createEventHandler.ts | 5 ++ packages/react-aria/src/toolbar/useToolbar.ts | 1 - 5 files changed, 110 insertions(+), 27 deletions(-) diff --git a/packages/react-aria-components/stories/GridList.stories.tsx b/packages/react-aria-components/stories/GridList.stories.tsx index 55adce21e64..9710c97e31f 100644 --- a/packages/react-aria-components/stories/GridList.stories.tsx +++ b/packages/react-aria-components/stories/GridList.stories.tsx @@ -1008,6 +1008,31 @@ export const GridListWithTextfield: GridListStory = args => { {' '} + + ComboBox + +
+ + +
+ + + Foo + Bar + Baz + Google + + +
+
Toolbar @@ -1077,31 +1102,6 @@ export const GridListWithTextfield: GridListStory = args => { - - ComboBox - -
- - -
- - - Foo - Bar - Baz - Google - - -
-
diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 7d835f8c94c..c5df18a2ff8 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -22,6 +22,7 @@ import { import {Checkbox as AriaCheckbox, CheckboxButton, CheckboxField} from '../src/Checkbox'; import {Button} from '../src/Button'; import {Collection} from 'react-aria/Collection'; +import {ComboBox} from '../src/ComboBox'; import {Dialog, DialogTrigger} from '../src/Dialog'; import {DropIndicator, useDragAndDrop} from '../src/useDragAndDrop'; import {getFocusableTreeWalker} from 'react-aria/private/focus/FocusScope'; @@ -33,13 +34,17 @@ import { GridListSection } from '../src/GridList'; import {GridListLoadMoreItem} from '../src/GridList'; +import {Input} from '../src/Input'; import {installPointerEvent, User} from '@react-aria/test-utils'; import {Label} from '../src/Label'; +import {ListBox, ListBoxItem} from '../src/ListBox'; import {ListLayout} from 'react-stately/useVirtualizerState'; import {Modal} from '../src/Modal'; +import {Popover} from '../src/Popover'; import React from 'react'; import {RouterProvider} from 'react-aria/private/utils/openLink'; import {Tag, TagGroup, TagList} from '../src/TagGroup'; +import {Toolbar} from '../src/Toolbar'; import userEvent from '@testing-library/user-event'; import {Virtualizer} from '../src/Virtualizer'; @@ -2002,5 +2007,79 @@ describe('GridList', () => { expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['item1'])); } ); + + it.each([ + ['keyboardNavigationBehavior="tab"', {keyboardNavigationBehavior: 'tab'}], + ['layout="grid"', {layout: 'grid'}] + ])( + 'should exit the grid when tabbing from a combobox or toolbar, not focus the next row (%s)', + async (_, listProps) => { + let {getByRole} = render( +
+ + + + + + + + + Foo + Bar + + + + + + + + + + + + + + + +
+ ); + + let combobox = getByRole('combobox'); + let afterInput = getByRole('textbox', {name: 'after'}); + let boldButton = getByRole('button', {name: 'Bold'}); + let row2Input = getByRole('textbox', {name: 'row 2 input'}); + + let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); + let rows = gridListTester.getRows(); + + await user.tab(); + await user.tab(); + await user.tab(); + expect(document.activeElement).toBe(combobox); + + await user.tab(); + expect(document.activeElement).toBe(afterInput); + + // note that shift tabbing move focus back to gridlist item not the combobox itself, + // will need to look into this later + await user.tab({shift: true}); + expect(document.activeElement).toBe(rows[0]); + await user.keyboard(listProps.layout === 'grid' ? '{ArrowRight}' : '{ArrowDown}'); + expect(document.activeElement).toBe(rows[1]); + await user.tab(); + expect(document.activeElement).toBe(boldButton); + await user.tab(); + expect(document.activeElement).toBe(afterInput); + + await user.tab({shift: true}); + expect(document.activeElement).toBe(rows[1]); + await user.keyboard(listProps.layout === 'grid' ? '{ArrowRight}' : '{ArrowDown}'); + expect(document.activeElement).toBe(rows[2]); + await user.tab(); + expect(document.activeElement).toBe(row2Input); + await user.tab(); + expect(document.activeElement).toBe(afterInput); + } + ); }); }); diff --git a/packages/react-aria/src/combobox/useComboBox.ts b/packages/react-aria/src/combobox/useComboBox.ts index 42fe109769c..a9006f5d642 100644 --- a/packages/react-aria/src/combobox/useComboBox.ts +++ b/packages/react-aria/src/combobox/useComboBox.ts @@ -223,7 +223,7 @@ export function useComboBox( if (state.isOpen) { state.commit(); } - return {shouldPreventDefault: false}; + return {shouldPreventDefault: false, shouldContinuePropagation: true}; }, Escape: () => { let shouldContinuePropagation = false; diff --git a/packages/react-aria/src/interactions/createEventHandler.ts b/packages/react-aria/src/interactions/createEventHandler.ts index 0c9d6900f9d..d1c08ab279b 100644 --- a/packages/react-aria/src/interactions/createEventHandler.ts +++ b/packages/react-aria/src/interactions/createEventHandler.ts @@ -45,6 +45,11 @@ export function createEventHandler( }, continuePropagation() { shouldStopPropagation = false; + // nested createEventHandler might have set continue propagation so we should continue + // propagation on wrappers + if (typeof (e as any).continuePropagation === 'function') { + (e as any).continuePropagation(); + } }, isPropagationStopped() { return shouldStopPropagation; diff --git a/packages/react-aria/src/toolbar/useToolbar.ts b/packages/react-aria/src/toolbar/useToolbar.ts index 00b0b54b28c..bedd41b674a 100644 --- a/packages/react-aria/src/toolbar/useToolbar.ts +++ b/packages/react-aria/src/toolbar/useToolbar.ts @@ -89,7 +89,6 @@ export function useToolbar( // out of the entire toolbar. To do this, move focus // to the first or last focusable child, and let the // browser handle the Tab key as usual from there. - e.stopPropagation(); lastFocused.current = getActiveElement() as HTMLElement; if (e.shiftKey) { focusManager.focusFirst(); From c1343f9b80bd19a11035445b27e214cd89eb9a1f Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 12 Jun 2026 15:12:12 -0700 Subject: [PATCH 11/12] Revert "feat: Keyboard shortcut handler (#9929)" This reverts commit 9e1b07014067f824a50cae586a4532a9d0b9f841. --- .../src/actionbar/ActionBar.tsx | 5 +- packages/@react-spectrum/s2/src/ActionBar.tsx | 7 +- .../test/ListBox.test.js | 37 -- .../src/actiongroup/useActionGroup.ts | 40 +- .../src/calendar/useCalendarGrid.ts | 84 ++-- packages/react-aria/src/color/useColorArea.ts | 88 ++-- .../react-aria/src/color/useColorWheel.ts | 30 +- .../react-aria/src/combobox/useComboBox.ts | 77 ++- .../react-aria/src/datepicker/useDateField.ts | 13 +- .../src/datepicker/useDatePickerGroup.ts | 64 ++- .../src/datepicker/useDateSegment.ts | 39 +- .../src/interactions/createEventHandler.ts | 8 +- .../createKeyboardShortcutHandler.ts | 217 --------- .../src/interactions/useKeyboard.ts | 59 +-- packages/react-aria/src/menu/useMenuItem.ts | 58 ++- .../react-aria/src/menu/useMenuTrigger.ts | 90 ++-- .../react-aria/src/menu/useSubmenuTrigger.ts | 91 ++-- .../src/numberfield/useNumberField.ts | 29 +- .../react-aria/src/overlays/useOverlay.ts | 19 +- .../react-aria/src/radio/useRadioGroup.ts | 57 +-- .../src/searchfield/useSearchField.ts | 63 +-- packages/react-aria/src/select/useSelect.ts | 39 +- .../src/selection/useSelectableCollection.ts | 457 ++++++++---------- .../react-aria/src/slider/useSliderThumb.ts | 60 +-- .../src/spinbutton/useSpinButton.ts | 87 ++-- .../src/steplist/useStepListItem.ts | 12 + .../src/table/useTableColumnResize.ts | 31 +- packages/react-aria/src/tag/useTag.ts | 33 +- .../useSearchAutocomplete.test.js | 4 +- .../test/combobox/useComboBox.test.js | 48 +- .../test/interactions/useKeyboard.test.js | 278 ----------- .../test/searchfield/useSearchField.test.js | 7 +- 32 files changed, 823 insertions(+), 1408 deletions(-) delete mode 100644 packages/react-aria/src/interactions/createKeyboardShortcutHandler.ts diff --git a/packages/@adobe/react-spectrum/src/actionbar/ActionBar.tsx b/packages/@adobe/react-spectrum/src/actionbar/ActionBar.tsx index c59b9c93a41..016a8c620ec 100644 --- a/packages/@adobe/react-spectrum/src/actionbar/ActionBar.tsx +++ b/packages/@adobe/react-spectrum/src/actionbar/ActionBar.tsx @@ -110,8 +110,9 @@ function ActionBarInner(props: ActionBarInnerProps, ref: Ref { + onKeyDown(e) { + if (e.key === 'Escape') { + e.preventDefault(); onClearSelection(); } } diff --git a/packages/@react-spectrum/s2/src/ActionBar.tsx b/packages/@react-spectrum/s2/src/ActionBar.tsx index e7b65502b36..323cab27972 100644 --- a/packages/@react-spectrum/s2/src/ActionBar.tsx +++ b/packages/@react-spectrum/s2/src/ActionBar.tsx @@ -167,9 +167,12 @@ const ActionBarInner = forwardRef(function ActionBarInner( }); let {keyboardProps} = useKeyboard({ - shortcuts: { - Escape: () => { + onKeyDown(e) { + if (e.key === 'Escape') { + e.preventDefault(); onClearSelection?.(); + } else { + e.continuePropagation(); } } }); diff --git a/packages/react-aria-components/test/ListBox.test.js b/packages/react-aria-components/test/ListBox.test.js index f851badd821..38a125105df 100644 --- a/packages/react-aria-components/test/ListBox.test.js +++ b/packages/react-aria-components/test/ListBox.test.js @@ -2396,40 +2396,3 @@ describe('ListBox', () => { }); } }); - -describe('keyboard modifier keys', () => { - let user; - let platformMock; - beforeAll(() => { - user = userEvent.setup({delay: null, pointerMap}); - }); - // selectionMode: 'none', 'single', 'multiple' - // selectionBehavior: 'toggle', 'replace' - // platform: 'mac', 'windows' - - // modifier key: 'alt', 'ctrl', 'meta', 'shift' - // key: 'arrow-up', 'arrow-down', 'arrow-left', 'arrow-right', 'home', 'end', 'page-up', 'page-down', 'enter', 'space', 'tab' - // expected behavior: 'navigate', 'select', 'toggle', 'replace' - describe('mac', () => { - beforeAll(() => { - platformMock = jest.spyOn(navigator, 'platform', 'get').mockImplementation(() => 'Mac'); - }); - afterAll(() => { - platformMock.mockRestore(); - }); - it('should not navigate when using unsupported modifier keys', async () => { - let {getByRole} = renderListbox({selectionMode: 'none'}); - await user.tab(); - let listbox = getByRole('listbox'); - let options = within(listbox).getAllByRole('option'); - await user.keyboard('{ArrowDown}'); - expect(document.activeElement).toBe(options[1]); - await user.keyboard('{Meta>}{ArrowDown}{/Meta}'); - expect(document.activeElement).toBe(options[1]); - await user.keyboard('{Meta>}{ArrowUp}{/Meta}'); - expect(document.activeElement).toBe(options[1]); - await user.keyboard('{Control>}{Home}{/Control}'); - expect(document.activeElement).toBe(options[1]); - }); - }); -}); diff --git a/packages/react-aria/src/actiongroup/useActionGroup.ts b/packages/react-aria/src/actiongroup/useActionGroup.ts index cf2dbe38864..15985ff3c46 100644 --- a/packages/react-aria/src/actiongroup/useActionGroup.ts +++ b/packages/react-aria/src/actiongroup/useActionGroup.ts @@ -24,11 +24,11 @@ import { } from '@react-types/shared'; import {createFocusManager} from '../focus/FocusScope'; import {filterDOMProps} from '../utils/filterDOMProps'; +import {getEventTarget, nodeContains} from '../utils/shadowdom/DOMFunctions'; +import {KeyboardEventHandler, useState} from 'react'; import {ListState} from 'react-stately/useListState'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLayoutEffect} from '../utils/useLayoutEffect'; import {useLocale} from '../i18n/I18nProvider'; -import {useState} from 'react'; const BUTTON_GROUP_ROLES = { none: 'toolbar', @@ -91,30 +91,34 @@ export function useActionGroup( let {direction} = useLocale(); let focusManager = createFocusManager(ref); let flipDirection = direction === 'rtl' && orientation === 'horizontal'; - let {keyboardProps} = useKeyboard({ - shortcuts: { - ArrowRight: () => { - if (flipDirection) { + let onKeyDown: KeyboardEventHandler = e => { + if (!nodeContains(e.currentTarget, getEventTarget(e))) { + return; + } + + switch (e.key) { + case 'ArrowRight': + case 'ArrowDown': + e.preventDefault(); + e.stopPropagation(); + if (e.key === 'ArrowRight' && flipDirection) { focusManager.focusPrevious({wrap: true}); } else { focusManager.focusNext({wrap: true}); } - }, - ArrowDown: () => { - focusManager.focusNext({wrap: true}); - }, - ArrowLeft: () => { - if (flipDirection) { + break; + case 'ArrowLeft': + case 'ArrowUp': + e.preventDefault(); + e.stopPropagation(); + if (e.key === 'ArrowLeft' && flipDirection) { focusManager.focusNext({wrap: true}); } else { focusManager.focusPrevious({wrap: true}); } - }, - ArrowUp: () => { - focusManager.focusPrevious({wrap: true}); - } + break; } - }); + }; let role: string | undefined = BUTTON_GROUP_ROLES[state.selectionManager.selectionMode]; if (isInToolbar && role === 'toolbar') { @@ -126,7 +130,7 @@ export function useActionGroup( role, 'aria-orientation': role === 'toolbar' ? orientation : undefined, 'aria-disabled': isDisabled, - ...keyboardProps + onKeyDown } }; } diff --git a/packages/react-aria/src/calendar/useCalendarGrid.ts b/packages/react-aria/src/calendar/useCalendarGrid.ts index 1a1efb20057..d610ab780ab 100644 --- a/packages/react-aria/src/calendar/useCalendarGrid.ts +++ b/packages/react-aria/src/calendar/useCalendarGrid.ts @@ -14,13 +14,12 @@ import {CalendarDate, startOfWeek, today} from '@internationalized/date'; import {CalendarSelectionMode, CalendarState} from 'react-stately/useCalendarState'; import {DOMAttributes} from '@react-types/shared'; import {hookData, useVisibleRangeDescription} from './utils'; +import {KeyboardEvent, useMemo} from 'react'; import {mergeProps} from '../utils/mergeProps'; import {RangeCalendarState} from 'react-stately/useRangeCalendarState'; import {useDateFormatter} from '../i18n/useDateFormatter'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLabels} from '../utils/useLabels'; import {useLocale} from '../i18n/I18nProvider'; -import {useMemo} from 'react'; export interface AriaCalendarGridProps { /** @@ -79,61 +78,70 @@ export function useCalendarGrid( let {direction} = useLocale(); - let {keyboardProps} = useKeyboard({ - shortcuts: { - Enter: () => { + let onKeyDown = (e: KeyboardEvent) => { + switch (e.key) { + case 'Enter': + case ' ': + e.preventDefault(); state.selectFocusedDate(); - }, - ' ': () => { - state.selectFocusedDate(); - }, - PageUp: () => { - state.focusPreviousSection(); - }, - 'Shift+PageUp': () => { - state.focusPreviousSection(true); - }, - PageDown: () => { - state.focusNextSection(); - }, - 'Shift+PageDown': () => { - state.focusNextSection(true); - }, - End: () => { + break; + case 'PageUp': + e.preventDefault(); + e.stopPropagation(); + state.focusPreviousSection(e.shiftKey); + break; + case 'PageDown': + e.preventDefault(); + e.stopPropagation(); + state.focusNextSection(e.shiftKey); + break; + case 'End': + e.preventDefault(); + e.stopPropagation(); state.focusSectionEnd(); - }, - Home: () => { + break; + case 'Home': + e.preventDefault(); + e.stopPropagation(); state.focusSectionStart(); - }, - ArrowLeft: () => { + break; + case 'ArrowLeft': + e.preventDefault(); + e.stopPropagation(); if (direction === 'rtl') { state.focusNextDay(); } else { state.focusPreviousDay(); } - }, - ArrowUp: () => { + break; + case 'ArrowUp': + e.preventDefault(); + e.stopPropagation(); state.focusPreviousRow(); - }, - ArrowRight: () => { + break; + case 'ArrowRight': + e.preventDefault(); + e.stopPropagation(); if (direction === 'rtl') { state.focusPreviousDay(); } else { state.focusNextDay(); } - }, - ArrowDown: () => { + break; + case 'ArrowDown': + e.preventDefault(); + e.stopPropagation(); state.focusNextRow(); - }, - Escape: () => { + break; + case 'Escape': // Cancel the selection. if ('setAnchorDate' in state) { + e.preventDefault(); state.setAnchorDate(null); } - return false; // TODO: is this really correct? or should it return true when we cancel and only propagate if there's nothing to do - } + break; } - }); + }; let visibleRangeDescription = useVisibleRangeDescription( startDate, @@ -174,7 +182,7 @@ export function useCalendarGrid( 'aria-disabled': state.isDisabled || undefined, 'aria-multiselectable': 'highlightedRange' in state || state.selectionMode === 'multiple' || undefined, - ...keyboardProps, + onKeyDown, onFocus: () => state.setFocused(true), onBlur: () => state.setFocused(false) }), diff --git a/packages/react-aria/src/color/useColorArea.ts b/packages/react-aria/src/color/useColorArea.ts index c7a293abf70..8351e622c25 100644 --- a/packages/react-aria/src/color/useColorArea.ts +++ b/packages/react-aria/src/color/useColorArea.ts @@ -111,56 +111,46 @@ export function useColorArea(props: AriaColorAreaOptions, state: ColorAreaState) let currentPosition = useRef<{x: number; y: number} | null>(null); - let keyboardUpdate = (cb, inputRef: RefObject, input: 'x' | 'y') => { - state.setDragging(true); - setValueChangedViaKeyboard(true); - cb(); - state.setDragging(false); - focusInput(inputRef); - setFocusedInput(input); - }; - let {keyboardProps} = useKeyboard({ - shortcuts: { - PageUp: () => { - return keyboardUpdate( - () => { - state.incrementY(state.yChannelPageStep); - }, - inputYRef, - 'y' - ); - }, - PageDown: () => { - return keyboardUpdate( - () => { - state.decrementY(state.yChannelPageStep); - }, - inputYRef, - 'y' - ); - }, - Home: () => { - return keyboardUpdate( - () => { - direction === 'rtl' - ? state.incrementX(state.xChannelPageStep) - : state.decrementX(state.xChannelPageStep); - }, - inputXRef, - 'x' - ); - }, - End: () => { - return keyboardUpdate( - () => { - direction === 'rtl' - ? state.decrementX(state.xChannelPageStep) - : state.incrementX(state.xChannelPageStep); - }, - inputXRef, - 'x' - ); + onKeyDown(e) { + // these are the cases that useMove doesn't handle + if (!/^(PageUp|PageDown|Home|End)$/.test(e.key)) { + e.continuePropagation(); + return; + } + // same handling as useMove, don't need to stop propagation, useKeyboard will do that for us + e.preventDefault(); + // remember to set this and unset it so that onChangeEnd is fired + state.setDragging(true); + setValueChangedViaKeyboard(true); + let dir; + switch (e.key) { + case 'PageUp': + state.incrementY(state.yChannelPageStep); + dir = 'y'; + break; + case 'PageDown': + state.decrementY(state.yChannelPageStep); + dir = 'y'; + break; + case 'Home': + direction === 'rtl' + ? state.incrementX(state.xChannelPageStep) + : state.decrementX(state.xChannelPageStep); + dir = 'x'; + break; + case 'End': + direction === 'rtl' + ? state.decrementX(state.xChannelPageStep) + : state.incrementX(state.xChannelPageStep); + dir = 'x'; + break; + } + state.setDragging(false); + if (dir) { + let input = dir === 'x' ? inputXRef : inputYRef; + focusInput(input); + setFocusedInput(dir); } } }); diff --git a/packages/react-aria/src/color/useColorWheel.ts b/packages/react-aria/src/color/useColorWheel.ts index cd313393755..5a1edc2ca59 100644 --- a/packages/react-aria/src/color/useColorWheel.ts +++ b/packages/react-aria/src/color/useColorWheel.ts @@ -75,17 +75,27 @@ export function useColorWheel( let currentPosition = useRef<{x: number; y: number} | null>(null); let {keyboardProps} = useKeyboard({ - shortcuts: { - PageUp: () => { - state.setDragging(true); - state.increment(state.pageStep); - state.setDragging(false); - }, - PageDown: () => { - state.setDragging(true); - state.decrement(state.pageStep); - state.setDragging(false); + onKeyDown(e) { + // these are the cases that useMove doesn't handle + if (!/^(PageUp|PageDown)$/.test(e.key)) { + e.continuePropagation(); + return; + } + // same handling as useMove, don't need to stop propagation, useKeyboard will do that for us + e.preventDefault(); + // remember to set this and unset it so that onChangeEnd is fired + state.setDragging(true); + switch (e.key) { + case 'PageUp': + e.preventDefault(); + state.increment(state.pageStep); + break; + case 'PageDown': + e.preventDefault(); + state.decrement(state.pageStep); + break; } + state.setDragging(false); } }); diff --git a/packages/react-aria/src/combobox/useComboBox.ts b/packages/react-aria/src/combobox/useComboBox.ts index a9006f5d642..7763da45591 100644 --- a/packages/react-aria/src/combobox/useComboBox.ts +++ b/packages/react-aria/src/combobox/useComboBox.ts @@ -15,6 +15,7 @@ import {AriaButtonProps} from '../button/useButton'; import {ariaHideOutside} from '../overlays/ariaHideOutside'; import { AriaLabelingProps, + BaseEvent, DOMAttributes, DOMProps, InputDOMProps, @@ -32,6 +33,7 @@ import {dispatchVirtualFocus} from '../focus/virtualFocus'; import { FocusEvent, InputHTMLAttributes, + KeyboardEvent, TouchEvent, useEffect, useMemo, @@ -51,7 +53,6 @@ import {privateValidationStateProp} from 'react-stately/private/form/useFormVali import {useEvent} from '../utils/useEvent'; import {useFormReset} from '../utils/useFormReset'; import {useId} from '../utils/useId'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLabels} from '../utils/useLabels'; import {useLocalizedStringFormatter} from '../i18n/useLocalizedStringFormatter'; import {useMenuTrigger} from '../menu/useMenuTrigger'; @@ -175,12 +176,19 @@ export function useComboBox( let router = useRouter(); - // for textfield specific operations - let {keyboardProps} = useKeyboard({ - shortcuts: { - Enter: e => { - // Prevent default form submission if menu is open since we may be selecting a option - let shouldPreventDefault = state.isOpen; + // For textfield specific keydown operations + let onKeyDown = (e: BaseEvent>) => { + if (e.nativeEvent.isComposing) { + return; + } + switch (e.key) { + case 'Enter': + case 'Tab': + // Prevent form submission if menu is open since we may be selecting a option + if (state.isOpen && e.key === 'Enter') { + e.preventDefault(); + } + // If the focused item is a link, trigger opening it. Items that are links are not selectable. if (state.isOpen && listBoxRef.current && state.selectionManager.focusedKey != null) { let collectionItem = state.collection.getItem(state.selectionManager.focusedKey); @@ -188,7 +196,7 @@ export function useComboBox( let item = listBoxRef.current.querySelector( `[data-key="${CSS.escape(state.selectionManager.focusedKey.toString())}"]` ); - if (item instanceof HTMLAnchorElement) { + if (e.key === 'Enter' && item instanceof HTMLAnchorElement) { router.open( item, e, @@ -197,56 +205,35 @@ export function useComboBox( ); } state.close(); - return {shouldPreventDefault}; + break; } else if (collectionItem?.props.onAction) { collectionItem.props.onAction(); state.close(); - return {shouldPreventDefault}; + break; } } - state.commit(); - return {shouldPreventDefault}; - }, - Tab: () => { - // If the focused item is a link, trigger opening it. Items that are links are not selectable. - if (state.isOpen && listBoxRef.current && state.selectionManager.focusedKey != null) { - let collectionItem = state.collection.getItem(state.selectionManager.focusedKey); - if (collectionItem?.props.href) { - state.close(); - return {shouldPreventDefault: false}; - } else if (collectionItem?.props.onAction) { - collectionItem.props.onAction(); - state.close(); - return {shouldPreventDefault: false}; - } - } - if (state.isOpen) { + if (e.key === 'Enter' || state.isOpen) { state.commit(); } - return {shouldPreventDefault: false, shouldContinuePropagation: true}; - }, - Escape: () => { - let shouldContinuePropagation = false; + break; + case 'Escape': if (!state.selectionManager.isEmpty || state.inputValue === '' || props.allowsCustomValue) { - shouldContinuePropagation = true; + e.continuePropagation(); } state.revert(); - return {shouldContinuePropagation}; - }, - ArrowDown: () => { + break; + case 'ArrowDown': state.open('first', 'manual'); - }, - ArrowUp: () => { + break; + case 'ArrowUp': state.open('last', 'manual'); - }, - ArrowLeft: () => { - state.selectionManager.setFocusedKey(null); - }, - ArrowRight: () => { + break; + case 'ArrowLeft': + case 'ArrowRight': state.selectionManager.setFocusedKey(null); - } + break; } - }); + }; let onBlur = (e: FocusEvent) => { let blurFromButton = buttonRef?.current && buttonRef.current === e.relatedTarget; @@ -290,7 +277,7 @@ export function useComboBox( : props.isRequired, onChange: state.setInputValue, onKeyDown: !isReadOnly - ? chain(state.isOpen && collectionProps.onKeyDown, keyboardProps.onKeyDown, props.onKeyDown) + ? chain(state.isOpen && collectionProps.onKeyDown, onKeyDown, props.onKeyDown) : props.onKeyDown, onBlur, value: state.inputValue, diff --git a/packages/react-aria/src/datepicker/useDateField.ts b/packages/react-aria/src/datepicker/useDateField.ts index 5383ee5aa7e..cf2d5475dc9 100644 --- a/packages/react-aria/src/datepicker/useDateField.ts +++ b/packages/react-aria/src/datepicker/useDateField.ts @@ -16,6 +16,7 @@ import { DOMProps, GroupDOMAttributes, InputDOMProps, + KeyboardEvent, RefObject, ValidationResult } from '@react-types/shared'; @@ -213,8 +214,16 @@ export function useDateField( } }, fieldProps: mergeProps(domProps, fieldDOMProps, groupProps, focusWithinProps, { - onKeyDown: props.onKeyDown, - onKeyUp: props.onKeyUp, + onKeyDown(e: KeyboardEvent) { + if (props.onKeyDown) { + props.onKeyDown(e); + } + }, + onKeyUp(e: KeyboardEvent) { + if (props.onKeyUp) { + props.onKeyUp(e); + } + }, style: { unicodeBidi: 'isolate' } diff --git a/packages/react-aria/src/datepicker/useDatePickerGroup.ts b/packages/react-aria/src/datepicker/useDatePickerGroup.ts index 8d353286e20..3bf75fbe3df 100644 --- a/packages/react-aria/src/datepicker/useDatePickerGroup.ts +++ b/packages/react-aria/src/datepicker/useDatePickerGroup.ts @@ -2,10 +2,9 @@ import {createFocusManager, getFocusableTreeWalker} from '../focus/FocusScope'; import {DateFieldState} from 'react-stately/useDateFieldState'; import {DatePickerState} from 'react-stately/useDatePickerState'; import {DateRangePickerState} from 'react-stately/useDateRangePickerState'; -import {DOMAttributes, FocusableElement, RefObject} from '@react-types/shared'; -import {getEventTarget} from '../utils/shadowdom/DOMFunctions'; +import {DOMAttributes, FocusableElement, KeyboardEvent, RefObject} from '@react-types/shared'; +import {getEventTarget, nodeContains} from '../utils/shadowdom/DOMFunctions'; import {mergeProps} from '../utils/mergeProps'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLocale} from '../i18n/I18nProvider'; import {useMemo} from 'react'; import {usePress} from '../interactions/usePress'; @@ -18,24 +17,26 @@ export function useDatePickerGroup( let {direction} = useLocale(); let focusManager = useMemo(() => createFocusManager(ref), [ref]); - let {keyboardProps} = useKeyboard({ - shortcuts: { - 'Alt+ArrowDown': () => { - if ('setOpen' in state) { - state.setOpen(true); - } - return false; - }, - 'Alt+ArrowUp': () => { - if ('setOpen' in state) { - state.setOpen(true); - } - return false; - }, - ArrowLeft: e => { - if (disableArrowNavigation) { - return false; - } + // Open the popover on alt + arrow down + let onKeyDown = (e: KeyboardEvent) => { + if (!nodeContains(e.currentTarget, getEventTarget(e) as Element)) { + return; + } + + if (e.altKey && (e.key === 'ArrowDown' || e.key === 'ArrowUp') && 'setOpen' in state) { + e.preventDefault(); + e.stopPropagation(); + state.setOpen(true); + } + + if (disableArrowNavigation) { + return; + } + + switch (e.key) { + case 'ArrowLeft': + e.preventDefault(); + e.stopPropagation(); if (direction === 'rtl') { if (ref.current) { let target = getEventTarget(e) as FocusableElement; @@ -43,19 +44,15 @@ export function useDatePickerGroup( if (prev) { prev.focus(); - return; } } } else { focusManager.focusPrevious(); - return; - } - return false; - }, - ArrowRight: e => { - if (disableArrowNavigation) { - return false; } + break; + case 'ArrowRight': + e.preventDefault(); + e.stopPropagation(); if (direction === 'rtl') { if (ref.current) { let target = getEventTarget(e) as FocusableElement; @@ -63,17 +60,14 @@ export function useDatePickerGroup( if (next) { next.focus(); - return; } } } else { focusManager.focusNext(); - return; } - return false; - } + break; } - }); + }; // Focus the first placeholder segment from the end on mouse down/touch up in the field. let focusLast = () => { @@ -129,7 +123,7 @@ export function useDatePickerGroup( } }); - return mergeProps(pressProps, keyboardProps); + return mergeProps(pressProps, {onKeyDown}); } function findNextSegment(group: Element, fromX: number, direction: number) { diff --git a/packages/react-aria/src/datepicker/useDateSegment.ts b/packages/react-aria/src/datepicker/useDateSegment.ts index 9119e2b7d76..0ecc714760f 100644 --- a/packages/react-aria/src/datepicker/useDateSegment.ts +++ b/packages/react-aria/src/datepicker/useDateSegment.ts @@ -15,7 +15,7 @@ import {DateFieldState, DateSegment} from 'react-stately/useDateFieldState'; import {getActiveElement, nodeContains} from '../utils/shadowdom/DOMFunctions'; import {getScrollParent} from '../utils/getScrollParent'; import {hookData} from './useDateField'; -import {isIOS} from '../utils/platform'; +import {isIOS, isMac} from '../utils/platform'; import {mergeProps} from '../utils/mergeProps'; import {NumberParser} from '@internationalized/number'; import React, {CSSProperties, useMemo, useRef} from 'react'; @@ -26,7 +26,6 @@ import {useDisplayNames} from './useDisplayNames'; import {useEvent} from '../utils/useEvent'; import {useFilter} from '../i18n/useFilter'; import {useId} from '../utils/useId'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLabels} from '../utils/useLabels'; import {useLayoutEffect} from '../utils/useLayoutEffect'; import {useLocale} from '../i18n/I18nProvider'; @@ -126,20 +125,28 @@ export function useDateSegment( } }; - let {keyboardProps} = useKeyboard({ - shortcuts: { - Backspace: () => { - backspace(); - }, - Delete: () => { + let onKeyDown = e => { + // Firefox does not fire selectstart for Ctrl/Cmd + A + // https://bugzilla.mozilla.org/show_bug.cgi?id=1742153 + if (e.key === 'a' && (isMac() ? e.metaKey : e.ctrlKey)) { + e.preventDefault(); + } + + if (e.ctrlKey || e.metaKey || e.shiftKey || e.altKey) { + return; + } + + switch (e.key) { + case 'Backspace': + case 'Delete': { + // Safari on iOS does not fire beforeinput for the backspace key because the cursor is at the start. + e.preventDefault(); + e.stopPropagation(); backspace(); - }, - 'Mod+a': () => { - // Firefox does not fire selectstart for Ctrl/Cmd + A - // https://bugzilla.mozilla.org/show_bug.cgi?id=1742153 + break; } } - }); + }; // Safari dayPeriod option doesn't work... let {startsWith} = useFilter({sensitivity: 'base'}); @@ -387,14 +394,13 @@ export function useDateSegment( segmentProps: mergeProps(spinButtonProps, labelProps, { id, ...touchPropOverrides, - ...keyboardProps, 'aria-invalid': state.isInvalid ? 'true' : undefined, 'aria-describedby': ariaDescribedBy, 'aria-readonly': state.isReadOnly || !segment.isEditable ? 'true' : undefined, 'data-placeholder': segment.isPlaceholder || undefined, contentEditable: isEditable, suppressContentEditableWarning: isEditable, - spellCheck: isEditable ? ('false' as const) : undefined, + spellCheck: isEditable ? 'false' : undefined, autoCorrect: isEditable ? 'off' : undefined, // Capitalization was changed in React 17... [parseInt(React.version, 10) >= 17 ? 'enterKeyHint' : 'enterkeyhint']: isEditable @@ -403,8 +409,9 @@ export function useDateSegment( inputMode: state.isDisabled || segment.type === 'dayPeriod' || segment.type === 'era' || !isEditable ? undefined - : ('numeric' as const), + : 'numeric', tabIndex: state.isDisabled ? undefined : 0, + onKeyDown, onFocus, style: segmentStyle, // Prevent pointer events from reaching useDatePickerGroup, and allow native browser behavior to focus the segment. diff --git a/packages/react-aria/src/interactions/createEventHandler.ts b/packages/react-aria/src/interactions/createEventHandler.ts index d1c08ab279b..18e04c3b7dc 100644 --- a/packages/react-aria/src/interactions/createEventHandler.ts +++ b/packages/react-aria/src/interactions/createEventHandler.ts @@ -24,8 +24,8 @@ export function createEventHandler( return undefined; } + let shouldStopPropagation = true; return (e: T) => { - let shouldStopPropagation = true; let event: BaseEvent = { ...e, preventDefault() { @@ -58,11 +58,7 @@ export function createEventHandler( handler(event); - // nested createEventHandler calls may already have stopped propagation - if ( - shouldStopPropagation && - !(typeof e.isPropagationStopped === 'function' && e.isPropagationStopped()) - ) { + if (shouldStopPropagation) { e.stopPropagation(); } }; diff --git a/packages/react-aria/src/interactions/createKeyboardShortcutHandler.ts b/packages/react-aria/src/interactions/createKeyboardShortcutHandler.ts deleted file mode 100644 index 071bfe3ec64..00000000000 --- a/packages/react-aria/src/interactions/createKeyboardShortcutHandler.ts +++ /dev/null @@ -1,217 +0,0 @@ -/* - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import {isMac} from '../utils/platform'; -import {KeyboardEvent} from '@react-types/shared'; - -export type KeyboardShortcutAction = ( - e: KeyboardEvent -) => - | void - | boolean - | Partial<{shouldContinuePropagation?: boolean; shouldPreventDefault?: boolean}>; - -/** Maps shortcut strings (e.g. `"Mod+s"`, `"Control+Shift+a"`) to handlers. */ -export type KeyboardShortcutBindings = Record; - -/** Modifier names in shortcut strings (case-insensitive). Order in the string does not matter. */ -const MODIFIER_NAMES = new Set([ - 'shift', - 'alt', - 'control', - 'meta', - 'mod' // OS dependent - Cmd on Mac, Control on Windows/Linux -]); - -/** Canonical modifier order for stable keys (sorted, fixed order). */ -const CANONICAL_MODIFIER_ORDER = ['Alt', 'Control', 'Meta', 'Shift'] as const; - -export interface ParsedKeyboardShortcut { - shift: boolean; - alt: boolean; - ctrl: boolean; - meta: boolean; - /** - * Platform primary: Cmd on Mac, Control on Windows/Linux — expands to Meta or Control in - * canonical form. - */ - mod: boolean; - key: string; -} - -/** - * Builds the set of canonical modifier tokens for a binding. - * `Mod` contributes Meta (Mac) or Ctrl (non-Mac); explicit Ctrl/Meta add those keys too. - */ -export function modifierSetFromParsed(parsed: ParsedKeyboardShortcut): Set { - let set = new Set(); - if (parsed.alt) { - set.add('Alt'); - } - if (parsed.shift) { - set.add('Shift'); - } - if (parsed.ctrl) { - set.add('Control'); - } - if (parsed.meta) { - set.add('Meta'); - } - if (parsed.mod) { - set.add(isMac() ? 'Meta' : 'Control'); - } - return set; -} - -/** Modifier set from a keydown event (native flags only). */ -export function modifierSetFromEvent(e: KeyboardEvent): Set { - let set = new Set(); - if (e.altKey) { - set.add('Alt'); - } - if (e.ctrlKey) { - set.add('Control'); - } - if (e.metaKey) { - set.add('Meta'); - } - if (e.shiftKey) { - set.add('Shift'); - } - return set; -} - -function sortedModifierTokens(set: Set): string[] { - return CANONICAL_MODIFIER_ORDER.filter(name => set.has(name)); -} - -/** - * Parses a shortcut like `"Mod+Shift+z"`, `"Ctrl+Alt+Enter"`, or `"Escape"`. - * Modifiers are case-insensitive; order does not matter. `control` is an alias for `ctrl`. - */ -export function parseKeyboardShortcut(spec: string): ParsedKeyboardShortcut { - let parts = spec.split('+').reduce( - (prev, part) => { - let lower = part.toLowerCase(); - if (MODIFIER_NAMES.has(lower)) { - if (lower === 'shift') { - prev.shift = true; - } else if (lower === 'alt') { - prev.alt = true; - } else if (lower === 'control') { - prev.ctrl = true; - } else if (lower === 'meta') { - prev.meta = true; - } else if (lower === 'mod') { - prev.mod = true; - } - } else { - prev.key = part; - } - return prev; - }, - {shift: false, alt: false, ctrl: false, meta: false, mod: false, key: ''} - ); - if (parts.key === '') { - throw new Error( - `Invalid keyboard shortcut: "${spec}". Must include exactly one non-modifier key (e.g. "a", "Enter", "ArrowDown"). Combine any of Shift, Alt, Ctrl, Meta, and Mod.` - ); - } - return parts; -} - -function normalizeEventKey(key: string): string { - return key.toLowerCase(); -} - -/** Short aliases for common keys (shortcut side, before match). */ -const KEY_ALIASES: Record = { - space: ' ', - esc: 'escape', - del: 'delete', - ins: 'insert', - left: 'arrowleft', - right: 'arrowright', - up: 'arrowup', - down: 'arrowdown', - pageup: 'pageup', - pagedown: 'pagedown' -}; - -/** Canonical key segment (lowercase); aliases like `down` → `arrowdown`. */ -function canonicalKeyFromSpecKey(specKey: string): string { - let k = normalizeEventKey(specKey); - let aliased = KEY_ALIASES[k]; - return aliased != null ? aliased : k; -} - -/** Canonical shortcut string for a binding (modifiers sorted: Alt, Ctrl, Meta, Shift, then key). */ -export function canonicalKeyboardShortcut(parsed: ParsedKeyboardShortcut): string { - let mods = sortedModifierTokens(modifierSetFromParsed(parsed)); - let key = canonicalKeyFromSpecKey(parsed.key); - return mods.length > 0 ? `${mods.join('+')}+${key}` : key; -} - -/** Canonical shortcut string for a keydown event. */ -export function keyboardEventToCanonicalShortcut(e: KeyboardEvent): string { - let mods = sortedModifierTokens(modifierSetFromEvent(e)); - let key = normalizeEventKey(e.key); - let prefix = mods.length > 0 ? `${mods.join('+')}+` : ''; - return prefix + key; -} - -/** - * Returns a keydown handler that runs the action only for an exact modifier+key match. - * Modifier order in the string does not matter (`Shift+Mod+a` ≡ `Mod+Shift+a`). - * Any combination of **Shift**, **Alt**, **Ctrl**, **Meta**, and **Mod** is allowed; **Mod** means - * Cmd on Apple platforms and Ctrl on Windows/Linux (same as before). **control** aliases **ctrl**. - * - * Duplicate bindings that normalize to the same shortcut: later object entries win. - * - * @example - * ```tsx - * let onKeyDown = createKeyboardShortcutHandler({ - * 'Mod+s': e => { - * e.preventDefault(); - * save(); - * }, - * 'Ctrl+Shift+k': () => palette(), - * 'Meta+Alt+ArrowLeft': () => back() - * }); - * ```; - */ -export function createKeyboardShortcutHandler( - bindings: KeyboardShortcutBindings -): (e: KeyboardEvent) => void { - let map = new Map(); - for (let [spec, action] of Object.entries(bindings)) { - let parsed = parseKeyboardShortcut(spec); - map.set(canonicalKeyboardShortcut(parsed), action); - } - - return (e: KeyboardEvent) => { - let canonical = keyboardEventToCanonicalShortcut(e); - let action = map.get(canonical); - let result = action?.(e); - if (result === undefined && action !== undefined) { - result = {shouldContinuePropagation: false, shouldPreventDefault: true}; - } else if (typeof result === 'boolean') { - result = {shouldContinuePropagation: !result, shouldPreventDefault: result}; - } - if (result?.shouldPreventDefault) { - e.preventDefault(); - } - if (!action || result?.shouldContinuePropagation) { - e.continuePropagation(); - } - }; -} diff --git a/packages/react-aria/src/interactions/useKeyboard.ts b/packages/react-aria/src/interactions/useKeyboard.ts index 9264a58ac22..f7f99ca827d 100644 --- a/packages/react-aria/src/interactions/useKeyboard.ts +++ b/packages/react-aria/src/interactions/useKeyboard.ts @@ -11,21 +11,11 @@ */ import {createEventHandler} from './createEventHandler'; -import { - createKeyboardShortcutHandler, - KeyboardShortcutBindings -} from './createKeyboardShortcutHandler'; import {DOMAttributes, KeyboardEvents} from '@react-types/shared'; -import {getEventTarget, nodeContains} from '../utils/shadowdom/DOMFunctions'; -import {KeyboardEvent as ReactKeyboardEvent} from 'react'; export interface KeyboardProps extends KeyboardEvents { /** Whether the keyboard events should be disabled. */ isDisabled?: boolean; - /** Keyboard shortcuts to handle. */ - shortcuts?: KeyboardShortcutBindings; - allowRepeats?: boolean; - allowComposing?: boolean; } export interface KeyboardResult { @@ -37,57 +27,12 @@ export interface KeyboardResult { * Handles keyboard interactions for a focusable element. */ export function useKeyboard(props: KeyboardProps): KeyboardResult { - let {shortcuts, allowRepeats = false, allowComposing = false} = props; - let onKeyDown; - let onKeyUp; - if (shortcuts) { - let shortcutHandler = createKeyboardShortcutHandler(shortcuts); - onKeyDown = createEventHandler>(e => { - // If keyboard event didn't originate from a child of the current target, - // then it's a React event coming through a portal. We should ignore it. - if (!nodeContains(e.currentTarget, getEventTarget(e))) { - e.continuePropagation(); - return; - } - if ( - (e.nativeEvent?.repeat && !allowRepeats) || - (e.nativeEvent?.isComposing && !allowComposing) - ) { - e.continuePropagation(); - return; - } - - shortcutHandler(e); - props.onKeyDown?.(e); - }); - onKeyUp = createEventHandler>(e => { - // If keyboard event didn't originate from a child of the current target, - // then it's a React event coming through a portal. We should ignore it. - if (!nodeContains(e.currentTarget, getEventTarget(e))) { - e.continuePropagation(); - return; - } - if ( - (e.nativeEvent?.repeat && !allowRepeats) || - (e.nativeEvent?.isComposing && !allowComposing) - ) { - e.continuePropagation(); - return; - } - // implement shortcut handler on keyup, what should the map be called? or should it be another syntax on shortcuts? - e.continuePropagation(); - props.onKeyUp?.(e); - }); - } else { - onKeyDown = createEventHandler(props.onKeyDown); - onKeyUp = createEventHandler(props.onKeyUp); - } return { keyboardProps: props.isDisabled ? {} : { - onKeyDown, - onKeyUp + onKeyDown: createEventHandler(props.onKeyDown), + onKeyUp: createEventHandler(props.onKeyUp) } }; } diff --git a/packages/react-aria/src/menu/useMenuItem.ts b/packages/react-aria/src/menu/useMenuItem.ts index 8dc2f764860..dbea1ac2b12 100644 --- a/packages/react-aria/src/menu/useMenuItem.ts +++ b/packages/react-aria/src/menu/useMenuItem.ts @@ -314,34 +314,44 @@ export function useMenuItem( }); let {keyboardProps} = useKeyboard({ - shortcuts: { - ' ': e => { - interaction.current = {pointerType: 'keyboard', key: ' '}; - (getEventTarget(e) as HTMLElement).click(); - - // click above sets modality to "virtual", need to set interaction modality back to 'keyboard' so focusSafely calls properly move focus - // to the newly opened submenu's first item. - setInteractionModality('keyboard'); - }, - Enter: e => { - interaction.current = {pointerType: 'keyboard', key: 'Enter'}; - let target = getEventTarget(e) as HTMLElement; - - // Trigger click unless this is a link. Links with real DOM focus activate on Enter natively. - // With virtual focus (e.g. Autocomplete) focus stays on the input and useAutocomplete dispatches - // keydown here then follows with a synthetic click only if dispatchEvent was not canceled—so - // links must not preventDefault on that keydown. - if (target.tagName !== 'A') { - target.click(); + onKeyDown: e => { + // Ignore repeating events, which may have started on the menu trigger before moving + // focus to the menu item. We want to wait for a second complete key press sequence. + if (e.repeat) { + e.continuePropagation(); + return; + } + + switch (e.key) { + case ' ': + interaction.current = {pointerType: 'keyboard', key: ' '}; + (getEventTarget(e) as HTMLElement).click(); + + // click above sets modality to "virtual", need to set interaction modality back to 'keyboard' so focusSafely calls properly move focus + // to the newly opened submenu's first item. setInteractionModality('keyboard'); - return; - } + break; + case 'Enter': + interaction.current = {pointerType: 'keyboard', key: 'Enter'}; - setInteractionModality('keyboard'); - return {shouldPreventDefault: false, shouldContinuePropagation: false}; + // Trigger click unless this is a link. Links trigger click natively. + if ((getEventTarget(e) as HTMLElement).tagName !== 'A') { + (getEventTarget(e) as HTMLElement).click(); + } + + // click above sets modality to "virtual", need to set interaction modality back to 'keyboard' so focusSafely calls properly move focus + // to the newly opened submenu's first item. + setInteractionModality('keyboard'); + break; + default: + if (!isTrigger) { + e.continuePropagation(); + } + + onKeyDown?.(e); + break; } }, - onKeyDown, onKeyUp }); diff --git a/packages/react-aria/src/menu/useMenuTrigger.ts b/packages/react-aria/src/menu/useMenuTrigger.ts index 61605da99ec..2cda3915319 100644 --- a/packages/react-aria/src/menu/useMenuTrigger.ts +++ b/packages/react-aria/src/menu/useMenuTrigger.ts @@ -12,13 +12,12 @@ import {AriaButtonProps} from '../button/useButton'; import {AriaMenuOptions} from './useMenu'; -import {FocusableElement, FocusStrategy, KeyboardEvent, RefObject} from '@react-types/shared'; +import {FocusableElement, RefObject} from '@react-types/shared'; import {focusWithoutScrolling} from '../utils/focusWithoutScrolling'; import intlMessages from '../../intl/menu/*.json'; import {MenuTriggerState, MenuTriggerType} from 'react-stately/useMenuTriggerState'; import {PressProps} from '../interactions/usePress'; import {useId} from '../utils/useId'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLocalizedStringFormatter} from '../i18n/useLocalizedStringFormatter'; import {useLongPress} from '../interactions/useLongPress'; import {useOverlayTrigger} from '../overlays/useOverlayTrigger'; @@ -57,53 +56,50 @@ export function useMenuTrigger( let menuTriggerId = useId(); let {triggerProps, overlayProps} = useOverlayTrigger({type}, state, ref); - let open = ( - shouldOpen: boolean, - e: KeyboardEvent, - focusStrategy: FocusStrategy = 'first' - ): boolean | void => { - if (!shouldOpen || e.isDefaultPrevented()) { - return false; + let onKeyDown = e => { + if (isDisabled) { + return; + } + + if (trigger === 'longPress' && !e.altKey) { + return; } - state.toggle(focusStrategy); - }; - // React puts listeners on the same root, so even if propagation was stopped, immediate propagation is still possible. - // useTypeSelect will handle the spacebar first if it's running, so we don't want to open if it's handled it already. - // We use isDefaultPrevented() instead of isPropagationStopped() because createEventHandler stops propagation by default. - // And default prevented means that the event was handled by something else (typeahead), so we don't want to open the menu. - let {keyboardProps} = useKeyboard({ - isDisabled, - shortcuts: { - Enter: e => { - return open(trigger !== 'longPress', e, 'first'); - }, - ' ': e => { - return open(trigger !== 'longPress', e, 'first'); - }, - ArrowDown: e => { - return open(trigger !== 'longPress', e, 'first'); - }, - ArrowUp: e => { - return open(trigger !== 'longPress', e, 'last'); - }, - 'Alt+Enter': e => { - return open(trigger === 'longPress', e, 'first'); - }, - 'Alt+ ': e => { - return open(trigger === 'longPress', e, 'first'); - }, - // Alt+Arrow* must open for both trigger modes: for `press` it matches the same `e.key` cases as - // plain Arrow*; for `longPress`, plain arrows are ignored elsewhere and Alt+Arrow is the opener - // (see legacy `if (trigger === 'longPress' && !e.altKey) return` before the ArrowDown/Up switch). - 'Alt+ArrowDown': e => { - return open(true, e, 'first'); - }, - 'Alt+ArrowUp': e => { - return open(true, e, 'last'); + if (ref && ref.current) { + switch (e.key) { + case 'Enter': + case ' ': + // React puts listeners on the same root, so even if propagation was stopped, immediate propagation is still possible. + // useTypeSelect will handle the spacebar first if it's running, so we don't want to open if it's handled it already. + // We use isDefaultPrevented() instead of isPropagationStopped() because createEventHandler stops propagation by default. + // And default prevented means that the event was handled by something else (typeahead), so we don't want to open the menu. + if (trigger === 'longPress' || e.isDefaultPrevented()) { + return; + } + // fallthrough + case 'ArrowDown': + // Stop propagation, unless it would already be handled by useKeyboard. + if (!('continuePropagation' in e)) { + e.stopPropagation(); + } + e.preventDefault(); + state.toggle('first'); + break; + case 'ArrowUp': + if (!('continuePropagation' in e)) { + e.stopPropagation(); + } + e.preventDefault(); + state.toggle('last'); + break; + default: + // Allow other keys. + if ('continuePropagation' in e) { + e.continuePropagation(); + } } } - }); + }; let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/menu'); let {longPressProps} = useLongPress({ @@ -148,8 +144,8 @@ export function useMenuTrigger( menuTriggerProps: { ...triggerProps, ...(trigger === 'press' ? pressProps : longPressProps), - ...keyboardProps, - id: menuTriggerId + id: menuTriggerId, + onKeyDown }, menuProps: { ...overlayProps, diff --git a/packages/react-aria/src/menu/useSubmenuTrigger.ts b/packages/react-aria/src/menu/useSubmenuTrigger.ts index 9adaa754e83..21d6ffaf6ca 100644 --- a/packages/react-aria/src/menu/useSubmenuTrigger.ts +++ b/packages/react-aria/src/menu/useSubmenuTrigger.ts @@ -13,7 +13,14 @@ import {AriaMenuItemProps} from './useMenuItem'; import {AriaMenuOptions} from './useMenu'; import type {AriaPopoverProps} from '../overlays/usePopover'; -import {FocusableElement, FocusStrategy, Node, PressEvent, RefObject} from '@react-types/shared'; +import { + FocusableElement, + FocusStrategy, + KeyboardEvent, + Node, + PressEvent, + RefObject +} from '@react-types/shared'; import {focusWithoutScrolling} from '../utils/focusWithoutScrolling'; import { getActiveElement, @@ -26,7 +33,6 @@ import type {SubmenuTriggerState} from 'react-stately/useMenuTriggerState'; import {useCallback, useRef} from 'react'; import {useEvent} from '../utils/useEvent'; import {useId} from '../utils/useId'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLayoutEffect} from '../utils/useLayoutEffect'; import {useLocale} from '../i18n/I18nProvider'; import {useSafelyMouseToSubmenu} from './useSafelyMouseToSubmenu'; @@ -128,51 +134,46 @@ export function useSubmenuTrigger( }; }, [cancelOpenTimeout]); - let {keyboardProps} = useKeyboard({ - shortcuts: { - ArrowLeft: e => { - // If focus is not within the menu, assume virtual focus is being used. - // This means some other input element is also within the popover, so we shouldn't close the menu. - if (!isFocusWithin(e.currentTarget)) { - return false; - } + let submenuKeyDown = (e: KeyboardEvent) => { + // If focus is not within the menu, assume virtual focus is being used. + // This means some other input element is also within the popover, so we shouldn't close the menu. + if (!isFocusWithin(e.currentTarget)) { + return; + } + + switch (e.key) { + case 'ArrowLeft': if (direction === 'ltr' && nodeContains(e.currentTarget, getEventTarget(e) as Element)) { + e.preventDefault(); + e.stopPropagation(); onSubmenuClose(); if (!shouldUseVirtualFocus && ref.current) { focusWithoutScrolling(ref.current); } - return; - } - return false; - }, - ArrowRight: e => { - if (!isFocusWithin(e.currentTarget)) { - return false; } + break; + case 'ArrowRight': if (direction === 'rtl' && nodeContains(e.currentTarget, getEventTarget(e) as Element)) { + e.preventDefault(); + e.stopPropagation(); onSubmenuClose(); if (!shouldUseVirtualFocus && ref.current) { focusWithoutScrolling(ref.current); } - return; - } - return false; - }, - Escape: e => { - if (!isFocusWithin(e.currentTarget)) { - return false; } + break; + case 'Escape': + // TODO: can remove this when we fix collection event leaks if (nodeContains(submenuRef.current, getEventTarget(e) as Element)) { + e.stopPropagation(); onSubmenuClose(); if (!shouldUseVirtualFocus && ref.current) { focusWithoutScrolling(ref.current); } - return; } - return false; - } + break; } - }); + }; let submenuProps = { id: overlayId, @@ -181,15 +182,16 @@ export function useSubmenuTrigger( ...(type === 'menu' && { onClose: state.closeAll, autoFocus: state.focusStrategy ?? undefined, - ...keyboardProps + onKeyDown: submenuKeyDown }) }; - let {keyboardProps: submenuTriggerKeyboardProps} = useKeyboard({ - shortcuts: { - ArrowRight: () => { + let submenuTriggerKeyDown = (e: KeyboardEvent) => { + switch (e.key) { + case 'ArrowRight': if (!isDisabled) { if (direction === 'ltr') { + e.preventDefault(); if (!state.isOpen) { onSubmenuOpen('first'); } @@ -197,19 +199,18 @@ export function useSubmenuTrigger( if (type === 'menu' && !!submenuRef?.current && getActiveElement() === ref?.current) { focusWithoutScrolling(submenuRef.current); } - return; } else if (state.isOpen) { onSubmenuClose(); - return; } else { - return false; + e.continuePropagation(); } } - return false; - }, - ArrowLeft: () => { + + break; + case 'ArrowLeft': if (!isDisabled) { if (direction === 'rtl') { + e.preventDefault(); if (!state.isOpen) { onSubmenuOpen('first'); } @@ -217,18 +218,18 @@ export function useSubmenuTrigger( if (type === 'menu' && !!submenuRef?.current && getActiveElement() === ref?.current) { focusWithoutScrolling(submenuRef.current); } - return; } else if (state.isOpen) { onSubmenuClose(); - return; } else { - return false; + e.continuePropagation(); } } - return false; - } + break; + default: + e.continuePropagation(); + break; } - }); + }; let onPressStart = (e: PressEvent) => { if (!isDisabled && (e.pointerType === 'virtual' || e.pointerType === 'keyboard')) { @@ -288,7 +289,6 @@ export function useSubmenuTrigger( return { submenuTriggerProps: { - ...(submenuTriggerKeyboardProps as any), // TODO: fix this id: submenuTriggerId, 'aria-controls': state.isOpen ? overlayId : undefined, 'aria-haspopup': !isDisabled ? type : undefined, @@ -296,6 +296,7 @@ export function useSubmenuTrigger( onPressStart, onPress, onHoverChange, + onKeyDown: submenuTriggerKeyDown, isOpen: state.isOpen }, submenuProps, diff --git a/packages/react-aria/src/numberfield/useNumberField.ts b/packages/react-aria/src/numberfield/useNumberField.ts index f6360aa4b2f..aebb6446e4c 100644 --- a/packages/react-aria/src/numberfield/useNumberField.ts +++ b/packages/react-aria/src/numberfield/useNumberField.ts @@ -21,6 +21,7 @@ import { TextInputDOMProps, ValidationResult } from '@react-types/shared'; +import {chain} from '../utils/chain'; import { type ClipboardEvent, type ClipboardEventHandler, @@ -44,7 +45,6 @@ import {useFocusWithin} from '../interactions/useFocusWithin'; import {useFormattedTextField} from '../textfield/useFormattedTextField'; import {useFormReset} from '../utils/useFormReset'; import {useId} from '../utils/useId'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLayoutEffect} from '../utils/useLayoutEffect'; import {useLocalizedStringFormatter} from '../i18n/useLocalizedStringFormatter'; import {useNumberFormatter} from '../i18n/useNumberFormatter'; @@ -255,24 +255,28 @@ export function useNumberField( }; let domProps = filterDOMProps(props); - let {keyboardProps} = useKeyboard({ - isDisabled: isDisabled || isReadOnly, - shortcuts: { - Enter: () => { + let onKeyDownEnter = useCallback( + e => { + if (e.nativeEvent.isComposing) { + return; + } + + if (e.key === 'Enter') { flushSync(() => { commit(); }); commitValidation(); + } else { + e.continuePropagation(); } }, - onKeyDown, - onKeyUp - }); + [commit, commitValidation] + ); let {isInvalid, validationErrors, validationDetails} = state.displayValidation; let { labelProps, - inputProps: textFieldPropsFromHook, + inputProps: textFieldProps, descriptionProps, errorMessageProps } = useFormattedTextField( @@ -301,6 +305,8 @@ export function useNumberField( onBlur, onFocus, onFocusChange, + onKeyDown: useMemo(() => chain(onKeyDownEnter, onKeyDown), [onKeyDownEnter, onKeyDown]), + onKeyUp, onPaste, description, errorMessage @@ -309,11 +315,6 @@ export function useNumberField( inputRef ); - // Merge outside useFormattedTextField so useKeyboard's createEventHandler is not nested inside - // useTextField/useFocusable's createEventHandler (avoids redundant stopPropagation on RS events). - // Shortcuts run first (mergeProps chains the second argument after the first). - let textFieldProps = mergeProps(keyboardProps, textFieldPropsFromHook); - useFormReset(inputRef, state.defaultNumberValue, state.setNumberValue); useNativeValidation( state, diff --git a/packages/react-aria/src/overlays/useOverlay.ts b/packages/react-aria/src/overlays/useOverlay.ts index 1f2f476f4f8..1e844bf6427 100644 --- a/packages/react-aria/src/overlays/useOverlay.ts +++ b/packages/react-aria/src/overlays/useOverlay.ts @@ -16,7 +16,6 @@ import {isElementInChildOfActiveScope} from '../focus/FocusScope'; import {useEffect, useRef} from 'react'; import {useFocusWithin} from '../interactions/useFocusWithin'; import {useInteractOutside} from '../interactions/useInteractOutside'; -import {useKeyboard} from '../interactions/useKeyboard'; export interface AriaOverlayProps { /** Whether the overlay is currently open. */ @@ -126,17 +125,13 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject { - if (!isKeyboardDismissDisabled) { - onHide(); - return; - } - return false; - } + let onKeyDown = e => { + if (e.key === 'Escape' && !isKeyboardDismissDisabled && !e.nativeEvent.isComposing) { + e.stopPropagation(); + e.preventDefault(); + onHide(); } - }); + }; // Handle clicking outside the overlay to close it useInteractOutside({ @@ -172,7 +167,7 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject { + let nextDir; + switch (e.key) { + case 'ArrowRight': + if (direction === 'rtl' && orientation !== 'vertical') { + nextDir = 'prev'; + } else { + nextDir = 'next'; + } + break; + case 'ArrowLeft': + if (direction === 'rtl' && orientation !== 'vertical') { + nextDir = 'next'; + } else { + nextDir = 'prev'; + } + break; + case 'ArrowDown': + nextDir = 'next'; + break; + case 'ArrowUp': + nextDir = 'prev'; + break; + default: + return; + } + e.preventDefault(); let walker = getFocusableTreeWalker(e.currentTarget, { from: getEventTarget(e) as Element, accept: node => node instanceof getOwnerWindow(node).HTMLInputElement && node.type === 'radio' @@ -109,36 +134,12 @@ export function useRadioGroup(props: AriaRadioGroupProps, state: RadioGroupState nextElem = walker.lastChild(); } } - if (nextElem) { // Call focus on nextElem so that keyboard navigation scrolls the radio into view nextElem.focus(); state.setSelectedValue(nextElem.value); - return true; } - return false; - } - - let {keyboardProps} = useKeyboard({ - shortcuts: { - ArrowRight: e => { - let nextDir: 'next' | 'prev' = - direction === 'rtl' && orientation !== 'vertical' ? 'prev' : 'next'; - return getNextElement(nextDir, e); - }, - ArrowLeft: e => { - let nextDir: 'next' | 'prev' = - direction === 'rtl' && orientation !== 'vertical' ? 'next' : 'prev'; - return getNextElement(nextDir, e); - }, - ArrowDown: e => { - return getNextElement('next', e); - }, - ArrowUp: e => { - return getNextElement('prev', e); - } - } - }); + }; let groupName = useId(name); radioGroupData.set(state, { @@ -153,7 +154,7 @@ export function useRadioGroup(props: AriaRadioGroupProps, state: RadioGroupState radioGroupProps: mergeProps(domProps, { // https://www.w3.org/TR/wai-aria-1.2/#radiogroup role: 'radiogroup', - ...keyboardProps, + onKeyDown, 'aria-invalid': state.isInvalid || undefined, 'aria-errormessage': props['aria-errormessage'], 'aria-readonly': isReadOnly || undefined, diff --git a/packages/react-aria/src/searchfield/useSearchField.ts b/packages/react-aria/src/searchfield/useSearchField.ts index 6278919130f..bde44998ae8 100644 --- a/packages/react-aria/src/searchfield/useSearchField.ts +++ b/packages/react-aria/src/searchfield/useSearchField.ts @@ -12,13 +12,11 @@ import {AriaButtonProps} from '../button/useButton'; import {AriaTextFieldProps, useTextField} from '../textfield/useTextField'; +import {chain} from '../utils/chain'; import {DOMAttributes, RefObject, ValidationResult} from '@react-types/shared'; import {InputHTMLAttributes, LabelHTMLAttributes} from 'react'; import intlMessages from '../../intl/searchfield/*.json'; -// @ts-ignore -import {mergeProps} from '../utils/mergeProps'; import {SearchFieldProps, SearchFieldState} from 'react-stately/useSearchFieldState'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLocalizedStringFormatter} from '../i18n/useLocalizedStringFormatter'; export interface AriaSearchFieldProps extends SearchFieldProps, Omit { @@ -65,29 +63,38 @@ export function useSearchField( let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/searchfield'); let {isDisabled, isReadOnly, onSubmit, onClear, type = 'search'} = props; - let {keyboardProps} = useKeyboard({ - isDisabled: isDisabled || isReadOnly, - shortcuts: { - Enter: () => { - if (onSubmit) { - // for backward compatibility; - // otherwise, "Enter" on an input would trigger a form submit, the default browser behavior - onSubmit(state.value); - return; - } - return false; - }, - Escape: () => { - // Also check the inputRef value for the case where the value was set directly on the input element instead of going through - // the hook - if (state.value === '' && (!inputRef.current || inputRef.current.value === '')) { - return false; - } + let onKeyDown = e => { + const key = e.key; + + if (key === 'Enter' && (isDisabled || isReadOnly)) { + e.preventDefault(); + } + + if (isDisabled || isReadOnly) { + return; + } + + // for backward compatibility; + // otherwise, "Enter" on an input would trigger a form submit, the default browser behavior + if (key === 'Enter' && onSubmit) { + e.preventDefault(); + onSubmit(state.value); + } + + if (key === 'Escape') { + // Also check the inputRef value for the case where the value was set directly on the input element instead of going through + // the hook + if (state.value === '' && (!inputRef.current || inputRef.current.value === '')) { + e.continuePropagation(); + } else { + e.preventDefault(); state.setValue(''); - onClear?.(); + if (onClear) { + onClear(); + } } } - }); + }; let onClearButtonClick = () => { state.setValue(''); @@ -108,8 +115,7 @@ export function useSearchField( ...props, value: state.value, onChange: state.setValue, - onKeyDown: props.onKeyDown, - onKeyUp: props.onKeyUp, + onKeyDown: !isReadOnly ? chain(onKeyDown, props.onKeyDown) : props.onKeyDown, type }, inputRef @@ -117,14 +123,11 @@ export function useSearchField( return { labelProps, - // An edge case, in Autocomplete, if the keyboard hanlders are not in this order, then - // Escape runs autocomplete/listbox first, then the search-field shortcut returns false and - // continues propagation, leaking Escape to a parent Dialog. - inputProps: mergeProps(keyboardProps, { + inputProps: { ...inputProps, // already handled by useSearchFieldState defaultValue: undefined - }), + }, clearButtonProps: { 'aria-label': stringFormatter.format('Clear search'), excludeFromTabOrder: true, diff --git a/packages/react-aria/src/select/useSelect.ts b/packages/react-aria/src/select/useSelect.ts index 7d561220454..d1aee0733b2 100644 --- a/packages/react-aria/src/select/useSelect.ts +++ b/packages/react-aria/src/select/useSelect.ts @@ -34,7 +34,6 @@ import {setInteractionModality} from '../interactions/useFocusVisible'; import {useCollator} from '../i18n/useCollator'; import {useField} from '../label/useField'; import {useId} from '../utils/useId'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useMenuTrigger} from '../menu/useMenuTrigger'; import {useTypeSelect} from '../selection/useTypeSelect'; @@ -137,12 +136,16 @@ export function useSelect( ref ); - let {keyboardProps} = useKeyboard({ - shortcuts: { - ArrowLeft: () => { - if (state.selectionManager.selectionMode === 'multiple') { - return false; - } + let onKeyDown = (e: KeyboardEvent) => { + if (state.selectionManager.selectionMode === 'multiple') { + return; + } + + switch (e.key) { + case 'ArrowLeft': { + // prevent scrolling containers + e.preventDefault(); + let key = state.selectedKey != null ? delegate.getKeyAbove?.(state.selectedKey) @@ -150,11 +153,12 @@ export function useSelect( if (key != null) { state.setSelectedKey(key); } - }, - ArrowRight: () => { - if (state.selectionManager.selectionMode === 'multiple') { - return false; - } + break; + } + case 'ArrowRight': { + // prevent scrolling containers + e.preventDefault(); + let key = state.selectedKey != null ? delegate.getKeyBelow?.(state.selectedKey) @@ -162,11 +166,10 @@ export function useSelect( if (key != null) { state.setSelectedKey(key); } + break; } - }, - onKeyDown: props.onKeyDown, - onKeyUp: props.onKeyUp - }); + } + }; let {typeSelectProps} = useTypeSelect({ keyboardDelegate: delegate, @@ -216,8 +219,8 @@ export function useSelect( triggerProps: mergeProps(domProps, { ...triggerProps, isDisabled, - onKeyDown: chain(triggerProps.onKeyDown, keyboardProps.onKeyDown), - onKeyUp: keyboardProps.onKeyUp, + onKeyDown: chain(triggerProps.onKeyDown, onKeyDown, props.onKeyDown), + onKeyUp: props.onKeyUp, 'aria-labelledby': [ valueId, triggerProps['aria-labelledby'], diff --git a/packages/react-aria/src/selection/useSelectableCollection.ts b/packages/react-aria/src/selection/useSelectableCollection.ts index b06482a1086..a834398112d 100644 --- a/packages/react-aria/src/selection/useSelectableCollection.ts +++ b/packages/react-aria/src/selection/useSelectableCollection.ts @@ -19,11 +19,10 @@ import { FocusStrategy, Key, KeyboardDelegate, - KeyboardEvent, RefObject } from '@react-types/shared'; import {flushSync} from 'react-dom'; -import {FocusEvent, useEffect, useRef} from 'react'; +import {FocusEvent, KeyboardEvent, useEffect, useRef} from 'react'; import {focusSafely} from '../interactions/focusSafely'; import {focusWithoutScrolling} from '../utils/focusWithoutScrolling'; import { @@ -36,13 +35,11 @@ import {getFocusableTreeWalker} from '../focus/FocusScope'; import {getInteractionModality} from '../interactions/useFocusVisible'; import {getItemElement, isNonContiguousSelectionModifier, useCollectionId} from './utils'; import {isCtrlKeyPressed} from '../utils/keyboard'; -import {isMac} from '../utils/platform'; import {isTabbable} from '../utils/isFocusable'; import {mergeProps} from '../utils/mergeProps'; import {MultipleSelectionManager} from 'react-stately/useMultipleSelectionState'; import {scrollIntoView, scrollIntoViewport} from '../utils/scrollIntoView'; import {useEvent} from '../utils/useEvent'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLocale} from '../i18n/I18nProvider'; import {useRouter} from '../utils/openLink'; import {useTypeSelect} from './useTypeSelect'; @@ -170,266 +167,234 @@ export function useSelectableCollection( let {direction} = useLocale(); let router = useRouter(); - const navigateToKey = ( - e: KeyboardEvent, - key: Key | undefined, - childFocus?: FocusStrategy - ): boolean | void => { - if (key != null) { - if ( - manager.isLink(key) && - linkBehavior === 'selection' && - selectOnFocus && - !isNonContiguousSelectionModifier(e) - ) { - // Set focused key and re-render synchronously to bring item into view if needed. - flushSync(() => { - manager.setFocusedKey(key, childFocus); - }); - - let item = getItemElement(ref, key); - let itemProps = manager.getItemProps(key); - if (item) { - router.open(item, e, itemProps.href, itemProps.routerOptions); - return; - } - - return false; - } - - manager.setFocusedKey(key, childFocus); - - if (manager.isLink(key) && linkBehavior === 'override') { - return false; - } - - if (e.shiftKey && manager.selectionMode === 'multiple') { - manager.extendSelection(key); - return; - } else if (selectOnFocus && !isNonContiguousSelectionModifier(e)) { - manager.replaceSelection(key); - return; - } + let onKeyDown = (e: KeyboardEvent) => { + // Prevent option + tab from doing anything since it doesn't move focus to the cells, only buttons/checkboxes + if (e.altKey && e.key === 'Tab') { + e.preventDefault(); } - return false; - }; - let arrowDown = (e: KeyboardEvent) => { - if (delegate.getKeyBelow) { - let nextKey = - manager.focusedKey != null - ? delegate.getKeyBelow?.(manager.focusedKey) - : delegate.getFirstKey?.(); - if (nextKey == null && shouldFocusWrap) { - nextKey = delegate.getFirstKey?.(manager.focusedKey); - } - if (nextKey != null) { - navigateToKey(e, nextKey); - return {shouldPreventDefault: true, shouldContinuePropagation: true}; - } + // Keyboard events bubble through portals. Don't handle keyboard events + // for elements outside the collection (e.g. menus). + if (!ref.current || !nodeContains(ref.current, getEventTarget(e) as Element)) { + return; } - return false; - }; - let arrowUp = (e: KeyboardEvent) => { - if (delegate.getKeyAbove) { - let nextKey = - manager.focusedKey != null - ? delegate.getKeyAbove?.(manager.focusedKey) - : delegate.getLastKey?.(); - if (nextKey == null && shouldFocusWrap) { - nextKey = delegate.getLastKey?.(manager.focusedKey); - } - if (nextKey != null) { - navigateToKey(e, nextKey); - return {shouldPreventDefault: true, shouldContinuePropagation: true}; - } - } - return false; - }; + const navigateToKey = (key: Key | undefined, childFocus?: FocusStrategy) => { + if (key != null) { + if ( + manager.isLink(key) && + linkBehavior === 'selection' && + selectOnFocus && + !isNonContiguousSelectionModifier(e) + ) { + // Set focused key and re-render synchronously to bring item into view if needed. + flushSync(() => { + manager.setFocusedKey(key, childFocus); + }); + + let item = getItemElement(ref, key); + let itemProps = manager.getItemProps(key); + if (item) { + router.open(item, e, itemProps.href, itemProps.routerOptions); + } - let home = (e: KeyboardEvent) => { - if (delegate.getFirstKey) { - if (manager.focusedKey === null && e.shiftKey) { - return false; - } - // TODO: should Home and End also be reversed in column reverse aka Home goes to top? Or should Home always to to the "first" (bottom) - let firstKey: Key | null = delegate.getFirstKey(manager.focusedKey, isCtrlKeyPressed(e)); - manager.setFocusedKey(firstKey); - if (firstKey != null) { - if (isCtrlKeyPressed(e) && e.shiftKey && manager.selectionMode === 'multiple') { - manager.extendSelection(firstKey); - return; - } else if (selectOnFocus) { - manager.replaceSelection(firstKey); return; } - } - } - return false; - }; - let arrowLeft = (e: KeyboardEvent) => { - if (delegate.getKeyLeftOf) { - let nextKey: Key | undefined | null = - manager.focusedKey != null - ? delegate.getKeyLeftOf?.(manager.focusedKey) - : delegate.getFirstKey?.(); - if (nextKey == null && shouldFocusWrap) { - nextKey = - direction === 'rtl' - ? delegate.getFirstKey?.(manager.focusedKey) - : delegate.getLastKey?.(manager.focusedKey); - } - if (nextKey != null) { - navigateToKey(e, nextKey, direction === 'rtl' ? 'first' : 'last'); - return {shouldPreventDefault: true, shouldContinuePropagation: true}; - } - } - return false; - }; + manager.setFocusedKey(key, childFocus); - let arrowRight = (e: KeyboardEvent) => { - if (delegate.getKeyRightOf) { - let nextKey: Key | undefined | null = - manager.focusedKey != null - ? delegate.getKeyRightOf?.(manager.focusedKey) - : delegate.getFirstKey?.(); - if (nextKey == null && shouldFocusWrap) { - nextKey = - direction === 'rtl' - ? delegate.getLastKey?.(manager.focusedKey) - : delegate.getFirstKey?.(manager.focusedKey); - } - if (nextKey != null) { - navigateToKey(e, nextKey, direction === 'rtl' ? 'last' : 'first'); - return {shouldPreventDefault: true, shouldContinuePropagation: true}; - } - } - return false; - }; - - let end = (e: KeyboardEvent) => { - if (delegate.getLastKey) { - if (manager.focusedKey === null && e.shiftKey) { - return false; - } - let lastKey = delegate.getLastKey(manager.focusedKey, isCtrlKeyPressed(e)); - manager.setFocusedKey(lastKey); - if (lastKey != null) { - if (isCtrlKeyPressed(e) && e.shiftKey && manager.selectionMode === 'multiple') { - manager.extendSelection(lastKey); - return; - } else if (selectOnFocus) { - manager.replaceSelection(lastKey); + if (manager.isLink(key) && linkBehavior === 'override') { return; } - } - } - return false; - }; - let pageDown = (e: KeyboardEvent) => { - if (delegate.getKeyPageBelow && manager.focusedKey != null) { - let nextKey = delegate.getKeyPageBelow(manager.focusedKey); - if (nextKey != null) { - return navigateToKey(e, nextKey); + if (e.shiftKey && manager.selectionMode === 'multiple') { + manager.extendSelection(key); + } else if (selectOnFocus && !isNonContiguousSelectionModifier(e)) { + manager.replaceSelection(key); + } } - } - return false; - }; + }; - let pageUp = (e: KeyboardEvent) => { - if (delegate.getKeyPageAbove && manager.focusedKey != null) { - let nextKey = delegate.getKeyPageAbove(manager.focusedKey); - if (nextKey != null) { - return navigateToKey(e, nextKey); + switch (e.key) { + case 'ArrowDown': { + if (delegate.getKeyBelow) { + let nextKey = + manager.focusedKey != null + ? delegate.getKeyBelow?.(manager.focusedKey) + : delegate.getFirstKey?.(); + if (nextKey == null && shouldFocusWrap) { + nextKey = delegate.getFirstKey?.(manager.focusedKey); + } + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } + } + break; + } + case 'ArrowUp': { + if (delegate.getKeyAbove) { + let nextKey = + manager.focusedKey != null + ? delegate.getKeyAbove?.(manager.focusedKey) + : delegate.getLastKey?.(); + if (nextKey == null && shouldFocusWrap) { + nextKey = delegate.getLastKey?.(manager.focusedKey); + } + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } + } + break; + } + case 'ArrowLeft': { + if (delegate.getKeyLeftOf) { + let nextKey: Key | undefined | null = + manager.focusedKey != null + ? delegate.getKeyLeftOf?.(manager.focusedKey) + : delegate.getFirstKey?.(); + if (nextKey == null && shouldFocusWrap) { + nextKey = + direction === 'rtl' + ? delegate.getFirstKey?.(manager.focusedKey) + : delegate.getLastKey?.(manager.focusedKey); + } + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey, direction === 'rtl' ? 'first' : 'last'); + } + } + break; + } + case 'ArrowRight': { + if (delegate.getKeyRightOf) { + let nextKey: Key | undefined | null = + manager.focusedKey != null + ? delegate.getKeyRightOf?.(manager.focusedKey) + : delegate.getFirstKey?.(); + if (nextKey == null && shouldFocusWrap) { + nextKey = + direction === 'rtl' + ? delegate.getLastKey?.(manager.focusedKey) + : delegate.getFirstKey?.(manager.focusedKey); + } + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey, direction === 'rtl' ? 'last' : 'first'); + } + } + break; } - } - return false; - }; - - let aHandler = () => { - if (manager.selectionMode === 'multiple' && disallowSelectAll !== true) { - manager.selectAll(); - return; - } - return false; - }; - - let escape = () => { - if ( - escapeKeyBehavior === 'clearSelection' && - !disallowEmptySelection && - manager.selectedKeys.size !== 0 - ) { - manager.clearSelection(); - return; - } - return false; - }; - - let tab = () => { - if (!allowsTabNavigation && ref.current) { - // There may be elements that are "tabbable" inside a collection (e.g. in a grid cell). - // However, collections should be treated as a single tab stop, with arrow key navigation internally. - // We don't control the rendering of these, so we can't override the tabIndex to prevent tabbing. - // Instead, we handle the Tab key, and move focus manually to the first/last tabbable element - // in the collection, so that the browser default behavior will apply starting from that element - // rather than the currently focused one. - - let walker = getFocusableTreeWalker(ref.current, {tabbable: true}); - let next: FocusableElement | undefined = undefined; - let last: FocusableElement; - do { - last = walker.lastChild() as FocusableElement; - if (last) { - next = last; + case 'Home': + if (delegate.getFirstKey) { + if (manager.focusedKey === null && e.shiftKey) { + return; + } + // TODO: should Home and End also be reversed in column reverse aka Home goes to top? Or should Home always to to the "first" (bottom) + e.preventDefault(); + let firstKey: Key | null = delegate.getFirstKey(manager.focusedKey, isCtrlKeyPressed(e)); + manager.setFocusedKey(firstKey); + if (firstKey != null) { + if (isCtrlKeyPressed(e) && e.shiftKey && manager.selectionMode === 'multiple') { + manager.extendSelection(firstKey); + } else if (selectOnFocus) { + manager.replaceSelection(firstKey); + } + } + } + break; + case 'End': + if (delegate.getLastKey) { + if (manager.focusedKey === null && e.shiftKey) { + return; + } + e.preventDefault(); + let lastKey = delegate.getLastKey(manager.focusedKey, isCtrlKeyPressed(e)); + manager.setFocusedKey(lastKey); + if (lastKey != null) { + if (isCtrlKeyPressed(e) && e.shiftKey && manager.selectionMode === 'multiple') { + manager.extendSelection(lastKey); + } else if (selectOnFocus) { + manager.replaceSelection(lastKey); + } + } + } + break; + case 'PageDown': + if (delegate.getKeyPageBelow && manager.focusedKey != null) { + let nextKey = delegate.getKeyPageBelow(manager.focusedKey); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } + } + break; + case 'PageUp': + if (delegate.getKeyPageAbove && manager.focusedKey != null) { + let nextKey = delegate.getKeyPageAbove(manager.focusedKey); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } + } + break; + case 'a': + if ( + isCtrlKeyPressed(e) && + manager.selectionMode === 'multiple' && + disallowSelectAll !== true + ) { + e.preventDefault(); + manager.selectAll(); + } + break; + case 'Escape': + if ( + escapeKeyBehavior === 'clearSelection' && + !disallowEmptySelection && + manager.selectedKeys.size !== 0 + ) { + e.stopPropagation(); + e.preventDefault(); + manager.clearSelection(); + } + break; + case 'Tab': { + if (!allowsTabNavigation) { + // There may be elements that are "tabbable" inside a collection (e.g. in a grid cell). + // However, collections should be treated as a single tab stop, with arrow key navigation internally. + // We don't control the rendering of these, so we can't override the tabIndex to prevent tabbing. + // Instead, we handle the Tab key, and move focus manually to the first/last tabbable element + // in the collection, so that the browser default behavior will apply starting from that element + // rather than the currently focused one. + if (e.shiftKey) { + ref.current.focus(); + } else { + let walker = getFocusableTreeWalker(ref.current, {tabbable: true}); + let next: FocusableElement | undefined = undefined; + let last: FocusableElement; + do { + last = walker.lastChild() as FocusableElement; + // oxlint-disable-next-line max-depth + if (last) { + next = last; + } + } while (last); + + // If the active element is NOT tabbable but is contained by an element that IS tabbable (aka the cell), the browser will actually move focus to + // the containing element. We need to special case this so that tab will move focus out of the grid instead of looping between + // focusing the containing cell and back to the non-tabbable child element + let activeElement = getActiveElement(); + if (next && (!isFocusWithin(next) || (activeElement && !isTabbable(activeElement)))) { + focusWithoutScrolling(next); + } + } + break; } - } while (last); - - // If the active element is NOT tabbable but is contained by an element that IS tabbable (aka the cell), the browser will actually move focus to - // the containing element. We need to special case this so that tab will move focus out of the grid instead of looping between - // focusing the containing cell and back to the non-tabbable child element - let activeElement = getActiveElement(); - if (next && (!isFocusWithin(next) || (activeElement && !isTabbable(activeElement)))) { - focusWithoutScrolling(next); } } - return {shouldContinuePropagation: true, shouldPreventDefault: false}; - }; - - let shiftTab = () => { - if (!allowsTabNavigation && ref.current) { - ref.current.focus(); - } - return {shouldContinuePropagation: true, shouldPreventDefault: false}; - }; - - let withShiftSel = (key, callback) => { - return { - [isMac() ? key + '+Shift+Alt' : key + '+Shift+Control']: callback, - [key + '+Shift']: callback, - [isMac() ? key + '+Alt' : key + '+Control']: callback, - [key]: callback - }; }; - let {keyboardProps} = useKeyboard({ - shortcuts: { - ...withShiftSel('ArrowDown', arrowDown), - ...withShiftSel('ArrowUp', arrowUp), - ...withShiftSel('ArrowLeft', arrowLeft), - ...withShiftSel('ArrowRight', arrowRight), - ...withShiftSel('Home', home), - ...withShiftSel('End', end), - ...withShiftSel('PageDown', pageDown), - ...withShiftSel('PageUp', pageUp), - [isMac() ? 'a+Alt' : 'a+Control']: aHandler, - Escape: escape, - Tab: tab, - 'Tab+Shift': shiftTab - } - }); // Store the scroll position so we can restore it later. /// TODO: should this happen all the time?? @@ -695,7 +660,7 @@ export function useSelectableCollection( }); let handlers = { - ...keyboardProps, + onKeyDown, onFocus, onBlur, onMouseDown(e) { diff --git a/packages/react-aria/src/slider/useSliderThumb.ts b/packages/react-aria/src/slider/useSliderThumb.ts index a077d07baab..efaaa0b50f8 100644 --- a/packages/react-aria/src/slider/useSliderThumb.ts +++ b/packages/react-aria/src/slider/useSliderThumb.ts @@ -142,35 +142,41 @@ export function useSliderThumb(opts: AriaSliderThumbOptions, state: SliderState) let reverseX = direction === 'rtl'; let currentPosition = useRef(null); - let keyboardUpdate = cb => { - // remember to set this so that onChangeEnd is fired - state.setThumbDragging(index, true); - cb(); - state.setThumbDragging(index, false); - }; - let {keyboardProps} = useKeyboard({ - shortcuts: { - PageUp: () => { - return keyboardUpdate(() => { - state.incrementThumb(index, state.pageSize); - }); - }, - PageDown: () => { - return keyboardUpdate(() => { - state.decrementThumb(index, state.pageSize); - }); - }, - Home: () => { - return keyboardUpdate(() => { - state.setThumbValue(index, state.getThumbMinValue(index)); - }); - }, - End: () => { - return keyboardUpdate(() => { - state.setThumbValue(index, state.getThumbMaxValue(index)); - }); + onKeyDown(e) { + let { + getThumbMaxValue, + getThumbMinValue, + decrementThumb, + incrementThumb, + setThumbValue, + setThumbDragging, + pageSize + } = state; + // these are the cases that useMove or useSlider don't handle + if (!/^(PageUp|PageDown|Home|End)$/.test(e.key)) { + e.continuePropagation(); + return; + } + // same handling as useMove, stopPropagation to prevent useSlider from handling the event as well. + e.preventDefault(); + // remember to set this so that onChangeEnd is fired + setThumbDragging(index, true); + switch (e.key) { + case 'PageUp': + incrementThumb(index, pageSize); + break; + case 'PageDown': + decrementThumb(index, pageSize); + break; + case 'Home': + setThumbValue(index, getThumbMinValue(index)); + break; + case 'End': + setThumbValue(index, getThumbMaxValue(index)); + break; } + setThumbDragging(index, false); } }); diff --git a/packages/react-aria/src/spinbutton/useSpinButton.ts b/packages/react-aria/src/spinbutton/useSpinButton.ts index 171419b46ac..39e214ef995 100644 --- a/packages/react-aria/src/spinbutton/useSpinButton.ts +++ b/packages/react-aria/src/spinbutton/useSpinButton.ts @@ -18,7 +18,6 @@ import intlMessages from '../../intl/spinbutton/*.json'; import {useCallback, useEffect, useRef, useState} from 'react'; import {useEffectEvent} from '../utils/useEffectEvent'; import {useGlobalListeners} from '../utils/useGlobalListeners'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLocalizedStringFormatter} from '../i18n/useLocalizedStringFormatter'; const noop = () => {}; @@ -72,61 +71,61 @@ export function useSpinButton(props: SpinButtonProps): SpinbuttonAria { return () => clearAsyncEvent(); }, []); - let {keyboardProps} = useKeyboard({ - isDisabled: isDisabled || isReadOnly, - shortcuts: { - PageUp: () => { + let onKeyDown = e => { + if ( + e.ctrlKey || + e.metaKey || + e.shiftKey || + e.altKey || + isReadOnly || + e.nativeEvent.isComposing + ) { + return; + } + + switch (e.key) { + case 'PageUp': if (onIncrementPage) { - onIncrementPage(); - return; + e.preventDefault(); + onIncrementPage?.(); + break; } + // fallthrough! + case 'ArrowUp': + case 'Up': if (onIncrement) { - onIncrement(); - return; - } - return false; - }, - ArrowUp: () => { - if (onIncrement) { - onIncrement(); - return; + e.preventDefault(); + onIncrement?.(); } - return false; - }, - PageDown: () => { + break; + case 'PageDown': if (onDecrementPage) { - onDecrementPage(); - return; + e.preventDefault(); + onDecrementPage?.(); + break; } + // fallthrough + case 'ArrowDown': + case 'Down': if (onDecrement) { - onDecrement(); - return; - } - return false; - }, - ArrowDown: () => { - if (onDecrement) { - onDecrement(); - return; + e.preventDefault(); + onDecrement?.(); } - return false; - }, - Home: () => { + break; + case 'Home': if (onDecrementToMin) { - onDecrementToMin(); - return; + e.preventDefault(); + onDecrementToMin?.(); } - return false; - }, - End: () => { + break; + case 'End': if (onIncrementToMax) { - onIncrementToMax(); - return; + e.preventDefault(); + onIncrementToMax?.(); } - return false; - } + break; } - }); + }; let isFocused = useRef(false); let onFocus = () => { @@ -245,7 +244,7 @@ export function useSpinButton(props: SpinButtonProps): SpinbuttonAria { 'aria-disabled': isDisabled || undefined, 'aria-readonly': isReadOnly || undefined, 'aria-required': isRequired || undefined, - ...keyboardProps, + onKeyDown, onFocus, onBlur }, diff --git a/packages/react-aria/src/steplist/useStepListItem.ts b/packages/react-aria/src/steplist/useStepListItem.ts index 802fa7ff89a..ede27181d74 100644 --- a/packages/react-aria/src/steplist/useStepListItem.ts +++ b/packages/react-aria/src/steplist/useStepListItem.ts @@ -47,9 +47,21 @@ export function useStepListItem( const isSelected = selectedKey === key; + let onKeyDown = event => { + const {key: eventKey} = event; + + if (eventKey === 'ArrowDown' || eventKey === 'ArrowUp') { + event.preventDefault(); + event.stopPropagation(); + } + + itemProps.onKeyDown?.(event); + }; + return { stepProps: { ...itemProps, + onKeyDown, role: 'link', 'aria-current': isSelected ? 'step' : undefined, 'aria-disabled': isDisabled ? true : undefined, diff --git a/packages/react-aria/src/table/useTableColumnResize.ts b/packages/react-aria/src/table/useTableColumnResize.ts index 5dd4c15d431..dc0b1f02f5d 100644 --- a/packages/react-aria/src/table/useTableColumnResize.ts +++ b/packages/react-aria/src/table/useTableColumnResize.ts @@ -141,31 +141,20 @@ export function useTableColumnResize( [state, triggerRef, onResizeEnd] ); - let endResizeEvent = () => { - if (editModeEnabled) { - endResize(item); - return; - } - return false; - }; - let {keyboardProps} = useKeyboard({ - shortcuts: { - Escape: () => { - return endResizeEvent(); - }, - Enter: () => { - if (editModeEnabled) { + onKeyDown: e => { + if (editModeEnabled) { + if (e.key === 'Escape' || e.key === 'Enter' || e.key === ' ' || e.key === 'Tab') { + e.preventDefault(); endResize(item); - } else { + } + } else { + // Continue propagation on keydown events so they still bubbles to useSelectableCollection and are handled there + e.continuePropagation(); + + if (e.key === 'Enter') { startResize(item); } - }, - ' ': () => { - return endResizeEvent(); - }, - Tab: () => { - return endResizeEvent(); } } }); diff --git a/packages/react-aria/src/tag/useTag.ts b/packages/react-aria/src/tag/useTag.ts index 84479e03603..c5b4ea5df25 100644 --- a/packages/react-aria/src/tag/useTag.ts +++ b/packages/react-aria/src/tag/useTag.ts @@ -15,6 +15,7 @@ import {DOMAttributes, FocusableElement, Node, RefObject} from '@react-types/sha import {filterDOMProps} from '../utils/filterDOMProps'; import {hookData} from './useTagGroup'; import intlMessages from '../../intl/tag/*.json'; +import {KeyboardEvent} from 'react'; import type {ListState} from 'react-stately/useListState'; import {mergeProps} from '../utils/mergeProps'; import {SelectableItemStates} from '../selection/useSelectableItem'; @@ -23,7 +24,6 @@ import {useFocusable} from '../interactions/useFocusable'; import {useGridListItem} from '../gridlist/useGridListItem'; import {useId} from '../utils/useId'; import {useInteractionModality} from '../interactions/useFocusVisible'; -import {useKeyboard} from '../interactions/useKeyboard'; import {useLocalizedStringFormatter} from '../i18n/useLocalizedStringFormatter'; import {useSyntheticLinkProps} from '../utils/openLink'; @@ -72,25 +72,20 @@ export function useTag( let {descriptionProps: _, ...stateWithoutDescription} = states; let isDisabled = state.disabledKeys.has(item.key) || item.props.isDisabled; - let {keyboardProps} = useKeyboard({ - isDisabled, - shortcuts: { - Delete: () => { - if (state.selectionManager.isSelected(item.key)) { - onRemove?.(new Set(state.selectionManager.selectedKeys)); - } else { - onRemove?.(new Set([item.key])); - } - }, - Backspace: () => { - if (state.selectionManager.isSelected(item.key)) { - onRemove?.(new Set(state.selectionManager.selectedKeys)); - } else { - onRemove?.(new Set([item.key])); - } + let onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Delete' || e.key === 'Backspace') { + if (isDisabled) { + return; + } + + e.preventDefault(); + if (state.selectionManager.isSelected(item.key)) { + onRemove?.(new Set(state.selectionManager.selectedKeys)); + } else { + onRemove?.(new Set([item.key])); } } - }); + }; let modality = useInteractionModality(); if (modality === 'virtual' && typeof window !== 'undefined' && 'ontouchstart' in window) { @@ -129,7 +124,7 @@ export function useTag( }, rowProps: mergeProps(focusableProps, rowProps, domProps, linkProps, { tabIndex, - ...(onRemove ? keyboardProps : {}), + onKeyDown: onRemove ? onKeyDown : undefined, 'aria-describedby': descProps['aria-describedby'] }), gridCellProps: mergeProps(gridCellProps, { diff --git a/packages/react-aria/test/autocomplete/useSearchAutocomplete.test.js b/packages/react-aria/test/autocomplete/useSearchAutocomplete.test.js index 23ac503b8a3..d732f5266e1 100644 --- a/packages/react-aria/test/autocomplete/useSearchAutocomplete.test.js +++ b/packages/react-aria/test/autocomplete/useSearchAutocomplete.test.js @@ -25,9 +25,7 @@ describe('useSearchAutocomplete', function () { isComposing: false }, preventDefault, - stopPropagation, - target: {}, - currentTarget: {contains: () => true} + stopPropagation }); let defaultProps = { diff --git a/packages/react-aria/test/combobox/useComboBox.test.js b/packages/react-aria/test/combobox/useComboBox.test.js index 62d2511adb4..389c7ec3e64 100644 --- a/packages/react-aria/test/combobox/useComboBox.test.js +++ b/packages/react-aria/test/combobox/useComboBox.test.js @@ -117,9 +117,7 @@ describe('useComboBox', function () { let {inputProps} = result.current; act(() => { - inputProps.onKeyDown( - event({key: 'Enter', target: {}, currentTarget: {contains: () => true}}) - ); + inputProps.onKeyDown(event({key: 'Enter'})); }); expect(preventDefault).toHaveBeenCalledTimes(1); @@ -151,9 +149,7 @@ describe('useComboBox', function () { initialProps: props }); act(() => { - openResult.current.inputProps.onKeyDown( - event({key: 'Tab', target: {}, currentTarget: {contains: () => true}}) - ); + openResult.current.inputProps.onKeyDown(event({key: 'Tab'})); }); expect(commitSpy).toHaveBeenCalledTimes(1); }); @@ -165,15 +161,11 @@ describe('useComboBox', function () { let {result} = renderHook(props => useComboBox(props, state.current), {initialProps: props}); let {inputProps, buttonProps} = result.current; - inputProps.onKeyDown( - event({key: 'ArrowDown', target: {}, currentTarget: {contains: () => true}}) - ); + inputProps.onKeyDown(event({key: 'ArrowDown'})); expect(openSpy).toHaveBeenCalledTimes(1); expect(openSpy).toHaveBeenLastCalledWith('first', 'manual'); expect(toggleSpy).toHaveBeenCalledTimes(0); - inputProps.onKeyDown( - event({key: 'ArrowUp', target: {}, currentTarget: {contains: () => true}}) - ); + inputProps.onKeyDown(event({key: 'ArrowUp'})); expect(openSpy).toHaveBeenCalledTimes(2); expect(openSpy).toHaveBeenLastCalledWith('last', 'manual'); expect(toggleSpy).toHaveBeenCalledTimes(0); @@ -198,4 +190,36 @@ describe('useComboBox', function () { expect(onBlurMock).toHaveBeenCalledTimes(1); }); + + it.each` + Name | componentProps + ${'disabled'} | ${{isDisabled: true}} + ${'readonly'} | ${{isReadOnly: true}} + `( + "press and keyboard events on the button doesn't toggle the menu if $Name", + function ({componentProps}) { + let additionalProps = { + ...props, + ...componentProps + }; + + let {result: state} = renderHook(props => useComboBoxState(props), { + initialProps: additionalProps + }); + state.current.open = openSpy; + state.current.toggle = toggleSpy; + + let {result} = renderHook(props => useComboBox(props, state.current), { + initialProps: additionalProps + }); + let {buttonProps} = result.current; + buttonProps.onKeyDown(event({key: 'ArrowDown'})); + expect(openSpy).toHaveBeenCalledTimes(0); + expect(toggleSpy).toHaveBeenCalledTimes(0); + buttonProps.onKeyDown(event({key: 'ArrowUp'})); + expect(openSpy).toHaveBeenCalledTimes(0); + expect(toggleSpy).toHaveBeenCalledTimes(0); + expect(buttonProps.isDisabled).toBeTruthy(); + } + ); }); diff --git a/packages/react-aria/test/interactions/useKeyboard.test.js b/packages/react-aria/test/interactions/useKeyboard.test.js index 901b8bb74ab..60a0d405772 100644 --- a/packages/react-aria/test/interactions/useKeyboard.test.js +++ b/packages/react-aria/test/interactions/useKeyboard.test.js @@ -10,8 +10,6 @@ * governing permissions and limitations under the License. */ -/* eslint-disable jsx-a11y/no-static-element-interactions */ - import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; import React from 'react'; import {useKeyboard} from '../../src/interactions/useKeyboard'; @@ -99,280 +97,4 @@ describe('useKeyboard', function () { expect(onWrapperKeyDown).toHaveBeenCalledTimes(1); expect(onWrapperKeyUp).toHaveBeenCalledTimes(1); }); - - describe('shortcuts', () => { - let platformMock; - let user; - beforeEach(() => { - user = userEvent.setup({delay: null, pointerMap}); - }); - afterEach(() => { - platformMock?.mockRestore(); - }); - let ExampleButton = props => { - let {keyboardProps} = useKeyboard(props); - return ; - }; - describe('Mac (Mod = Meta)', () => { - beforeEach(() => { - platformMock = jest - .spyOn(navigator, 'platform', 'get') - .mockImplementation(() => 'MacIntel'); - }); - - it('matches Mod+key with metaKey', async () => { - let save = jest.fn(() => true); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - - render( -
- -
- ); - - await user.tab(); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(1); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Meta>}s{/Meta}'); - expect(save).toHaveBeenCalledTimes(1); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(1); // Meta keydown should be only one - expect(onWrapperKeyUp).toHaveBeenCalledTimes(2); // both s keyup and meta keyup - - save.mockClear(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - // None of the below should trigger the preventDefault and stopPropagation because - // we are not handling the event. - await user.keyboard('{Control>}s{/Control}'); - expect(save).not.toHaveBeenCalled(); - - await user.keyboard('s'); - expect(save).not.toHaveBeenCalled(); - - expect(onWrapperKeyDown).toHaveBeenCalledTimes(3); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(3); - }); - - it('plain key ignores meta', async () => { - let save = jest.fn(() => true); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - - render( -
- -
- ); - - await user.tab(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Meta>}s{/Meta}'); - expect(save).not.toHaveBeenCalled(); - - expect(onWrapperKeyDown).toHaveBeenCalledTimes(2); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(2); - }); - - it('Control+Shift distinct from Mod+Shift', async () => { - let modShift = jest.fn(); - let ctrlShift = jest.fn(); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - render( -
- -
- ); - await user.tab(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Meta>}{Shift>}a{/Shift}{/Meta}'); - expect(modShift).toHaveBeenCalledTimes(1); - expect(ctrlShift).not.toHaveBeenCalled(); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(2); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(3); - modShift.mockClear(); - ctrlShift.mockClear(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Control>}{Shift>}a{/Shift}{/Control}'); - expect(modShift).not.toHaveBeenCalled(); - expect(ctrlShift).toHaveBeenCalledTimes(1); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(2); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(3); - }); - - it('Meta+Control+Alt combination', async () => { - let fn = jest.fn(); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - render( -
- -
- ); - await user.tab(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Meta>}{Control>}{Alt>}z{/Alt}{/Control}{/Meta}'); - expect(fn).toHaveBeenCalledTimes(1); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(3); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(4); - }); - - it('Shift+Alt and key aliases', async () => { - let save = jest.fn(() => true); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - - render( -
- -
- ); - - await user.tab(); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(1); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Shift>}{Alt>}{ArrowDown}{/Alt}{/Shift}'); - expect(save).toHaveBeenCalledTimes(1); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(2); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(3); - }); - - it('Mod+Shift+a matches only that binding, not Mod+a', async () => { - let modA = jest.fn(() => true); - let modShiftA = jest.fn(() => true); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - - render( -
- -
- ); - - await user.tab(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Shift>}{Meta>}a{/Meta}{/Shift}'); - expect(modShiftA).toHaveBeenCalled(); - expect(modA).not.toHaveBeenCalled(); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(2); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(3); - modShiftA.mockClear(); - modA.mockClear(); - - await user.keyboard('{Meta>}a{/Meta}'); - expect(modA).toHaveBeenCalled(); - expect(modShiftA).not.toHaveBeenCalled(); - }); - - it('passes event to handler', async () => { - let fn = jest.fn(e => { - expect(e.key).toBe('Escape'); - }); - render(); - await user.tab(); - await user.keyboard('{Escape}'); - }); - - it('continues propagation if the function did not handle the event', async () => { - let fn = jest.fn(e => { - return false; - }); - let onWrapperKeyDown = jest.fn(e => { - expect(e.isDefaultPrevented()).toBe(false); - }); - let onWrapperKeyUp = jest.fn(); - render( -
- -
- ); - await user.tab(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - await user.keyboard('{Escape}'); - expect(fn).toHaveBeenCalledTimes(1); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(1); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(1); - }); - - it('prevent default and stop propagation can both be finely controlled', async () => { - let fn = jest.fn(e => { - return {shouldPreventDefault: false, shouldContinuePropagation: true}; - }); - let onWrapperKeyDown = jest.fn(e => { - expect(e.isDefaultPrevented()).toBe(false); - }); - let onWrapperKeyUp = jest.fn(); - render( -
- -
- ); - await user.tab(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - await user.keyboard('{Escape}'); - expect(fn).toHaveBeenCalledTimes(1); - }); - }); - - describe('Windows (Mod = Ctrl)', () => { - beforeEach(() => { - platformMock = jest.spyOn(navigator, 'platform', 'get').mockImplementation(() => 'Win32'); - }); - - it('matches Mod+key with ctrlKey', async () => { - let save = jest.fn(() => true); - let onWrapperKeyDown = jest.fn(); - let onWrapperKeyUp = jest.fn(); - - render( -
- -
- ); - - await user.tab(); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(1); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - await user.keyboard('{Control>}s{/Control}'); - expect(save).toHaveBeenCalledTimes(1); - expect(onWrapperKeyDown).toHaveBeenCalledTimes(1); // Meta keydown should be only one - expect(onWrapperKeyUp).toHaveBeenCalledTimes(2); // both s keyup and meta keyup - - save.mockClear(); - onWrapperKeyDown.mockClear(); - onWrapperKeyUp.mockClear(); - - // None of the below should trigger the preventDefault and stopPropagation because - // we are not handling the event. - await user.keyboard('{Meta>}s{/Meta}'); - expect(save).not.toHaveBeenCalled(); - - await user.keyboard('s'); - expect(save).not.toHaveBeenCalled(); - - expect(onWrapperKeyDown).toHaveBeenCalledTimes(3); - expect(onWrapperKeyUp).toHaveBeenCalledTimes(3); - }); - }); - }); }); diff --git a/packages/react-aria/test/searchfield/useSearchField.test.js b/packages/react-aria/test/searchfield/useSearchField.test.js index 3422c75175b..604711162f1 100644 --- a/packages/react-aria/test/searchfield/useSearchField.test.js +++ b/packages/react-aria/test/searchfield/useSearchField.test.js @@ -61,13 +61,8 @@ describe('useSearchField hook', () => { let onKeyDown = jest.fn(); let event = key => ({ key, - nativeEvent: { - isComposing: false - }, preventDefault, - stopPropagation, - target: {}, - currentTarget: {contains: () => true} + stopPropagation }); afterEach(() => { From 8a4e0b4de38fc3be5613016346d35cf4ee17099a Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 12 Jun 2026 15:43:17 -0700 Subject: [PATCH 12/12] propagate tab in combobox so it exits the gridlist --- packages/react-aria/src/combobox/useComboBox.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react-aria/src/combobox/useComboBox.ts b/packages/react-aria/src/combobox/useComboBox.ts index 7763da45591..8ea8a9e3121 100644 --- a/packages/react-aria/src/combobox/useComboBox.ts +++ b/packages/react-aria/src/combobox/useComboBox.ts @@ -215,6 +215,10 @@ export function useComboBox( if (e.key === 'Enter' || state.isOpen) { state.commit(); } + if (e.key === 'Tab') { + e.continuePropagation(); + } + break; case 'Escape': if (!state.selectionManager.isEmpty || state.inputValue === '' || props.allowsCustomValue) {