Skip to content

fix: Fix crash in RAC Table DnD keyboard navigation #8645

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
snowystinger merged 3 commits into from
Oct 22, 2025
Merged

fix: Fix crash in RAC Table DnD keyboard navigation #8645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f3c4ff
test
chirokas Jul 28, 2025
282d0d8
Fix crash on renderAfterDropIndicators in Table
chirokas Jul 28, 2025
2e69bba
Fix table dnd keyboard navigation
chirokas Jul 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions packages/@react-aria/dnd/src/DropTargetKeyboardNavigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,25 @@ function nextDropTarget(
}
case 'after': {
// If this is the last sibling in a level, traverse to the parent.
let targetNode = collection.getItem(target.key);
if (targetNode && targetNode.nextKey == null && targetNode.parentKey != null) {
let targetNode = collection.getItem(target.key);
let nextItemInSameLevel = targetNode?.nextKey != null ? collection.getItem(targetNode.nextKey) : null;
while (nextItemInSameLevel != null && nextItemInSameLevel.type !== 'item') {
nextItemInSameLevel = nextItemInSameLevel.nextKey != null ? collection.getItem(nextItemInSameLevel.nextKey) : null;
}

if (targetNode && nextItemInSameLevel == null && targetNode.parentKey != null) {
// If the parent item has an item after it, use the "before" position.
let parentNode = collection.getItem(targetNode.parentKey);
if (parentNode?.nextKey != null) {
const nextNode = parentNode?.nextKey != null ? collection.getItem(parentNode.nextKey) : null;
if (nextNode?.type === 'item') {
return {
type: 'item',
key: parentNode.nextKey,
key: nextNode.key,
dropPosition: 'before'
};
}

if (parentNode) {
if (parentNode?.type === 'item') {
return {
type: 'item',
key: parentNode.key,
Expand All @@ -121,10 +127,10 @@ function nextDropTarget(
}
}

if (targetNode?.nextKey != null) {
if (nextItemInSameLevel) {
return {
type: 'item',
key: targetNode.nextKey,
key: nextItemInSameLevel.key,
dropPosition: 'on'
};
}
Expand Down Expand Up @@ -154,8 +160,11 @@ function previousDropTarget(
let prevKey: Key | null = null;
let lastKey = keyboardDelegate.getLastKey?.();
while (lastKey != null) {
prevKey = lastKey;
let node = collection.getItem(lastKey);
if (node?.type !== 'item') {
break;
}
prevKey = lastKey;
lastKey = node?.parentKey;
}

Expand Down
43 changes: 43 additions & 0 deletions packages/@react-spectrum/table/test/TableDnd.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1967,6 +1967,49 @@ describe('TableView', function () {
});
});

it('support drop target keyboard navigation', async () => {
render(<ReorderExample />);
await user.tab();
await user.keyboard('{ArrowRight}');
await user.keyboard('{Enter}');
act(() => jest.runAllTimers());

const labels = ['Vin Charlet', 'Lexy Maddison', 'Robbi Persence', 'Dodie Hurworth', 'Audrye Hember', 'Beau Oller', 'Roarke Gration', 'Cathy Lishman', 'Enrika Soitoux', 'Aloise Tuxsell'];

for (let i = 0; i < labels.length; i++) {
if (i === labels.length - 1) {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert after ${labels[i]}`);
} else {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i]} and ${labels[i + 1]}`);
}

fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'});
fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'});
}

expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Vin Charlet');
await user.keyboard('{End}');
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Aloise Tuxsell');

for (let i = labels.length - 1; i >= 0; i--) {
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});

if (i === 0) {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`);
} else {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`);
}
}

await user.keyboard('{ArrowUp}');
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Aloise Tuxsell');
await user.keyboard('{Home}');
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Vin Charlet');
await user.keyboard('{Escape}');
act(() => jest.runAllTimers());
});

describe('using util handlers', function () {
async function beginDrag(tree) {
let grids = tree.getAllByRole('grid');
Expand Down
2 changes: 1 addition & 1 deletion packages/react-aria-components/src/Collection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export function renderAfterDropIndicators(collection: ICollection<Node<unknown>>
let afterIndicators: ReactNode[] = [];
if (nextItemInSameLevel == null) {
let current: Node<unknown> | null = node;
while (current && (!nextItemInFlattenedCollection || (current.parentKey !== nextItemInFlattenedCollection.parentKey && nextItemInFlattenedCollection.level < current.level))) {
while (current?.type === 'item' && (!nextItemInFlattenedCollection || (current.parentKey !== nextItemInFlattenedCollection.parentKey && nextItemInFlattenedCollection.level < current.level))) {
Copy link
Member

@LFDanLu LFDanLu Oct 22, 2025

Choose a reason for hiding this comment

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

We might need to tweak to for the eventual tree with sections case? This is fine for now, I'm not really sure if after drop positions would ever become available for sections

let indicator = renderDropIndicator({
type: 'item',
key: current.key,
Expand Down
7 changes: 6 additions & 1 deletion packages/react-aria-components/stories/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,14 @@
outline: 1px solid slateblue;
}


:global(.react-aria-Table) {
border-collapse: collapse;

&[data-drop-target] {
outline: 2px solid purple;
outline-offset: -2px;
background: rgb(from purple r g b / 20%);
}
}

:global(.react-aria-Cell) {
Expand Down
50 changes: 50 additions & 0 deletions packages/react-aria-components/test/Table.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,56 @@ describe('Table', () => {
expect(checkbox).toBeChecked();
}
});

it('support drop target keyboard navigation', async () => {
const DndTableExample = stories.DndTableExample;
render(<DndTableExample />);
await user.tab();
await user.keyboard('{ArrowRight}');
await user.keyboard('{Enter}');
act(() => jest.runAllTimers());
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Adobe Photoshop and Adobe XD');
await user.tab();
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');

const labels = ['Pictures', 'Adobe Fresco', 'Apps', 'Adobe Illustrator', 'Adobe Lightroom', 'Adobe Dreamweaver'];

for (let i = 0; i <= labels.length; i++) {
fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'});
fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'});

if (i === 0) {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`);
} else if (i === labels.length) {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert after ${labels[i - 1]}`);
} else {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`);
}
}

await user.keyboard('{Home}');
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');

for (let i = labels.length; i >= 0; i--) {
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});

if (i === 0) {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`);
} else if (i === labels.length) {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert after ${labels[i - 1]}`);
} else {
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`);
}
}

await user.keyboard('{End}');
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Adobe Dreamweaver');
await user.keyboard('{ArrowDown}');
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
await user.keyboard('{Escape}');
act(() => jest.runAllTimers());
});
});

describe('column resizing', () => {
Expand Down