From 837b2aaff22dc8a88cc1fb8aafa566c799c26caa Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 13 Aug 2025 11:58:56 +1000 Subject: [PATCH 1/5] fix: event leaks --- packages/@react-aria/select/src/useSelect.ts | 13 +++-- .../selection/src/useSelectableCollection.ts | 20 +++++-- .../selection/src/useTypeSelect.ts | 57 ++++++++++++++----- .../@react-aria/textfield/src/useTextField.ts | 24 +++++++- .../combobox/test/ComboBox.test.js | 16 +++--- .../test/GridList.test.js | 44 +++++++++++++- 6 files changed, 137 insertions(+), 37 deletions(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 38aee11f510..532207dc936 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -19,7 +19,7 @@ import {FocusEvent, useMemo} from 'react'; import {HiddenSelectProps} from './HiddenSelect'; import {ListKeyboardDelegate, useTypeSelect} from '@react-aria/selection'; import {SelectState} from '@react-stately/select'; -import {setInteractionModality} from '@react-aria/interactions'; +import {setInteractionModality, useKeyboard} from '@react-aria/interactions'; import {useCollator} from '@react-aria/i18n'; import {useField} from '@react-aria/label'; import {useMenuTrigger} from '@react-aria/menu'; @@ -136,9 +136,6 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, errorMessage: props.errorMessage || validationErrors }); - typeSelectProps.onKeyDown = typeSelectProps.onKeyDownCapture; - delete typeSelectProps.onKeyDownCapture; - let domProps = filterDOMProps(props, {labelable: true}); let triggerProps = mergeProps(typeSelectProps, menuTriggerProps, fieldProps); @@ -152,6 +149,11 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, validationBehavior }); + let {keyboardProps} = useKeyboard({ + onKeyDown: chain(triggerProps.onKeyDown, onKeyDown, props.onKeyDown), + onKeyUp: props.onKeyUp + }); + return { labelProps: { ...labelProps, @@ -166,9 +168,8 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, }, triggerProps: mergeProps(domProps, { ...triggerProps, + ...keyboardProps, isDisabled, - onKeyDown: chain(triggerProps.onKeyDown, onKeyDown, props.onKeyDown), - onKeyUp: props.onKeyUp, 'aria-labelledby': [ valueId, triggerProps['aria-labelledby'], diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 825888ffea6..39e9906bb8d 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -12,10 +12,10 @@ import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils'; import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus'; -import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; +import {BaseEvent, DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; import {flushSync} from 'react-dom'; -import {FocusEvent, KeyboardEvent, useEffect, useRef} from 'react'; -import {focusSafely, getInteractionModality} from '@react-aria/interactions'; +import {FocusEvent, KeyboardEvent, useCallback, useEffect, useRef} from 'react'; +import {focusSafely, getInteractionModality, useKeyboard} from '@react-aria/interactions'; import {getItemElement, isNonContiguousSelectionModifier, useCollectionId} from './utils'; import {MultipleSelectionManager} from '@react-stately/selection'; import {useLocale} from '@react-aria/i18n'; @@ -126,7 +126,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions let {direction} = useLocale(); let router = useRouter(); - let onKeyDown = (e: KeyboardEvent) => { + let onKeyDown = (e: BaseEvent>) => { // 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(); @@ -135,6 +135,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions // Keyboard events bubble through portals. Don't handle keyboard events // for elements outside the collection (e.g. menus). if (!ref.current?.contains(e.target as Element)) { + // e.continuePropagation(); return; } @@ -169,6 +170,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions } }; + console.log('onKeyDown', e.key); switch (e.key) { case 'ArrowDown': { if (delegate.getKeyBelow) { @@ -319,6 +321,8 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions break; } } + default: + e.continuePropagation(); } }; @@ -560,7 +564,6 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions }); let handlers = { - onKeyDown, onFocus, onBlur, onMouseDown(e) { @@ -577,8 +580,13 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions selectionManager: manager }); + let {keyboardProps} = useKeyboard({ + onKeyDown, + onKeyUp: useCallback(() => {}, []) + }); + if (!disallowTypeAhead) { - handlers = mergeProps(typeSelectProps, handlers); + handlers = mergeProps(typeSelectProps, handlers, keyboardProps); } // If nothing is focused within the collection, make the collection itself tabbable. diff --git a/packages/@react-aria/selection/src/useTypeSelect.ts b/packages/@react-aria/selection/src/useTypeSelect.ts index 6a3e7dd7031..d49e0be0bac 100644 --- a/packages/@react-aria/selection/src/useTypeSelect.ts +++ b/packages/@react-aria/selection/src/useTypeSelect.ts @@ -11,7 +11,7 @@ */ import {DOMAttributes, Key, KeyboardDelegate} from '@react-types/shared'; -import {KeyboardEvent, useRef} from 'react'; +import {KeyboardEvent, useEffect, useRef} from 'react'; import {MultipleSelectionManager} from '@react-stately/selection'; /** @@ -51,21 +51,45 @@ export function useTypeSelect(options: AriaTypeSelectOptions): TypeSelectAria { timeout: undefined }).current; - let onKeyDown = (e: KeyboardEvent) => { - let character = getStringForKey(e.key); - if (!character || e.ctrlKey || e.metaKey || !e.currentTarget.contains(e.target as HTMLElement) || (state.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) { + 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.search.length > 0 && e.key === ' ') { e.preventDefault(); if (!('continuePropagation' in e)) { e.stopPropagation(); } + state.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.search, selectionManager.focusedKey); + + // If no key found, search from the top. + if (key == null) { + key = keyboardDelegate.getKeyForSearch(state.search); + } + + if (key != null) { + selectionManager.setFocusedKey(key); + if (onTypeSelect) { + onTypeSelect(key); + } + } + } + } + + clearTimeout(state.timeout); + state.timeout = setTimeout(() => { + state.search = ''; + }, TYPEAHEAD_DEBOUNCE_WAIT_MS); + }; + + let onKeyDown = (e: KeyboardEvent) => { + let character = getStringForKey(e.key); + if (!character || e.ctrlKey || e.metaKey || character === ' ' || !e.currentTarget.contains(e.target as HTMLElement)) { + return; } state.search += character; @@ -94,11 +118,18 @@ export function useTypeSelect(options: AriaTypeSelectOptions): TypeSelectAria { }, TYPEAHEAD_DEBOUNCE_WAIT_MS); }; + useEffect(() => { + return () => { + clearTimeout(state.timeout); + }; + }, []); + 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 + onKeyDownCapture: keyboardDelegate.getKeyForSearch ? onKeyDownCapture : undefined, + onKeyDown: keyboardDelegate.getKeyForSearch ? onKeyDown : undefined } }; } diff --git a/packages/@react-aria/textfield/src/useTextField.ts b/packages/@react-aria/textfield/src/useTextField.ts index e232514d052..3e5228145b3 100644 --- a/packages/@react-aria/textfield/src/useTextField.ts +++ b/packages/@react-aria/textfield/src/useTextField.ts @@ -11,14 +11,16 @@ */ import {AriaTextFieldProps} from '@react-types/textfield'; -import {DOMAttributes, ValidationResult} from '@react-types/shared'; +import {BaseEvent, DOMAttributes, ValidationResult} from '@react-types/shared'; import {filterDOMProps, getOwnerWindow, mergeProps, useFormReset} from '@react-aria/utils'; import React, { ChangeEvent, HTMLAttributes, type JSX, + KeyboardEvent, LabelHTMLAttributes, RefObject, + useCallback, useEffect, useState } from 'react'; @@ -107,6 +109,8 @@ export interface TextFieldAria(props.value, props.defaultValue || '', props.onChange); - let {focusableProps} = useFocusable(props, ref); + let onKeyDown = useCallback((e: BaseEvent>) => { + if (KEYS_TO_CONTINUE_PROPAGATION.has(e.key)) { + e.continuePropagation(); + }; + onKeyDownProp?.(e); + }, [onKeyDownProp]); + let onKeyUp = useCallback((e: BaseEvent>) => { + if (KEYS_TO_CONTINUE_PROPAGATION.has(e.key)) { + e.continuePropagation(); + }; + onKeyUpProp?.(e); + }, [onKeyUpProp]); + let {focusableProps} = useFocusable({...props, onKeyDown, onKeyUp}, ref); let validationState = useFormValidationState({ ...props, value diff --git a/packages/@react-spectrum/combobox/test/ComboBox.test.js b/packages/@react-spectrum/combobox/test/ComboBox.test.js index 8a6b7ee6e54..92f4d3d50f3 100644 --- a/packages/@react-spectrum/combobox/test/ComboBox.test.js +++ b/packages/@react-spectrum/combobox/test/ComboBox.test.js @@ -361,7 +361,7 @@ describe('ComboBox', function () { expect(onOpenChange).not.toHaveBeenCalled(); }); - it('features default behavior of completionMode suggest and menuTrigger input', async function () { + it.only('features default behavior of completionMode suggest and menuTrigger input', async function () { let tree = renderComboBox(); let comboboxTester = testUtilUser.createTester('ComboBox', {root: tree.container}); @@ -5268,9 +5268,9 @@ describe('ComboBox', function () { if (parseInt(React.version, 10) >= 19) { it('resets to defaultSelectedKey when submitting form action', async () => { - function Test() { + function Test() { const [value, formAction] = React.useActionState(() => '2', '1'); - + return (
@@ -5280,11 +5280,11 @@ describe('ComboBox', function () { ); } - + let {getByTestId, getByRole} = render(); let input = getByRole('combobox'); expect(input).toHaveValue('One'); - + let button = getByTestId('submit'); await act(async () => await user.click(button)); expect(input).toHaveValue('Two'); @@ -5599,7 +5599,7 @@ describe('ComboBox', function () { it('resets to defaultSelectedKey when submitting form action', async () => { function Test() { const [value, formAction] = React.useActionState(() => '2', '1'); - + return ( @@ -5609,11 +5609,11 @@ describe('ComboBox', function () { ); } - + let {getByTestId} = render(); let input = document.querySelector('input[name=combobox]'); expect(input).toHaveValue('One'); - + let button = getByTestId('submit'); await act(async () => await user.click(button)); expect(input).toHaveValue('Two'); diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 3dc1f0bf7f7..b12b0cbf18c 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -21,6 +21,7 @@ import { GridList, GridListContext, GridListItem, + Input, Label, ListLayout, Modal, @@ -28,6 +29,7 @@ import { Tag, TagGroup, TagList, + TextField, useDragAndDrop, Virtualizer } from '../'; @@ -1328,7 +1330,7 @@ describe('GridList', () => { let {getByRole} = renderGridList({}, {onAction, onPressStart, onPressEnd, onPress, onClick}); let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); await gridListTester.triggerRowAction({row: 1, interactionType}); - + expect(onAction).toHaveBeenCalledTimes(1); expect(onPressStart).toHaveBeenCalledTimes(1); expect(onPressEnd).toHaveBeenCalledTimes(1); @@ -1336,4 +1338,44 @@ describe('GridList', () => { expect(onClick).toHaveBeenCalledTimes(1); }); }); + + describe('editing', () => { + it('should render a gridlist with edit mode', async () => { + let tree = render( + + {(item) => ( + + + + + + + + + + + + + + + )} + + ); + let gridListTester = testUtilUser.createTester('GridList', {root: tree.getByRole('grid')}); + await user.tab(); + await user.tab(); + let row1Inputs = [...gridListTester.rows[0].querySelectorAll('input')]; + expect(row1Inputs[0]).toHaveFocus(); + await user.keyboard('Chris E'); + expect(row1Inputs[0]).toHaveFocus(); + expect(row1Inputs[0]).toHaveValue('Chris E'); + }); + }); }); From e0ca81352908c709dee0d21c0eaa8a108bb06fd4 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 13 Aug 2025 16:28:26 +1000 Subject: [PATCH 2/5] fix all tests associated with type select changes and fallout --- ...ary-user-event-npm-14.6.1-5da7e1d4e2.patch | 13 ++ .../interactions/src/createEventHandler.ts | 6 +- .../searchfield/src/useSearchField.ts | 2 + .../selection/src/useSelectableCollection.ts | 16 +- .../@react-aria/textfield/src/useTextField.ts | 2 +- .../combobox/test/ComboBox.test.js | 2 +- .../list/test/ListViewDnd.test.js | 1 - .../table/test/TableDnd.test.js | 3 - .../@react-spectrum/tabs/test/Tabs.test.js | 7 - .../test/GridList.test.js | 27 ++- yarn.lock | 162 +++++++++--------- 11 files changed, 133 insertions(+), 108 deletions(-) diff --git a/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch b/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch index a602426af6f..fc4c114ded1 100644 --- a/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch +++ b/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch @@ -11,6 +11,19 @@ index 39a24b8f2ccdc52739d130480ab18975073616cb..0c3f5199401c15b90230c25a02de364e } UI.clearInitialValue(el); } +diff --git a/dist/cjs/event/behavior/keydown.js b/dist/cjs/event/behavior/keydown.js +index 55027cb256f66b808d17280dc01bc55a796a1032..993d5de5a838a711d7ae009344354772a42ed0c1 100644 +--- a/dist/cjs/event/behavior/keydown.js ++++ b/dist/cjs/event/behavior/keydown.js +@@ -110,7 +110,7 @@ const keydownBehavior = { + }, + Tab: (event, target, instance)=>{ + return ()=>{ +- const dest = getTabDestination.getTabDestination(target, instance.system.keyboard.modifiers.Shift); ++ const dest = getTabDestination.getTabDestination(document.activeElement, instance.system.keyboard.modifiers.Shift); + focus.focusElement(dest); + if (selection.hasOwnSelection(dest)) { + UI.setUISelection(dest, { diff --git a/dist/cjs/utils/focus/getActiveElement.js b/dist/cjs/utils/focus/getActiveElement.js index d25f3a8ef67e856e43614559f73012899c0b53d7..4ed9ee45565ed438ee9284d8d3043c0bd50463eb 100644 --- a/dist/cjs/utils/focus/getActiveElement.js diff --git a/packages/@react-aria/interactions/src/createEventHandler.ts b/packages/@react-aria/interactions/src/createEventHandler.ts index fd9a9d27649..50a00a65c73 100644 --- a/packages/@react-aria/interactions/src/createEventHandler.ts +++ b/packages/@react-aria/interactions/src/createEventHandler.ts @@ -23,6 +23,10 @@ export function createEventHandler(handler?: (e: BaseE let shouldStopPropagation = true; return (e: T) => { + if ('continuePropagation' in e) { + handler(e as any); + return undefined; + } let event: BaseEvent = { ...e, preventDefault() { @@ -48,7 +52,7 @@ export function createEventHandler(handler?: (e: BaseE handler(event); - if (shouldStopPropagation) { + if (shouldStopPropagation && ('isPropagationStopped' in e && !e.isPropagationStopped())) { e.stopPropagation(); } }; diff --git a/packages/@react-aria/searchfield/src/useSearchField.ts b/packages/@react-aria/searchfield/src/useSearchField.ts index 15aae4a1a67..e2d8979a6ed 100644 --- a/packages/@react-aria/searchfield/src/useSearchField.ts +++ b/packages/@react-aria/searchfield/src/useSearchField.ts @@ -79,6 +79,8 @@ export function useSearchField( e.continuePropagation(); } else { e.preventDefault(); + // by default textfield will continue this one because it doesn't do anything there, so we have to explicitly stop it here + e.stopPropagation(); state.setValue(''); if (onClear) { onClear(); diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 39e9906bb8d..a19ab3159c5 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -135,7 +135,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions // Keyboard events bubble through portals. Don't handle keyboard events // for elements outside the collection (e.g. menus). if (!ref.current?.contains(e.target as Element)) { - // e.continuePropagation(); + e.continuePropagation(); return; } @@ -170,7 +170,6 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions } }; - console.log('onKeyDown', e.key); switch (e.key) { case 'ArrowDown': { if (delegate.getKeyBelow) { @@ -288,9 +287,10 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions break; case 'Escape': if (escapeKeyBehavior === 'clearSelection' && !disallowEmptySelection && manager.selectedKeys.size !== 0) { - e.stopPropagation(); e.preventDefault(); manager.clearSelection(); + } else { + e.continuePropagation(); } break; case 'Tab': { @@ -316,9 +316,13 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions if (next && !next.contains(document.activeElement)) { focusWithoutScrolling(next); + } else { + e.continuePropagation(); } } break; + } else { + e.continuePropagation(); } } default: @@ -581,12 +585,12 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions }); let {keyboardProps} = useKeyboard({ - onKeyDown, - onKeyUp: useCallback(() => {}, []) + onKeyDown }); + handlers = mergeProps(handlers, keyboardProps); if (!disallowTypeAhead) { - handlers = mergeProps(typeSelectProps, handlers, keyboardProps); + handlers = mergeProps(typeSelectProps, handlers); } // If nothing is focused within the collection, make the collection itself tabbable. diff --git a/packages/@react-aria/textfield/src/useTextField.ts b/packages/@react-aria/textfield/src/useTextField.ts index 3e5228145b3..41341a64ffe 100644 --- a/packages/@react-aria/textfield/src/useTextField.ts +++ b/packages/@react-aria/textfield/src/useTextField.ts @@ -109,7 +109,7 @@ export interface TextFieldAria); await user.tab({shift: true}); - await user.tab({shift: true}); await beginDrag(tree); await user.tab(); // Should automatically jump to the folder target since we didn't provide onRootDrop and onInsert diff --git a/packages/@react-spectrum/table/test/TableDnd.test.js b/packages/@react-spectrum/table/test/TableDnd.test.js index a97231f92a7..295e8883347 100644 --- a/packages/@react-spectrum/table/test/TableDnd.test.js +++ b/packages/@react-spectrum/table/test/TableDnd.test.js @@ -2145,7 +2145,6 @@ describe('TableView', function () { }); act(() => jest.runAllTimers()); await user.tab({shift: true}); - await user.tab({shift: true}); await user.keyboard('{ArrowLeft}'); // Drop on folder in same table @@ -2301,8 +2300,6 @@ describe('TableView', function () { await user.keyboard('{Escape}'); tree.rerender(); - await user.tab({shift: true}); - await user.tab({shift: true}); await user.keyboard('{ArrowLeft}'); await user.keyboard('{ArrowRight}'); diff --git a/packages/@react-spectrum/tabs/test/Tabs.test.js b/packages/@react-spectrum/tabs/test/Tabs.test.js index 802c7701e79..40329666703 100644 --- a/packages/@react-spectrum/tabs/test/Tabs.test.js +++ b/packages/@react-spectrum/tabs/test/Tabs.test.js @@ -142,30 +142,23 @@ describe('Tabs', function () { }); it('allows user to change tab item select via arrow keys with horizontal tabs (rtl)', async function () { - let onKeyDown = jest.fn(); let container = renderComponent({orientation: 'horizontal', providerProps: {locale: 'ar-AE'}}); let tabsTester = testUtilUser.createTester('Tabs', {root: container.getByRole('tablist'), interactionType: 'keyboard', direction: 'rtl'}); let tabs = tabsTester.tabs; - window.addEventListener('keydown', onKeyDown); expect(tabs[0]).toHaveAttribute('aria-selected', 'true'); await tabsTester.triggerTab({tab: 1}); expect(tabs[0]).not.toHaveAttribute('aria-selected', 'true'); expect(tabs[1]).toHaveAttribute('aria-selected', 'true'); - // Just to double check that the util is actually pressing the expected arrow key - expect(onKeyDown.mock.calls[0][0].key).toBe('ArrowLeft'); await tabsTester.triggerTab({tab: 2}); expect(tabs[1]).not.toHaveAttribute('aria-selected', 'true'); expect(tabs[2]).toHaveAttribute('aria-selected', 'true'); - expect(onKeyDown.mock.calls[1][0].key).toBe('ArrowLeft'); await tabsTester.triggerTab({tab: 1}); expect(tabs[2]).not.toHaveAttribute('aria-selected', 'true'); expect(tabs[1]).toHaveAttribute('aria-selected', 'true'); - expect(onKeyDown.mock.calls[2][0].key).toBe('ArrowRight'); - window.removeEventListener('keydown', onKeyDown); }); it('allows user to change tab item select via arrow keys with vertical tabs', function () { diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index b12b0cbf18c..5ce8d4cd101 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -703,10 +703,7 @@ describe('GridList', () => { expect(document.activeElement).toBe(items[1]); await user.tab(); - expect(document.activeElement).toBe(buttonRef.current); - - await user.tab(); - expect(document.activeElement).toBe(document.body); + expect(document.body).toHaveFocus(); }); it('should support rendering a TagGroup with tabbing navigation inside a GridListItem', async () => { @@ -1341,10 +1338,12 @@ describe('GridList', () => { describe('editing', () => { it('should render a gridlist with edit mode', async () => { + let onSelectionChange = jest.fn(); let tree = render( { {(item) => ( - + - + - + @@ -1376,6 +1375,20 @@ describe('GridList', () => { await user.keyboard('Chris E'); expect(row1Inputs[0]).toHaveFocus(); expect(row1Inputs[0]).toHaveValue('Chris E'); + + await user.tab(); + expect(row1Inputs[1]).toHaveFocus(); + await user.keyboard('{Enter}'); + expect(onSelectionChange).not.toHaveBeenCalled(); + + await user.tab(); + expect(row1Inputs[2]).toHaveFocus(); + + await user.keyboard('{ArrowLeft}'); + expect(row1Inputs[2]).toHaveFocus(); + + await user.keyboard('{ArrowRight}'); + expect(row1Inputs[2]).toHaveFocus(); }); }); }); diff --git a/yarn.lock b/yarn.lock index f812e28692e..e3f0bd2e825 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10220,10 +10220,10 @@ __metadata: "@testing-library/user-event@patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch": version: 14.6.1 - resolution: "@testing-library/user-event@patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch::version=14.6.1&hash=13cf21" + resolution: "@testing-library/user-event@patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch::version=14.6.1&hash=3511b9" peerDependencies: "@testing-library/dom": ">=7.21.4" - checksum: 10c0/ede32fec9345bb5e5c19a5abcb647d8c4704239f3f5417afe2914c1397067dae7ce547e46adfd4027c913f5735c0651ec530c73bdc5c7ea955efa860cc6a9dd9 + checksum: 10c0/5a3e378cfdcad1ae09b73141ba9ea5adb0e7ed0d9f6bf1c4ba3631c91554414c4f2ab255c23f08425d2d398daa11d745ead8ef7ba0d1de76e19252db0b5dbba3 languageName: node linkType: hard @@ -10718,63 +10718,63 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/eslint-plugin@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/eslint-plugin@npm:8.38.0" +"@typescript-eslint/eslint-plugin@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/eslint-plugin@npm:8.39.1" dependencies: "@eslint-community/regexpp": "npm:^4.10.0" - "@typescript-eslint/scope-manager": "npm:8.38.0" - "@typescript-eslint/type-utils": "npm:8.38.0" - "@typescript-eslint/utils": "npm:8.38.0" - "@typescript-eslint/visitor-keys": "npm:8.38.0" + "@typescript-eslint/scope-manager": "npm:8.39.1" + "@typescript-eslint/type-utils": "npm:8.39.1" + "@typescript-eslint/utils": "npm:8.39.1" + "@typescript-eslint/visitor-keys": "npm:8.39.1" graphemer: "npm:^1.4.0" ignore: "npm:^7.0.0" natural-compare: "npm:^1.4.0" ts-api-utils: "npm:^2.1.0" peerDependencies: - "@typescript-eslint/parser": ^8.38.0 + "@typescript-eslint/parser": ^8.39.1 eslint: ^8.57.0 || ^9.0.0 - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/199b82e9f0136baecf515df7c31bfed926a7c6d4e6298f64ee1a77c8bdd7a8cb92a2ea55a5a345c9f2948a02f7be6d72530efbe803afa1892b593fbd529d0c27 + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/7a55de558ed6ea6f09ee0b0d994b4a70e1df9f72e4afc7b3073de1b41504a36d905779304d59c34db700af60da3bb438c62480d30462a13b8b72d0b50318aeee languageName: node linkType: hard -"@typescript-eslint/parser@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/parser@npm:8.38.0" +"@typescript-eslint/parser@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/parser@npm:8.39.1" dependencies: - "@typescript-eslint/scope-manager": "npm:8.38.0" - "@typescript-eslint/types": "npm:8.38.0" - "@typescript-eslint/typescript-estree": "npm:8.38.0" - "@typescript-eslint/visitor-keys": "npm:8.38.0" + "@typescript-eslint/scope-manager": "npm:8.39.1" + "@typescript-eslint/types": "npm:8.39.1" + "@typescript-eslint/typescript-estree": "npm:8.39.1" + "@typescript-eslint/visitor-keys": "npm:8.39.1" debug: "npm:^4.3.4" peerDependencies: eslint: ^8.57.0 || ^9.0.0 - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/5580c2a328f0c15f85e4a0961a07584013cc0aca85fe868486187f7c92e9e3f6602c6e3dab917b092b94cd492ed40827c6f5fea42730bef88eb17592c947adf4 + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/da30372c4e8dee48a0c421996bf0bf73a62a57039ee6b817eda64de2d70fdb88dd20b50615c81be7e68fd29cdd7852829b859bb8539b4a4c78030f93acaf5664 languageName: node linkType: hard -"@typescript-eslint/project-service@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/project-service@npm:8.38.0" +"@typescript-eslint/project-service@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/project-service@npm:8.39.1" dependencies: - "@typescript-eslint/tsconfig-utils": "npm:^8.38.0" - "@typescript-eslint/types": "npm:^8.38.0" + "@typescript-eslint/tsconfig-utils": "npm:^8.39.1" + "@typescript-eslint/types": "npm:^8.39.1" debug: "npm:^4.3.4" peerDependencies: - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/87d2f55521e289bbcdc666b1f4587ee2d43039cee927310b05abaa534b528dfb1b5565c1545bb4996d7fbdf9d5a3b0aa0e6c93a8f1289e3fcfd60d246364a884 + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/40207af4f4e2a260ea276766d502c4736f6dc5488e84bbab6444e2786289ece2dbca2686323c48d4e9c265e409a309bf3d97d4aa03767dff8cc7642b436bda35 languageName: node linkType: hard -"@typescript-eslint/scope-manager@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/scope-manager@npm:8.38.0" +"@typescript-eslint/scope-manager@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/scope-manager@npm:8.39.1" dependencies: - "@typescript-eslint/types": "npm:8.38.0" - "@typescript-eslint/visitor-keys": "npm:8.38.0" - checksum: 10c0/ceaf489ea1f005afb187932a7ee363dfe1e0f7cc3db921283991e20e4c756411a5e25afbec72edd2095d6a4384f73591f4c750cf65b5eaa650c90f64ef9fe809 + "@typescript-eslint/types": "npm:8.39.1" + "@typescript-eslint/visitor-keys": "npm:8.39.1" + checksum: 10c0/9466db557c1a0eaaf24b0ece5810413d11390d046bf6e47c4074879e8dba0348b835a21106c842ab20ff85f2384312cf9e20bfe7684e31640696e29957003511 languageName: node linkType: hard @@ -10788,35 +10788,35 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/tsconfig-utils@npm:8.38.0, @typescript-eslint/tsconfig-utils@npm:^8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/tsconfig-utils@npm:8.38.0" +"@typescript-eslint/tsconfig-utils@npm:8.39.1, @typescript-eslint/tsconfig-utils@npm:^8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/tsconfig-utils@npm:8.39.1" peerDependencies: - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/1a90da16bf1f7cfbd0303640a8ead64a0080f2b1d5969994bdac3b80abfa1177f0c6fbf61250bae082e72cf5014308f2f5cc98edd6510202f13420a7ffd07a84 + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/664dff0b4ae908cb98c78f9ca73c36cf57c3a2206965d9d0659649ffc02347eb30e1452499671a425592f14a2a5c5eb82ae389b34f3c415a12119506b4ebb61c languageName: node linkType: hard -"@typescript-eslint/type-utils@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/type-utils@npm:8.38.0" +"@typescript-eslint/type-utils@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/type-utils@npm:8.39.1" dependencies: - "@typescript-eslint/types": "npm:8.38.0" - "@typescript-eslint/typescript-estree": "npm:8.38.0" - "@typescript-eslint/utils": "npm:8.38.0" + "@typescript-eslint/types": "npm:8.39.1" + "@typescript-eslint/typescript-estree": "npm:8.39.1" + "@typescript-eslint/utils": "npm:8.39.1" debug: "npm:^4.3.4" ts-api-utils: "npm:^2.1.0" peerDependencies: eslint: ^8.57.0 || ^9.0.0 - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/27795c4bd0be395dda3424e57d746639c579b7522af1c17731b915298a6378fd78869e8e141526064b6047db2c86ba06444469ace19c98cda5779d06f4abd37c + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/430dfefe040eae5f0c8dfbce37b5ce071095a28f335e74793923d113682e26313586e90f7bbe2c2f9bffb0da52ffdf5055ea36b96d9f218cef35aa14853122d5 languageName: node linkType: hard -"@typescript-eslint/types@npm:8.38.0, @typescript-eslint/types@npm:^8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/types@npm:8.38.0" - checksum: 10c0/f0ac0060c98c0f3d1871f107177b6ae25a0f1846ca8bd8cfc7e1f1dd0ddce293cd8ac4a5764d6a767de3503d5d01defcd68c758cb7ba6de52f82b209a918d0d2 +"@typescript-eslint/types@npm:8.39.1, @typescript-eslint/types@npm:^8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/types@npm:8.39.1" + checksum: 10c0/0e188d2d52509a24c500a87adf561387ffcac56b62cb9fd0ca1f929bb3d4eedb6b8f9d516c1890855d39930c9dd8d502d5b4600b8c9cc832d3ebb595d81c7533 languageName: node linkType: hard @@ -10827,14 +10827,14 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/typescript-estree@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/typescript-estree@npm:8.38.0" +"@typescript-eslint/typescript-estree@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/typescript-estree@npm:8.39.1" dependencies: - "@typescript-eslint/project-service": "npm:8.38.0" - "@typescript-eslint/tsconfig-utils": "npm:8.38.0" - "@typescript-eslint/types": "npm:8.38.0" - "@typescript-eslint/visitor-keys": "npm:8.38.0" + "@typescript-eslint/project-service": "npm:8.39.1" + "@typescript-eslint/tsconfig-utils": "npm:8.39.1" + "@typescript-eslint/types": "npm:8.39.1" + "@typescript-eslint/visitor-keys": "npm:8.39.1" debug: "npm:^4.3.4" fast-glob: "npm:^3.3.2" is-glob: "npm:^4.0.3" @@ -10842,8 +10842,8 @@ __metadata: semver: "npm:^7.6.0" ts-api-utils: "npm:^2.1.0" peerDependencies: - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/00a00f6549877f4ae5c2847fa5ac52bf42cbd59a87533856c359e2746e448ed150b27a6137c92fd50c06e6a4b39e386d6b738fac97d80d05596e81ce55933230 + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/1de1a37fed354600a08bc971492c2f14238f0a4bf07a43bedb416c17b7312d18bec92c68c8f2790bb0a1bffcd757f7962914be9f6213068f18f6c4fdde259af4 languageName: node linkType: hard @@ -10866,18 +10866,18 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/utils@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/utils@npm:8.38.0" +"@typescript-eslint/utils@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/utils@npm:8.39.1" dependencies: "@eslint-community/eslint-utils": "npm:^4.7.0" - "@typescript-eslint/scope-manager": "npm:8.38.0" - "@typescript-eslint/types": "npm:8.38.0" - "@typescript-eslint/typescript-estree": "npm:8.38.0" + "@typescript-eslint/scope-manager": "npm:8.39.1" + "@typescript-eslint/types": "npm:8.39.1" + "@typescript-eslint/typescript-estree": "npm:8.39.1" peerDependencies: eslint: ^8.57.0 || ^9.0.0 - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/e97a45bf44f315f9ed8c2988429e18c88e3369c9ee3227ee86446d2d49f7325abebbbc9ce801e178f676baa986d3e1fd4b5391f1640c6eb8944c123423ae43bb + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/ebc01d736af43728df9a0915058d0c771dec9cc58846ffdcbb986c78e7dabf547ea7daecd75db58b2af88a3c2a43de8a7e5f81feefacfa31be173fc384d25d77 languageName: node linkType: hard @@ -10895,13 +10895,13 @@ __metadata: languageName: node linkType: hard -"@typescript-eslint/visitor-keys@npm:8.38.0": - version: 8.38.0 - resolution: "@typescript-eslint/visitor-keys@npm:8.38.0" +"@typescript-eslint/visitor-keys@npm:8.39.1": + version: 8.39.1 + resolution: "@typescript-eslint/visitor-keys@npm:8.39.1" dependencies: - "@typescript-eslint/types": "npm:8.38.0" + "@typescript-eslint/types": "npm:8.39.1" eslint-visitor-keys: "npm:^4.2.1" - checksum: 10c0/071a756e383f41a6c9e51d78c8c64bd41cd5af68b0faef5fbaec4fa5dbd65ec9e4cd610c2e2cdbe9e2facc362995f202850622b78e821609a277b5b601a1d4ec + checksum: 10c0/4d81f6826a211bc2752e25cd16d1f415f28ebc92b35142402ec23f3765f2d00963b75ac06266ad9c674ca5b057d07d8c114116e5bf14f5465dde1d1aa60bc72f languageName: node linkType: hard @@ -29954,17 +29954,17 @@ __metadata: linkType: hard "typescript-eslint@npm:^8.38.0": - version: 8.38.0 - resolution: "typescript-eslint@npm:8.38.0" + version: 8.39.1 + resolution: "typescript-eslint@npm:8.39.1" dependencies: - "@typescript-eslint/eslint-plugin": "npm:8.38.0" - "@typescript-eslint/parser": "npm:8.38.0" - "@typescript-eslint/typescript-estree": "npm:8.38.0" - "@typescript-eslint/utils": "npm:8.38.0" + "@typescript-eslint/eslint-plugin": "npm:8.39.1" + "@typescript-eslint/parser": "npm:8.39.1" + "@typescript-eslint/typescript-estree": "npm:8.39.1" + "@typescript-eslint/utils": "npm:8.39.1" peerDependencies: eslint: ^8.57.0 || ^9.0.0 - typescript: ">=4.8.4 <5.9.0" - checksum: 10c0/486b9862ee08f7827d808a2264ce03b58087b11c4c646c0da3533c192a67ae3fcb4e68d7a1e69d0f35a1edc274371a903a50ecfe74012d5eaa896cb9d5a81e0b + typescript: ">=4.8.4 <6.0.0" + checksum: 10c0/4070729621c20f8a9bad3df13fb8ac175609a57d046c155df785d474c2926d3e506f0bd5e762be7e2aacd03839c9c9a2015ad087086cee5838c486b9bf46b27b languageName: node linkType: hard From 4024ad56be68702961a11a4010d0d8d5c278de94 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 26 Aug 2025 16:44:32 +1000 Subject: [PATCH 3/5] fix all tests so far --- .../test/useSearchAutocomplete.test.js | 5 +- .../combobox/test/useComboBox.test.js | 5 +- packages/@react-aria/grid/src/useGridCell.ts | 24 +- .../@react-aria/grid/test/useGrid.test.js | 4 +- .../@react-aria/menu/src/useMenuTrigger.ts | 6 + .../@react-aria/textfield/src/useTextField.ts | 22 +- .../@react-spectrum/card/src/CardView.tsx | 2 +- .../s2/stories/TableView.stories.tsx | 178 +++++++++++- .../@react-spectrum/table/test/TableTests.js | 36 +-- .../react-aria-components/test/Table.test.js | 253 +++++++++++++++++- 10 files changed, 496 insertions(+), 39 deletions(-) diff --git a/packages/@react-aria/autocomplete/test/useSearchAutocomplete.test.js b/packages/@react-aria/autocomplete/test/useSearchAutocomplete.test.js index 1ff0745bb64..8742b21d022 100644 --- a/packages/@react-aria/autocomplete/test/useSearchAutocomplete.test.js +++ b/packages/@react-aria/autocomplete/test/useSearchAutocomplete.test.js @@ -75,11 +75,12 @@ describe('useSearchAutocomplete', function () { let {result} = renderHook((props) => useSearchAutocomplete(props, state.current), {initialProps: props}); let {inputProps} = result.current; - inputProps.onKeyDown(event({key: 'ArrowDown'})); + let input = document.createElement('input'); + inputProps.onKeyDown(event({key: 'ArrowDown', target: input})); expect(openSpy).toHaveBeenCalledTimes(1); expect(openSpy).toHaveBeenLastCalledWith('first', 'manual'); expect(toggleSpy).toHaveBeenCalledTimes(0); - inputProps.onKeyDown(event({key: 'ArrowUp'})); + inputProps.onKeyDown(event({key: 'ArrowUp', target: input})); expect(openSpy).toHaveBeenCalledTimes(2); expect(openSpy).toHaveBeenLastCalledWith('last', 'manual'); expect(toggleSpy).toHaveBeenCalledTimes(0); diff --git a/packages/@react-aria/combobox/test/useComboBox.test.js b/packages/@react-aria/combobox/test/useComboBox.test.js index 9b83283f355..882c191ebdd 100644 --- a/packages/@react-aria/combobox/test/useComboBox.test.js +++ b/packages/@react-aria/combobox/test/useComboBox.test.js @@ -110,11 +110,12 @@ describe('useComboBox', function () { let {result} = renderHook((props) => useComboBox(props, state.current), {initialProps: props}); let {inputProps, buttonProps} = result.current; - inputProps.onKeyDown(event({key: 'ArrowDown'})); + let input = document.createElement('input'); + inputProps.onKeyDown(event({key: 'ArrowDown', target: input})); expect(openSpy).toHaveBeenCalledTimes(1); expect(openSpy).toHaveBeenLastCalledWith('first', 'manual'); expect(toggleSpy).toHaveBeenCalledTimes(0); - inputProps.onKeyDown(event({key: 'ArrowUp'})); + inputProps.onKeyDown(event({key: 'ArrowUp', target: input})); expect(openSpy).toHaveBeenCalledTimes(2); expect(openSpy).toHaveBeenLastCalledWith('last', 'manual'); expect(toggleSpy).toHaveBeenCalledTimes(0); diff --git a/packages/@react-aria/grid/src/useGridCell.ts b/packages/@react-aria/grid/src/useGridCell.ts index b8e09c29cdd..8a4e3df0a43 100644 --- a/packages/@react-aria/grid/src/useGridCell.ts +++ b/packages/@react-aria/grid/src/useGridCell.ts @@ -108,11 +108,31 @@ export function useGridCell>(props: GridCellProps isDisabled: state.collection.size === 0 }); - let onKeyDownCapture = (e: ReactKeyboardEvent) => { + let onKeyDown = (e: ReactKeyboardEvent) => { if (!e.currentTarget.contains(e.target as Element) || state.isKeyboardNavigationDisabled || !ref.current || !document.activeElement) { return; } + if (e.target !== ref.current && e.key !== 'ArrowLeft' && e.key !== 'ArrowRight' && e.key !== 'ArrowUp' && e.key !== 'ArrowDown') { + if (e.key === 'Tab' && ref.current.contains(e.target as Element)) { + let cellWalker = getFocusableTreeWalker(ref.current, {tabbable: true}); + if (e.shiftKey) { + cellWalker.currentNode = ref.current; + let isFirstFocusable = cellWalker.firstChild() === e.target; + if (!isFirstFocusable) { + e.stopPropagation(); + } + } else { + cellWalker.currentNode = ref.current; + let isLastFocusable = cellWalker.lastChild() === e.target; + if (!isLastFocusable) { + e.stopPropagation(); + } + } + } + return; + } + let walker = getFocusableTreeWalker(ref.current); walker.currentNode = document.activeElement; @@ -252,7 +272,7 @@ export function useGridCell>(props: GridCellProps let gridCellProps: DOMAttributes = mergeProps(itemProps, { role: 'gridcell', - onKeyDownCapture, + onKeyDown, 'aria-colspan': node.colSpan, 'aria-colindex': node.colIndex != null ? node.colIndex + 1 : undefined, // aria-colindex is 1-based colSpan: isVirtualized ? undefined : node.colSpan, diff --git a/packages/@react-aria/grid/test/useGrid.test.js b/packages/@react-aria/grid/test/useGrid.test.js index 8804655b940..bb479975c9f 100644 --- a/packages/@react-aria/grid/test/useGrid.test.js +++ b/packages/@react-aria/grid/test/useGrid.test.js @@ -60,7 +60,7 @@ describe('useGrid', () => { await user.keyboard('[ArrowRight]'); expect(document.activeElement).toBe(tree.getAllByRole('switch')[0]); - await user.keyboard('[ArrowRight]'); + await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('switch')[1]); await user.keyboard('[ArrowRight]'); @@ -71,7 +71,7 @@ describe('useGrid', () => { act(() => tree.getAllByRole('switch')[1].focus()); - await user.keyboard('[ArrowLeft]'); + await user.tab({shift: true}); expect(document.activeElement).toBe(tree.getAllByRole('switch')[0]); await user.keyboard('[ArrowLeft]'); diff --git a/packages/@react-aria/menu/src/useMenuTrigger.ts b/packages/@react-aria/menu/src/useMenuTrigger.ts index 2d2227276a3..eb7756abd16 100644 --- a/packages/@react-aria/menu/src/useMenuTrigger.ts +++ b/packages/@react-aria/menu/src/useMenuTrigger.ts @@ -77,6 +77,9 @@ export function useMenuTrigger(props: AriaMenuTriggerProps, state: MenuTrigge } // fallthrough case 'ArrowDown': + if (e.key === 'ArrowDown' && !e.altKey && e.target.closest('[role="gridcell"]')) { + return; + } // Stop propagation, unless it would already be handled by useKeyboard. if (!('continuePropagation' in e)) { e.stopPropagation(); @@ -85,6 +88,9 @@ export function useMenuTrigger(props: AriaMenuTriggerProps, state: MenuTrigge state.toggle('first'); break; case 'ArrowUp': + if (e.key === 'ArrowUp' && !e.altKey && e.target.closest('[role="gridcell"]')) { + return; + } if (!('continuePropagation' in e)) { e.stopPropagation(); } diff --git a/packages/@react-aria/textfield/src/useTextField.ts b/packages/@react-aria/textfield/src/useTextField.ts index 41341a64ffe..6b7f9601da2 100644 --- a/packages/@react-aria/textfield/src/useTextField.ts +++ b/packages/@react-aria/textfield/src/useTextField.ts @@ -131,13 +131,33 @@ export function useTextField(props.value, props.defaultValue || '', props.onChange); - let onKeyDown = useCallback((e: BaseEvent>) => { + let onKeyDown = useCallback((e: BaseEvent>) => { + if ((e.key === 'ArrowLeft' || e.key === 'ArrowUp' || e.key === 'Home') + && (e.target as HTMLInputElement).selectionStart === 0 + && (e.target as HTMLInputElement).selectionEnd === 0) { + e.continuePropagation(); + } + if ((e.key === 'ArrowRight' || e.key === 'ArrowDown' || e.key === 'End') + && (e.target as HTMLInputElement).selectionStart === (e.target as HTMLInputElement).value.length + && (e.target as HTMLInputElement).selectionEnd === (e.target as HTMLInputElement).value.length) { + e.continuePropagation(); + } if (KEYS_TO_CONTINUE_PROPAGATION.has(e.key)) { e.continuePropagation(); }; onKeyDownProp?.(e); }, [onKeyDownProp]); let onKeyUp = useCallback((e: BaseEvent>) => { + if ((e.key === 'ArrowLeft' || e.key === 'ArrowUp') + && (e.target as HTMLInputElement).selectionStart === 0 + && (e.target as HTMLInputElement).selectionEnd === 0) { + e.continuePropagation(); + } + if ((e.key === 'ArrowRight' || e.key === 'ArrowDown') + && (e.target as HTMLInputElement).selectionStart === (e.target as HTMLInputElement).value.length + && (e.target as HTMLInputElement).selectionEnd === (e.target as HTMLInputElement).value.length) { + e.continuePropagation(); + } if (KEYS_TO_CONTINUE_PROPAGATION.has(e.key)) { e.continuePropagation(); }; diff --git a/packages/@react-spectrum/card/src/CardView.tsx b/packages/@react-spectrum/card/src/CardView.tsx index e060a2e84a4..24789daedd6 100644 --- a/packages/@react-spectrum/card/src/CardView.tsx +++ b/packages/@react-spectrum/card/src/CardView.tsx @@ -233,7 +233,7 @@ function InternalCard(props) { // We don't want to focus the checkbox (or any other focusable elements) within the Card // when pressing the arrow keys so we delete the key down handler here. Arrow key navigation between // the cards in the CardView is handled by useGrid => useSelectableCollection instead. - delete gridCellProps.onKeyDownCapture; + delete gridCellProps.onKeyDown; return (
= { } }; +let data: {id: string, name: string, description: string, type: string}[] = [ + {id: '1', name: 'Name', description: 'Who you are', type: 'text'}, + {id: '2', name: 'Date of birth', description: 'For horoscopes', type: 'date'}, + {id: '3', name: 'Favourite colour', description: 'For your personality', type: 'combobox'}, + {id: '4', name: 'Pets', description: 'For your enjoyment', type: 'picker'}, + {id: '5', name: 'Allowance', description: 'For your future', type: 'number'}, + {id: '6', name: 'Height', description: 'In inches, for your basketball career', type: 'slider'}, + {id: '7', name: 'Actions', description: 'To take right now', type: 'menu'}, + {id: '8', name: 'Checkbox', description: 'To check', type: 'checkbox'}, + {id: '9', name: 'Radio', description: 'To choose', type: 'radio'}, + {id: '10', name: 'Wall colour', description: 'So your room sparks joy', type: 'color'}, + {id: '11', name: 'References', description: 'Handy link to your favourite website', type: 'link'}, + {id: '12', name: 'Mythical dogs', description: 'Which would you adopt', type: 'tags'}, + {id: '13', name: 'Superstitions enabled', description: 'Whether or not 13 is bad luck', type: 'switch'} +]; + +let dataColumns = [ + {name: 'Name', id: 'name', isRowHeader: true, minWidth: 200}, + {name: 'Data', id: 'editable', minWidth: 300}, + {name: 'Description', id: 'description', minWidth: 200} +]; + +let formatOptions: Intl.NumberFormatOptions = { + style: 'currency', + currency: 'USD' +}; + +export const TableWithTextFields: StoryObj = { + render: (args) => ( + + + {(column) => ( + {column.name} + )} + + + {item => ( + + {(column) => { + if (column.name === 'Data') { + switch (item.type) { + case 'text': + return
; + case 'date': + return
; + case 'combobox': + return ( + +
+ + Red + Green + Blue + +
+ + ); + case 'picker': + return ( + +
+ + Cat + Dog + Bird + +
+ + ); + case 'number': + return
; + case 'slider': + return
; + case 'menu': + return ( + +
+ + Copy + Delete + +
+ + ); + case 'checkbox': + return ( + +
+ + Airpods + Kindle + +
+ + ); + case 'radio': + return ( + +
+ + Chicken + Veggie + +
+ + ); + case 'color': + return ( + +
+ +
+ + ); + case 'link': + return ( + +
Adobe
+ + ); + case 'tags': + return ( + +
+ + Cerberus + Gellert + Fenris + +
+ + ); + case 'switch': + return ( + +
+ +
+ + ); + } + } + return {item[column.id]}; + }} + + )} + + + ), + args: { + ...Example.args + }, + parameters: { + docs: { + disable: true + } + } +}; + Example.parameters = { docs: { source: { diff --git a/packages/@react-spectrum/table/test/TableTests.js b/packages/@react-spectrum/table/test/TableTests.js index c2f21cc6fd5..1e74fe9e0db 100644 --- a/packages/@react-spectrum/table/test/TableTests.js +++ b/packages/@react-spectrum/table/test/TableTests.js @@ -1815,30 +1815,21 @@ export let tableTests = () => { expect(document.activeElement).toBe(tree.getAllByRole('switch')[1]); // Simulate tabbing within the table - fireEvent.keyDown(document.activeElement, {key: 'Tab'}); - let walker = getFocusableTreeWalker(document.body, {tabbable: true}); - walker.currentNode = document.activeElement; - act(() => {walker.nextNode().focus();}); - fireEvent.keyUp(document.activeElement, {key: 'Tab'}); + await user.tab(); + await user.tab(); let after = tree.getByTestId('after'); expect(document.activeElement).toBe(after); }); - it('should move focus after the table when tabbing from the last row', function () { + it('should move focus after the table when tabbing from the last row', async function () { let tree = renderFocusable(); act(() => tree.getAllByRole('row')[2].focus()); expect(document.activeElement).toBe(tree.getAllByRole('row')[2]); // Simulate tabbing within the table - act(() => { - fireEvent.keyDown(document.activeElement, {key: 'Tab'}); - let walker = getFocusableTreeWalker(document.body, {tabbable: true}); - walker.currentNode = document.activeElement; - walker.nextNode().focus(); - fireEvent.keyUp(document.activeElement, {key: 'Tab'}); - }); + await user.tab(); let after = tree.getByTestId('after'); expect(document.activeElement).toBe(after); @@ -1851,11 +1842,7 @@ export let tableTests = () => { expect(document.activeElement).toBe(tree.getAllByRole('switch')[1]); // Simulate shift tabbing within the table - fireEvent.keyDown(document.activeElement, {key: 'Tab', shiftKey: true}); - let walker = getFocusableTreeWalker(document.body, {tabbable: true}); - walker.currentNode = document.activeElement; - act(() => {walker.previousNode().focus();}); - fireEvent.keyUp(document.activeElement, {key: 'Tab', shiftKey: true}); + await user.tab({shift: true}); let before = tree.getByTestId('before'); expect(document.activeElement).toBe(before); @@ -3919,24 +3906,21 @@ export let tableTests = () => { await user.tab(); expect(document.activeElement).toBe(rows[1]); - fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft'}); - fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft'}); + await user.keyboard('{ArrowLeft}'); expect(document.activeElement).toBe(within(rows[1]).getByRole('button')); - fireEvent.keyDown(document.activeElement, {key: 'Enter'}); - fireEvent.keyUp(document.activeElement, {key: 'Enter'}); + await user.keyboard('{Enter}'); let menu = tree.getByRole('menu'); let menuItems = within(menu).getAllByRole('menuitem'); expect(menuItems.length).toBe(2); expect(document.activeElement).toBe(menuItems[0]); - fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); - fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); + await user.keyboard('{ArrowDown}'); expect(document.activeElement).toBe(menuItems[1]); - fireEvent.keyDown(document.activeElement, {key: 'Enter'}); - fireEvent.keyUp(document.activeElement, {key: 'Enter'}); + await user.keyboard('{Enter}'); + act(() => jest.runAllTimers()); expect(menu).not.toBeInTheDocument(); let dialog = tree.getByRole('alertdialog', {hidden: true}); diff --git a/packages/react-aria-components/test/Table.test.js b/packages/react-aria-components/test/Table.test.js index 667096ceabd..ca71f6b147f 100644 --- a/packages/react-aria-components/test/Table.test.js +++ b/packages/react-aria-components/test/Table.test.js @@ -11,7 +11,7 @@ */ import {act, fireEvent, installPointerEvent, mockClickDefault, pointerMap, render, setupIntersectionObserverMock, triggerLongPress, within} from '@react-spectrum/test-utils-internal'; -import {Button, Cell, Checkbox, Collection, Column, ColumnResizer, Dialog, DialogTrigger, DropIndicator, Label, Modal, ResizableTableContainer, Row, Table, TableBody, TableHeader, TableLayout, TableLoadMoreItem, Tag, TagGroup, TagList, useDragAndDrop, useTableOptions, Virtualizer} from '../'; +import {Button, Cell, Checkbox, Collection, Column, ColumnResizer, Dialog, DialogTrigger, DropIndicator, Input, Label, Modal, ResizableTableContainer, Row, Table, TableBody, TableHeader, TableLayout, TableLoadMoreItem, Tag, TagGroup, TagList, TextField, useDragAndDrop, useTableOptions, Virtualizer} from '../'; import {composeStories} from '@storybook/react'; import {DataTransfer, DragEvent} from '@react-aria/dnd/test/mocks'; import React, {useMemo, useState} from 'react'; @@ -2646,7 +2646,7 @@ describe('Table', () => { let {getByRole} = renderTable({rowProps: {onAction, onPressStart, onPressEnd, onPress, onClick}}); let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')}); await tableTester.triggerRowAction({row: 1, interactionType}); - + expect(onAction).toHaveBeenCalledTimes(1); expect(onPressStart).toHaveBeenCalledTimes(1); expect(onPressEnd).toHaveBeenCalledTimes(1); @@ -2654,6 +2654,255 @@ describe('Table', () => { expect(onClick).toHaveBeenCalledTimes(1); }); }); + + describe.skip('Editable fields in cells', () => { + describe.each(['none', 'single', 'multiple'])('selectionMode: %s', (selectionMode) => { + it('should support editing a textfield in a cell in a table with keyboard interactions', async () => { + let {getByRole, getAllByRole} = render( + <> + + + + + + Name + Type + Description + + + + + + + Games + File folder + + + + + + + + + + + + + + Fonts + Font folder + + + + + + + + + + +
+ + + ); + + // Keyboard navigate to first textfield in first row + let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid'), interactionType: 'keyboard'}); + let inputs = getAllByRole('textbox'); + let button = getByRole('button', {name: 'After'}); + await user.tab(); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{ArrowRight}'); + if (selectionMode === 'none') { + expect(tableTester.cells({element: tableTester.rows[0]})[2]).toHaveFocus(); + } else { + // in selection modes, account for extra checkbox column + await user.keyboard('{ArrowRight}'); + expect(tableTester.cells({element: tableTester.rows[0]})[3]).toHaveFocus(); + } + expect(inputs[0]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); + expect(inputs[0]).toHaveFocus(); + await user.keyboard('{ArrowLeft}'); + expect(inputs[0]).toHaveFocus(); + // Type a string that would trigger a typeahead or selection if we weren't in a textfield + await user.keyboard('B '); + expect(tableTester.selectedRows).toHaveLength(0); + expect(inputs[0]).toHaveFocus(); + + // Navigate to second textfield in first row + await user.tab(); + expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); + expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowLeft}'); + expect(inputs[1]).toHaveFocus(); + // Type a string that would trigger a typeahead or selection if we weren't in a textfield + await user.keyboard('E '); + expect(tableTester.selectedRows).toHaveLength(0); + expect(inputs[1]).toHaveFocus(); + + await user.tab(); + expect(button).toHaveFocus(); + + // Come back to the table, we should remember roughly where we were, in this case, on the cell containing the input. + // We may want this to focus the input itself instead of the cell. + await user.tab({shift: true}); + if (selectionMode === 'none') { + expect(tableTester.cells({element: tableTester.rows[0]})[2]).toHaveFocus(); + } else { + expect(tableTester.cells({element: tableTester.rows[0]})[3]).toHaveFocus(); + } + }); + + describe('pointer interactions', () => { + installPointerEvent(); + + it('should support editing a textfield in a cell in a table with mouse interactions', async () => { + let {getByRole, getAllByRole} = render( + <> + + + + + + Name + Type + Description + + + + + + + Games + File folder + + + + + + + + + + + + + + Fonts + Font folder + + + + + + + + + + +
+ + + ); + + // click on the first textfield in the first row + let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')}); + let inputs = getAllByRole('textbox'); + await user.click(inputs[0]); + expect(inputs[0]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); + expect(inputs[0]).toHaveFocus(); + await user.keyboard('{ArrowLeft}'); + expect(inputs[0]).toHaveFocus(); + // Type a string that would trigger a typeahead or selection if we weren't in a textfield + await user.keyboard('B '); + expect(tableTester.selectedRows).toHaveLength(0); + expect(inputs[0]).toHaveFocus(); + + // click on the second textfield in the first row + await user.click(inputs[1]); + expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); + expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowLeft}'); + expect(inputs[1]).toHaveFocus(); + // Type a string that would trigger a typeahead or selection if we weren't in a textfield + await user.keyboard('E '); + expect(tableTester.selectedRows).toHaveLength(0); + expect(inputs[1]).toHaveFocus(); + }); + }); + }); + + it('should support navigation with a disabled textfield in a cell in a non-selectable table', async () => { + let {getByRole, getAllByRole} = render( + <> + + + Name + Type + Description + + + + Games + File folder + + + + + + + + + + + Fonts + Font folder + + + + + + + + + + +
+ + + ); + + // Keyboard navigate to first textfield in first row + let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid'), interactionType: 'keyboard'}); + let inputs = getAllByRole('textbox'); + let button = getByRole('button', {name: 'After'}); + await user.tab(); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{ArrowRight}'); + expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); + expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowLeft}'); + expect(inputs[1]).toHaveFocus(); + // Type a string that would trigger a typeahead or selection if we weren't in a textfield + await user.keyboard('B '); + expect(tableTester.selectedRows).toHaveLength(0); + expect(inputs[1]).toHaveFocus(); + + await user.tab({shift: true}); + + await user.keyboard('{ArrowDown}'); + await user.tab(); + expect(button).toHaveFocus(); + }); + }); }); function HidingColumnsExample({dynamic = false}) { From 3bc865fc2ecc9d8d8c41514e6f3a8c2b753b2f7a Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 26 Aug 2025 16:53:39 +1000 Subject: [PATCH 4/5] fix lint --- packages/@react-aria/selection/src/useSelectableCollection.ts | 4 ++-- packages/@react-spectrum/table/test/TableTests.js | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index a19ab3159c5..e005f159ec1 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -10,11 +10,11 @@ * governing permissions and limitations under the License. */ +import {BaseEvent, DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils'; import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus'; -import {BaseEvent, DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; import {flushSync} from 'react-dom'; -import {FocusEvent, KeyboardEvent, useCallback, useEffect, useRef} from 'react'; +import {FocusEvent, KeyboardEvent, useEffect, useRef} from 'react'; import {focusSafely, getInteractionModality, useKeyboard} from '@react-aria/interactions'; import {getItemElement, isNonContiguousSelectionModifier, useCollectionId} from './utils'; import {MultipleSelectionManager} from '@react-stately/selection'; diff --git a/packages/@react-spectrum/table/test/TableTests.js b/packages/@react-spectrum/table/test/TableTests.js index 1e74fe9e0db..57b76a66a8c 100644 --- a/packages/@react-spectrum/table/test/TableTests.js +++ b/packages/@react-spectrum/table/test/TableTests.js @@ -23,7 +23,6 @@ import {Content} from '@react-spectrum/view'; import {CRUDExample} from '../stories/CRUDExample'; import {Dialog, DialogTrigger} from '@react-spectrum/dialog'; import {Divider} from '@react-spectrum/divider'; -import {getFocusableTreeWalker} from '@react-aria/focus'; import {Heading} from '@react-spectrum/text'; import {Item, Picker} from '@react-spectrum/picker'; import {Link} from '@react-spectrum/link'; From 044c69014efb7dea4326330fffda2efebaede10d Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 28 Aug 2025 17:26:21 +1000 Subject: [PATCH 5/5] enable textfield inside table tests and fix logic --- packages/@react-aria/grid/src/useGridCell.ts | 39 ++++++------ .../@react-aria/grid/test/useGrid.test.js | 4 +- .../@react-aria/textfield/src/useTextField.ts | 62 ++++++++++++++----- .../@react-spectrum/table/test/TableTests.js | 4 +- .../react-aria-components/test/Table.test.js | 28 ++++----- 5 files changed, 82 insertions(+), 55 deletions(-) diff --git a/packages/@react-aria/grid/src/useGridCell.ts b/packages/@react-aria/grid/src/useGridCell.ts index 8a4e3df0a43..9b06bd0e4b3 100644 --- a/packages/@react-aria/grid/src/useGridCell.ts +++ b/packages/@react-aria/grid/src/useGridCell.ts @@ -113,25 +113,26 @@ export function useGridCell>(props: GridCellProps return; } - if (e.target !== ref.current && e.key !== 'ArrowLeft' && e.key !== 'ArrowRight' && e.key !== 'ArrowUp' && e.key !== 'ArrowDown') { - if (e.key === 'Tab' && ref.current.contains(e.target as Element)) { - let cellWalker = getFocusableTreeWalker(ref.current, {tabbable: true}); - if (e.shiftKey) { - cellWalker.currentNode = ref.current; - let isFirstFocusable = cellWalker.firstChild() === e.target; - if (!isFirstFocusable) { - e.stopPropagation(); - } - } else { - cellWalker.currentNode = ref.current; - let isLastFocusable = cellWalker.lastChild() === e.target; - if (!isLastFocusable) { - e.stopPropagation(); - } - } - } - return; - } + // TODO: keyboard handler should only stop propagation on keys we intend to handle, not ALL keys, that way we don't have to call continue then call stop else where but only *sometimes* depending on the order + // if (e.target !== ref.current && e.key !== 'ArrowLeft' && e.key !== 'ArrowRight' && e.key !== 'ArrowUp' && e.key !== 'ArrowDown') { + // if (e.key === 'Tab' && ref.current.contains(e.target as Element)) { + // let cellWalker = getFocusableTreeWalker(ref.current, {tabbable: true}); + // if (e.shiftKey) { + // cellWalker.currentNode = ref.current; + // let isFirstFocusable = cellWalker.firstChild() === e.target; + // if (!isFirstFocusable) { + // e.stopPropagation(); + // } + // } else { + // cellWalker.currentNode = ref.current; + // let isLastFocusable = cellWalker.lastChild() === e.target; + // if (!isLastFocusable) { + // e.stopPropagation(); + // } + // } + // } + // return; + // } let walker = getFocusableTreeWalker(ref.current); walker.currentNode = document.activeElement; diff --git a/packages/@react-aria/grid/test/useGrid.test.js b/packages/@react-aria/grid/test/useGrid.test.js index bb479975c9f..8804655b940 100644 --- a/packages/@react-aria/grid/test/useGrid.test.js +++ b/packages/@react-aria/grid/test/useGrid.test.js @@ -60,7 +60,7 @@ describe('useGrid', () => { await user.keyboard('[ArrowRight]'); expect(document.activeElement).toBe(tree.getAllByRole('switch')[0]); - await user.tab(); + await user.keyboard('[ArrowRight]'); expect(document.activeElement).toBe(tree.getAllByRole('switch')[1]); await user.keyboard('[ArrowRight]'); @@ -71,7 +71,7 @@ describe('useGrid', () => { act(() => tree.getAllByRole('switch')[1].focus()); - await user.tab({shift: true}); + await user.keyboard('[ArrowLeft]'); expect(document.activeElement).toBe(tree.getAllByRole('switch')[0]); await user.keyboard('[ArrowLeft]'); diff --git a/packages/@react-aria/textfield/src/useTextField.ts b/packages/@react-aria/textfield/src/useTextField.ts index 6b7f9601da2..a368b537af9 100644 --- a/packages/@react-aria/textfield/src/useTextField.ts +++ b/packages/@react-aria/textfield/src/useTextField.ts @@ -135,31 +135,47 @@ export function useTextField>) => { if ((e.key === 'ArrowLeft' || e.key === 'ArrowUp') && (e.target as HTMLInputElement).selectionStart === 0 && (e.target as HTMLInputElement).selectionEnd === 0) { - e.continuePropagation(); - } - if ((e.key === 'ArrowRight' || e.key === 'ArrowDown') + if (e.isPropagationStopped()) { + e.continuePropagation(); + } + } else if ((e.key === 'ArrowRight' || e.key === 'ArrowDown') && (e.target as HTMLInputElement).selectionStart === (e.target as HTMLInputElement).value.length && (e.target as HTMLInputElement).selectionEnd === (e.target as HTMLInputElement).value.length) { - e.continuePropagation(); - } - if (KEYS_TO_CONTINUE_PROPAGATION.has(e.key)) { - e.continuePropagation(); + if (e.isPropagationStopped()) { + e.continuePropagation(); + } + } else if (KEYS_TO_CONTINUE_PROPAGATION.has(e.key)) { + if (e.isPropagationStopped()) { + e.continuePropagation(); + } + } else { + if (!e.isPropagationStopped()) { + e.stopPropagation(); + } }; onKeyUpProp?.(e); }, [onKeyUpProp]); @@ -233,6 +249,24 @@ export function useTextField= 17 ? 'enterKeyHint' : 'enterkeyhint']: props.enterKeyHint, + // TODO: Always?? or only if we're inside a grid? in which case maybe I should do this in all of Grid hooks by checking if target instance of HTMLInputElement? + onPointerDown: (e) => { + e.stopPropagation(); + props.onKeyDown?.(e); + }, + onPointerUp: (e) => { + e.stopPropagation(); + props.onKeyUp?.(e); + }, + onClick: (e) => { + e.stopPropagation(); + props.onClick?.(e); + }, + onDoubleClick: (e) => { + e.stopPropagation(); + props.onDoubleClick?.(e); + }, + // Clipboard events onCopy: props.onCopy, onCut: props.onCut, diff --git a/packages/@react-spectrum/table/test/TableTests.js b/packages/@react-spectrum/table/test/TableTests.js index 57b76a66a8c..e1db50dbd97 100644 --- a/packages/@react-spectrum/table/test/TableTests.js +++ b/packages/@react-spectrum/table/test/TableTests.js @@ -1815,7 +1815,6 @@ export let tableTests = () => { // Simulate tabbing within the table await user.tab(); - await user.tab(); let after = tree.getByTestId('after'); expect(document.activeElement).toBe(after); @@ -4252,8 +4251,7 @@ export let tableTests = () => { expect(document.activeElement).toEqual(input); - fireEvent.keyDown(input, {key: 'Escape', code: 27, charCode: 27}); - fireEvent.keyUp(input, {key: 'Escape', code: 27, charCode: 27}); + await user.keyboard('{Escape}'); act(() => { jest.runAllTimers(); }); diff --git a/packages/react-aria-components/test/Table.test.js b/packages/react-aria-components/test/Table.test.js index ca71f6b147f..4c4de8901e7 100644 --- a/packages/react-aria-components/test/Table.test.js +++ b/packages/react-aria-components/test/Table.test.js @@ -2655,7 +2655,7 @@ describe('Table', () => { }); }); - describe.skip('Editable fields in cells', () => { + describe('Editable fields in cells', () => { describe.each(['none', 'single', 'multiple'])('selectionMode: %s', (selectionMode) => { it('should support editing a textfield in a cell in a table with keyboard interactions', async () => { let {getByRole, getAllByRole} = render( @@ -2716,15 +2716,14 @@ describe('Table', () => { await user.keyboard('{ArrowRight}'); await user.keyboard('{ArrowRight}'); if (selectionMode === 'none') { - expect(tableTester.cells({element: tableTester.rows[0]})[2]).toHaveFocus(); + expect(inputs[0]).toHaveFocus(); } else { // in selection modes, account for extra checkbox column await user.keyboard('{ArrowRight}'); - expect(tableTester.cells({element: tableTester.rows[0]})[3]).toHaveFocus(); + expect(inputs[0]).toHaveFocus(); } - expect(inputs[0]).toHaveFocus(); await user.keyboard('{ArrowRight}'); - expect(inputs[0]).toHaveFocus(); + expect(inputs[1]).toHaveFocus(); await user.keyboard('{ArrowLeft}'); expect(inputs[0]).toHaveFocus(); // Type a string that would trigger a typeahead or selection if we weren't in a textfield @@ -2733,10 +2732,10 @@ describe('Table', () => { expect(inputs[0]).toHaveFocus(); // Navigate to second textfield in first row - await user.tab(); - expect(inputs[1]).toHaveFocus(); await user.keyboard('{ArrowRight}'); expect(inputs[1]).toHaveFocus(); + await user.keyboard('{ArrowRight}'); + expect(tableTester.rows[0]).toHaveFocus(); await user.keyboard('{ArrowLeft}'); expect(inputs[1]).toHaveFocus(); // Type a string that would trigger a typeahead or selection if we weren't in a textfield @@ -2750,11 +2749,7 @@ describe('Table', () => { // Come back to the table, we should remember roughly where we were, in this case, on the cell containing the input. // We may want this to focus the input itself instead of the cell. await user.tab({shift: true}); - if (selectionMode === 'none') { - expect(tableTester.cells({element: tableTester.rows[0]})[2]).toHaveFocus(); - } else { - expect(tableTester.cells({element: tableTester.rows[0]})[3]).toHaveFocus(); - } + expect(inputs[0]).toHaveFocus(); // TODO: this should be the second input if we were on it previously, but it's always the first input right now }); describe('pointer interactions', () => { @@ -2815,7 +2810,7 @@ describe('Table', () => { await user.click(inputs[0]); expect(inputs[0]).toHaveFocus(); await user.keyboard('{ArrowRight}'); - expect(inputs[0]).toHaveFocus(); + expect(inputs[1]).toHaveFocus(); await user.keyboard('{ArrowLeft}'); expect(inputs[0]).toHaveFocus(); // Type a string that would trigger a typeahead or selection if we weren't in a textfield @@ -2827,7 +2822,7 @@ describe('Table', () => { await user.click(inputs[1]); expect(inputs[1]).toHaveFocus(); await user.keyboard('{ArrowRight}'); - expect(inputs[1]).toHaveFocus(); + expect(tableTester.rows[0]).toHaveFocus(); await user.keyboard('{ArrowLeft}'); expect(inputs[1]).toHaveFocus(); // Type a string that would trigger a typeahead or selection if we weren't in a textfield @@ -2888,7 +2883,7 @@ describe('Table', () => { await user.keyboard('{ArrowRight}'); expect(inputs[1]).toHaveFocus(); await user.keyboard('{ArrowRight}'); - expect(inputs[1]).toHaveFocus(); + expect(tableTester.rows[0]).toHaveFocus(); await user.keyboard('{ArrowLeft}'); expect(inputs[1]).toHaveFocus(); // Type a string that would trigger a typeahead or selection if we weren't in a textfield @@ -2896,8 +2891,7 @@ describe('Table', () => { expect(tableTester.selectedRows).toHaveLength(0); expect(inputs[1]).toHaveFocus(); - await user.tab({shift: true}); - + // TODO: correct behaviour? selection cursor is where it would be if you pressed down, so it doesn't do anything, so should it be allowed to navigate cells now? await user.keyboard('{ArrowDown}'); await user.tab(); expect(button).toHaveFocus();