From 58817da824ab176f19c6eedc1487d25a05ee9ef5 Mon Sep 17 00:00:00 2001 From: jsmitrah Date: Thu, 18 Jun 2026 15:48:35 +0530 Subject: [PATCH 1/3] fix: prevent useGridCell from overriding child focus restoration loops --- packages/react-aria/src/grid/useGridCell.ts | 71 ++++++++++--------- packages/react-aria/test/grid/useGrid.test.js | 50 ++++++++++--- 2 files changed, 77 insertions(+), 44 deletions(-) diff --git a/packages/react-aria/src/grid/useGridCell.ts b/packages/react-aria/src/grid/useGridCell.ts index a7f366eab2d..2984c378909 100644 --- a/packages/react-aria/src/grid/useGridCell.ts +++ b/packages/react-aria/src/grid/useGridCell.ts @@ -10,28 +10,28 @@ * governing permissions and limitations under the License. */ -import {DOMAttributes, FocusableElement, Key, RefObject} from '@react-types/shared'; -import {focusSafely} from '../interactions/focusSafely'; +import { DOMAttributes, FocusableElement, Key, RefObject } from '@react-types/shared'; +import { focusSafely } from '../interactions/focusSafely'; import { getActiveElement, getEventTarget, isFocusWithin, nodeContains } from '../utils/shadowdom/DOMFunctions'; -import {getFocusableTreeWalker} from '../focus/FocusScope'; -import {getScrollParent} from '../utils/getScrollParent'; +import { getFocusableTreeWalker } from '../focus/FocusScope'; +import { getScrollParent } from '../utils/getScrollParent'; import { IGridCollection as GridCollection, GridNode } from 'react-stately/private/grid/GridCollection'; -import {gridMap} from './utils'; -import {GridState} from 'react-stately/private/grid/useGridState'; -import {isFocusVisible} from '../interactions/useFocusVisible'; -import {mergeProps} from '../utils/mergeProps'; -import {KeyboardEvent as ReactKeyboardEvent, useRef} from 'react'; -import {scrollIntoViewport} from '../utils/scrollIntoView'; -import {useLocale} from '../i18n/I18nProvider'; -import {useSelectableItem} from '../selection/useSelectableItem'; +import { gridMap } from './utils'; +import { GridState } from 'react-stately/private/grid/useGridState'; +import { isFocusVisible } from '../interactions/useFocusVisible'; +import { mergeProps } from '../utils/mergeProps'; +import { KeyboardEvent as ReactKeyboardEvent, useRef } from 'react'; +import { scrollIntoViewport } from '../utils/scrollIntoView'; +import { useLocale } from '../i18n/I18nProvider'; +import { useSelectableItem } from '../selection/useSelectableItem'; export interface GridCellProps { /** @@ -77,12 +77,12 @@ export function useGridCell>( state: GridState, ref: RefObject ): GridCellAria { - let {node, isVirtualized, focusMode = 'child', shouldSelectOnPressUp, onAction} = props; + let { node, isVirtualized, focusMode = 'child', shouldSelectOnPressUp, onAction } = props; - let {direction} = useLocale(); + let { direction } = useLocale(); let { keyboardDelegate, - actions: {onCellAction} + actions: { onCellAction } } = gridMap.get(state)!; // We need to track the key of the item at the time it was last focused so that we force @@ -119,7 +119,7 @@ export function useGridCell>( } }; - let {itemProps, isPressed} = useSelectableItem({ + let { itemProps, isPressed } = useSelectableItem({ selectionManager: state.selectionManager, key: node.key, ref, @@ -161,7 +161,7 @@ export function useGridCell>( e.stopPropagation(); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); + scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); } else { // If there is no next focusable child, then move to the next cell to the left of this one. // This will be handled by useSelectableCollection. However, if there is no cell to the left @@ -181,14 +181,14 @@ export function useGridCell>( if (focusMode === 'cell' && direction === 'rtl') { focusSafely(ref.current); - scrollIntoViewport(ref.current, {containingElement: getScrollParent(ref.current)}); + scrollIntoViewport(ref.current, { containingElement: getScrollParent(ref.current) }); } else { walker.currentNode = ref.current; focusable = direction === 'rtl' ? (walker.firstChild() as FocusableElement) : last(walker); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); + scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); } } } @@ -208,7 +208,7 @@ export function useGridCell>( e.stopPropagation(); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); + scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); } else { let next = keyboardDelegate.getKeyRightOf?.(node.key); if (next !== node.key) { @@ -223,14 +223,14 @@ export function useGridCell>( if (focusMode === 'cell' && direction === 'ltr') { focusSafely(ref.current); - scrollIntoViewport(ref.current, {containingElement: getScrollParent(ref.current)}); + scrollIntoViewport(ref.current, { containingElement: getScrollParent(ref.current) }); } else { walker.currentNode = ref.current; focusable = direction === 'rtl' ? last(walker) : (walker.firstChild() as FocusableElement); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); + scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); } } } @@ -254,28 +254,29 @@ export function useGridCell>( // Grid cells can have focusable elements inside them. In this case, focus should // be marshalled to that element rather than focusing the cell itself. - let onFocus = e => { + let onFocus = (e: any) => { keyWhenFocused.current = node.key; + + // FIX: If focus is moving directly to a specific inner child element + // (like our restored Open Dialog button), update the selection state + // and exit early. Do not call focus(), which resets to the first child. if (getEventTarget(e) !== ref.current) { - // useSelectableItem only handles setting the focused key when - // the focused element is the gridcell itself. We also want to - // set the focused key when a child element receives focus. - // If focus is currently visible (e.g. the user is navigating with the keyboard), - // then skip this. We want to restore focus to the previously focused row/cell - // in that case since the table should act like a single tab stop. if (!isFocusVisible()) { state.selectionManager.setFocusedKey(node.key); } return; } - // If the cell itself is focused, wait a frame so that focus finishes propagatating - // up to the tree, and move focus to a focusable child if possible. - requestAnimationFrame(() => { - if (focusMode === 'child' && getActiveElement() === ref.current) { - focus(); + if (focusMode === 'child') { + // Safeguard: Check if the browser's active element has already shifted + // into a nested child node during this event loop tick before forcing a reset. + let activeElement = getActiveElement(); + if (ref.current && isFocusWithin(ref.current) && activeElement !== ref.current) { + return; } - }); + + focus(); + } }; let gridCellProps: DOMAttributes = mergeProps(itemProps, { diff --git a/packages/react-aria/test/grid/useGrid.test.js b/packages/react-aria/test/grid/useGrid.test.js index 20b1915ef35..5668df18209 100644 --- a/packages/react-aria/test/grid/useGrid.test.js +++ b/packages/react-aria/test/grid/useGrid.test.js @@ -10,11 +10,11 @@ * governing permissions and limitations under the License. */ -import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; -import {Grid} from '../../stories/grid/example'; -import {Item} from 'react-stately/Item'; +import { act, pointerMap, render } from '@react-spectrum/test-utils-internal'; +import { Grid } from '../../stories/grid/example'; +import { Item } from 'react-stately/Item'; import React from 'react'; -import {Switch} from '@adobe/react-spectrum/Switch'; +import { Switch } from '@adobe/react-spectrum/Switch'; import userEvent from '@testing-library/user-event'; function renderGrid(props = {}) { @@ -41,7 +41,7 @@ describe('useGrid', () => { let user; beforeAll(() => { - user = userEvent.setup({delay: null, pointerMap}); + user = userEvent.setup({ delay: null, pointerMap }); jest.useFakeTimers(); }); afterEach(() => { @@ -51,7 +51,7 @@ describe('useGrid', () => { }); }); it('gridFocusMode = row, cellFocusMode = cell', async () => { - let tree = renderGrid({gridFocusMode: 'row', cellFocusMode: 'cell'}); + let tree = renderGrid({ gridFocusMode: 'row', cellFocusMode: 'cell' }); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('row')[0]); @@ -84,7 +84,7 @@ describe('useGrid', () => { }); it('gridFocusMode = row, cellFocusMode = child', async () => { - let tree = renderGrid({gridFocusMode: 'row', cellFocusMode: 'child'}); + let tree = renderGrid({ gridFocusMode: 'row', cellFocusMode: 'child' }); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('row')[0]); @@ -112,7 +112,7 @@ describe('useGrid', () => { }); it('gridFocusMode = cell, cellFocusMode = child', async () => { - let tree = renderGrid({gridFocusMode: 'cell', cellFocusMode: 'child'}); + let tree = renderGrid({ gridFocusMode: 'cell', cellFocusMode: 'child' }); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('switch')[0]); @@ -134,7 +134,7 @@ describe('useGrid', () => { }); it('gridFocusMode = cell, cellFocusMode = cell', async () => { - let tree = renderGrid({gridFocusMode: 'cell', cellFocusMode: 'cell'}); + let tree = renderGrid({ gridFocusMode: 'cell', cellFocusMode: 'cell' }); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('gridcell')[0]); @@ -157,4 +157,36 @@ describe('useGrid', () => { await user.keyboard('[ArrowLeft]'); expect(document.activeElement).toBe(tree.getAllByRole('gridcell')[0]); }); + + it('should retain focus on a specific child element if focus is restored to it', async () => { + let tree = renderGrid({ gridFocusMode: 'cell', cellFocusMode: 'child' }); + let switches = tree.getAllByRole('switch'); + let cells = tree.getAllByRole('gridcell'); + + // 1. Initially move focus onto the grid via tab flow (focuses Switch 1) + await user.tab(); + expect(document.activeElement).toBe(switches[0]); + + // 2. Simulate focus returning directly to the second target element (Switch 2) + // exactly how the focus-restoration logic inside an overlay does it. + act(() => { + switches[1].focus(); + }); + expect(document.activeElement).toBe(switches[1]); + + // 3. Fire a focus event directly on the gridcell container to trigger your + // onFocus handler in useGridCell.ts and simulate event bubbling + act(() => { + cells[0].dispatchEvent(new FocusEvent('focus', { bubbles: true })); + }); + + // 4. Force Jest's timer and requestAnimationFrame microtask cycles to execute completely + act(() => { + jest.runAllTimers(); + }); + + // 5. FINAL ASSERTION: Focus should accurately stay locked onto Switch 2 + // instead of resetting to the initial index element Switch 1! + expect(document.activeElement).toBe(switches[1]); + }); }); From 714723c15eeb70d4cadb373d5c996bf515278e33 Mon Sep 17 00:00:00 2001 From: jsmitrah Date: Thu, 18 Jun 2026 19:45:50 +0530 Subject: [PATCH 2/3] fix: resolve lint and CI test compilation errors --- packages/react-aria/src/grid/useGridCell.ts | 54 ++++++++++--------- packages/react-aria/test/grid/useGrid.test.js | 24 ++++----- 2 files changed, 41 insertions(+), 37 deletions(-) mode change 100644 => 100755 packages/react-aria/test/grid/useGrid.test.js diff --git a/packages/react-aria/src/grid/useGridCell.ts b/packages/react-aria/src/grid/useGridCell.ts index 2984c378909..4e2ddfe9bd6 100644 --- a/packages/react-aria/src/grid/useGridCell.ts +++ b/packages/react-aria/src/grid/useGridCell.ts @@ -10,28 +10,28 @@ * governing permissions and limitations under the License. */ -import { DOMAttributes, FocusableElement, Key, RefObject } from '@react-types/shared'; -import { focusSafely } from '../interactions/focusSafely'; +import {DOMAttributes, FocusableElement, Key, RefObject} from '@react-types/shared'; +import {focusSafely} from '../interactions/focusSafely'; import { getActiveElement, getEventTarget, isFocusWithin, nodeContains } from '../utils/shadowdom/DOMFunctions'; -import { getFocusableTreeWalker } from '../focus/FocusScope'; -import { getScrollParent } from '../utils/getScrollParent'; +import {getFocusableTreeWalker} from '../focus/FocusScope'; +import {getScrollParent} from '../utils/getScrollParent'; import { IGridCollection as GridCollection, GridNode } from 'react-stately/private/grid/GridCollection'; -import { gridMap } from './utils'; -import { GridState } from 'react-stately/private/grid/useGridState'; -import { isFocusVisible } from '../interactions/useFocusVisible'; -import { mergeProps } from '../utils/mergeProps'; -import { KeyboardEvent as ReactKeyboardEvent, useRef } from 'react'; -import { scrollIntoViewport } from '../utils/scrollIntoView'; -import { useLocale } from '../i18n/I18nProvider'; -import { useSelectableItem } from '../selection/useSelectableItem'; +import {gridMap} from './utils'; +import {GridState} from 'react-stately/private/grid/useGridState'; +import {isFocusVisible} from '../interactions/useFocusVisible'; +import {mergeProps} from '../utils/mergeProps'; +import {KeyboardEvent as ReactKeyboardEvent, useRef} from 'react'; +import {scrollIntoViewport} from '../utils/scrollIntoView'; +import {useLocale} from '../i18n/I18nProvider'; +import {useSelectableItem} from '../selection/useSelectableItem'; export interface GridCellProps { /** @@ -77,12 +77,12 @@ export function useGridCell>( state: GridState, ref: RefObject ): GridCellAria { - let { node, isVirtualized, focusMode = 'child', shouldSelectOnPressUp, onAction } = props; + let {node, isVirtualized, focusMode = 'child', shouldSelectOnPressUp, onAction} = props; - let { direction } = useLocale(); + let {direction} = useLocale(); let { keyboardDelegate, - actions: { onCellAction } + actions: {onCellAction} } = gridMap.get(state)!; // We need to track the key of the item at the time it was last focused so that we force @@ -119,7 +119,7 @@ export function useGridCell>( } }; - let { itemProps, isPressed } = useSelectableItem({ + let {itemProps, isPressed} = useSelectableItem({ selectionManager: state.selectionManager, key: node.key, ref, @@ -161,7 +161,7 @@ export function useGridCell>( e.stopPropagation(); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); + scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); } else { // If there is no next focusable child, then move to the next cell to the left of this one. // This will be handled by useSelectableCollection. However, if there is no cell to the left @@ -181,14 +181,14 @@ export function useGridCell>( if (focusMode === 'cell' && direction === 'rtl') { focusSafely(ref.current); - scrollIntoViewport(ref.current, { containingElement: getScrollParent(ref.current) }); + scrollIntoViewport(ref.current, {containingElement: getScrollParent(ref.current)}); } else { walker.currentNode = ref.current; focusable = direction === 'rtl' ? (walker.firstChild() as FocusableElement) : last(walker); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); + scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); } } } @@ -208,7 +208,7 @@ export function useGridCell>( e.stopPropagation(); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); + scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); } else { let next = keyboardDelegate.getKeyRightOf?.(node.key); if (next !== node.key) { @@ -223,14 +223,14 @@ export function useGridCell>( if (focusMode === 'cell' && direction === 'ltr') { focusSafely(ref.current); - scrollIntoViewport(ref.current, { containingElement: getScrollParent(ref.current) }); + scrollIntoViewport(ref.current, {containingElement: getScrollParent(ref.current)}); } else { walker.currentNode = ref.current; focusable = direction === 'rtl' ? last(walker) : (walker.firstChild() as FocusableElement); if (focusable) { focusSafely(focusable); - scrollIntoViewport(focusable, { containingElement: getScrollParent(ref.current) }); + scrollIntoViewport(focusable, {containingElement: getScrollParent(ref.current)}); } } } @@ -257,8 +257,12 @@ export function useGridCell>( let onFocus = (e: any) => { keyWhenFocused.current = node.key; - // FIX: If focus is moving directly to a specific inner child element - // (like our restored Open Dialog button), update the selection state + // FIX: If focus is moving directly to a specific inner child element + // (like our restored Open Dialog button), update the selection state + // and exit early. Do not call focus(), which resets to the first child. + + // FIX: If focus is moving directly to a specific inner child element + // (like our restored Open Dialog button), update the selection state // and exit early. Do not call focus(), which resets to the first child. if (getEventTarget(e) !== ref.current) { if (!isFocusVisible()) { @@ -268,7 +272,7 @@ export function useGridCell>( } if (focusMode === 'child') { - // Safeguard: Check if the browser's active element has already shifted + // Safeguard: Check if the browser's active element has already shifted // into a nested child node during this event loop tick before forcing a reset. let activeElement = getActiveElement(); if (ref.current && isFocusWithin(ref.current) && activeElement !== ref.current) { diff --git a/packages/react-aria/test/grid/useGrid.test.js b/packages/react-aria/test/grid/useGrid.test.js old mode 100644 new mode 100755 index 5668df18209..24086ec8168 --- a/packages/react-aria/test/grid/useGrid.test.js +++ b/packages/react-aria/test/grid/useGrid.test.js @@ -10,11 +10,11 @@ * governing permissions and limitations under the License. */ -import { act, pointerMap, render } from '@react-spectrum/test-utils-internal'; -import { Grid } from '../../stories/grid/example'; -import { Item } from 'react-stately/Item'; +import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; +import {Grid} from '../../stories/grid/example'; +import {Item} from 'react-stately/Item'; import React from 'react'; -import { Switch } from '@adobe/react-spectrum/Switch'; +import {Switch} from '@adobe/react-spectrum/Switch'; import userEvent from '@testing-library/user-event'; function renderGrid(props = {}) { @@ -41,7 +41,7 @@ describe('useGrid', () => { let user; beforeAll(() => { - user = userEvent.setup({ delay: null, pointerMap }); + user = userEvent.setup({delay: null, pointerMap}); jest.useFakeTimers(); }); afterEach(() => { @@ -51,7 +51,7 @@ describe('useGrid', () => { }); }); it('gridFocusMode = row, cellFocusMode = cell', async () => { - let tree = renderGrid({ gridFocusMode: 'row', cellFocusMode: 'cell' }); + let tree = renderGrid({gridFocusMode: 'row', cellFocusMode: 'cell'}); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('row')[0]); @@ -84,7 +84,7 @@ describe('useGrid', () => { }); it('gridFocusMode = row, cellFocusMode = child', async () => { - let tree = renderGrid({ gridFocusMode: 'row', cellFocusMode: 'child' }); + let tree = renderGrid({gridFocusMode: 'row', cellFocusMode: 'child'}); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('row')[0]); @@ -112,7 +112,7 @@ describe('useGrid', () => { }); it('gridFocusMode = cell, cellFocusMode = child', async () => { - let tree = renderGrid({ gridFocusMode: 'cell', cellFocusMode: 'child' }); + let tree = renderGrid({gridFocusMode: 'cell', cellFocusMode: 'child'}); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('switch')[0]); @@ -134,7 +134,7 @@ describe('useGrid', () => { }); it('gridFocusMode = cell, cellFocusMode = cell', async () => { - let tree = renderGrid({ gridFocusMode: 'cell', cellFocusMode: 'cell' }); + let tree = renderGrid({gridFocusMode: 'cell', cellFocusMode: 'cell'}); await user.tab(); expect(document.activeElement).toBe(tree.getAllByRole('gridcell')[0]); @@ -159,7 +159,7 @@ describe('useGrid', () => { }); it('should retain focus on a specific child element if focus is restored to it', async () => { - let tree = renderGrid({ gridFocusMode: 'cell', cellFocusMode: 'child' }); + let tree = renderGrid({gridFocusMode: 'cell', cellFocusMode: 'child'}); let switches = tree.getAllByRole('switch'); let cells = tree.getAllByRole('gridcell'); @@ -174,10 +174,10 @@ describe('useGrid', () => { }); expect(document.activeElement).toBe(switches[1]); - // 3. Fire a focus event directly on the gridcell container to trigger your + // 3. Fire a focus event directly on the gridcell container to trigger your // onFocus handler in useGridCell.ts and simulate event bubbling act(() => { - cells[0].dispatchEvent(new FocusEvent('focus', { bubbles: true })); + cells[0].dispatchEvent(new FocusEvent('focus', {bubbles: true})); }); // 4. Force Jest's timer and requestAnimationFrame microtask cycles to execute completely From 4b28c4ac23971d631ecba43e206bf39be614a32f Mon Sep 17 00:00:00 2001 From: jsmitrah Date: Fri, 19 Jun 2026 13:48:51 +0530 Subject: [PATCH 3/3] changed the type for focus event. --- packages/react-aria/src/grid/useGridCell.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-aria/src/grid/useGridCell.ts b/packages/react-aria/src/grid/useGridCell.ts index 4e2ddfe9bd6..25f562aa4ac 100644 --- a/packages/react-aria/src/grid/useGridCell.ts +++ b/packages/react-aria/src/grid/useGridCell.ts @@ -254,7 +254,7 @@ export function useGridCell>( // Grid cells can have focusable elements inside them. In this case, focus should // be marshalled to that element rather than focusing the cell itself. - let onFocus = (e: any) => { + let onFocus = (e: FocusEvent) => { keyWhenFocused.current = node.key; // FIX: If focus is moving directly to a specific inner child element