Skip to content

Commit bf87614

Browse files
authored
fix: Fix keyboard DnD navigation in expandable rows RAC Table (#9773)
* fix: expandable table rows keyboard navigation during DnD this is specifcally handling the case where the user navigates upwards through nested row drop targets * update drop target keyboard navigation logic to be more resilient to new collections instead * fix lint * review comments * copy pasta
1 parent 4a2eea0 commit bf87614

File tree

3 files changed

+159
-84
lines changed

3 files changed

+159
-84
lines changed

packages/@react-aria/dnd/src/DropTargetKeyboardNavigation.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {Collection, DropTarget, Key, KeyboardDelegate, Node} from '@react-types/shared';
2-
import {getChildNodes} from '@react-stately/collections';
32

43
export function navigate(
54
keyboardDelegate: KeyboardDelegate,
@@ -11,11 +10,11 @@ export function navigate(
1110
): DropTarget | null {
1211
switch (direction) {
1312
case 'left':
14-
return rtl
13+
return rtl
1514
? nextDropTarget(keyboardDelegate, collection, target, wrap, 'left')
1615
: previousDropTarget(keyboardDelegate, collection, target, wrap, 'left');
1716
case 'right':
18-
return rtl
17+
return rtl
1918
? previousDropTarget(keyboardDelegate, collection, target, wrap, 'right')
2019
: nextDropTarget(keyboardDelegate, collection, target, wrap, 'right');
2120
case 'up':
@@ -70,7 +69,7 @@ function nextDropTarget(
7069
dropPosition: target.dropPosition
7170
};
7271
}
73-
72+
7473
switch (target.dropPosition) {
7574
case 'before': {
7675
return {
@@ -105,7 +104,7 @@ function nextDropTarget(
105104
while (nextItemInSameLevel != null && nextItemInSameLevel.type !== 'item') {
106105
nextItemInSameLevel = nextItemInSameLevel.nextKey != null ? collection.getItem(nextItemInSameLevel.nextKey) : null;
107106
}
108-
107+
109108
if (targetNode && nextItemInSameLevel == null && targetNode.parentKey != null) {
110109
// If the parent item has an item after it, use the "before" position.
111110
let parentNode = collection.getItem(targetNode.parentKey);
@@ -261,12 +260,14 @@ function getLastChild(collection: Collection<Node<unknown>>, key: Key): DropTarg
261260
let nextKey = getNextItem(collection, key, key => collection.getKeyAfter(key));
262261
let nextNode = nextKey != null ? collection.getItem(nextKey) : null;
263262
if (targetNode && nextNode && nextNode.level > targetNode.level) {
264-
let children = getChildNodes(targetNode, collection);
265263
let lastChild: Node<unknown> | null = null;
266-
for (let child of children) {
267-
if (child.type === 'item') {
268-
lastChild = child;
264+
if ('lastChildKey' in targetNode) {
265+
lastChild = targetNode.lastChildKey != null ? collection.getItem(targetNode.lastChildKey) : null;
266+
while (lastChild && lastChild.type !== 'item' && lastChild.prevKey != null) {
267+
lastChild = collection.getItem(lastChild.prevKey)!;
269268
}
269+
} else {
270+
lastChild = Array.from(targetNode.childNodes).findLast(item => item.type === 'item') || null;
270271
}
271272

272273
if (lastChild) {

packages/@react-aria/dnd/test/DropTargetKeyboardNavigation.test.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ let rows: Item[] = [
3939
]}
4040
];
4141

42+
function toArray(iterableCollection: Iterable<Node<Item>>, predicate) {
43+
const result: Node<Item>[] = [];
44+
for (const o of iterableCollection) {
45+
if (predicate(o)) {
46+
result.push(o);
47+
}
48+
}
49+
return result;
50+
}
4251
// Collection implementation backed by item objects above.
4352
// This way we don't need to render React components to test.
4453
class TestCollection implements Collection<Node<Item>> {
@@ -186,7 +195,7 @@ class TestCollection implements Collection<Node<Item>> {
186195

187196
getChildren(key: Key): Iterable<Node<Item>> {
188197
let item = this.map.get(key);
189-
return item?.childNodes ?? [];
198+
return toArray(item?.childNodes || [], (node) => node.type !== 'item');
190199
}
191200
}
192201

@@ -294,7 +303,7 @@ describe('drop target keyboard navigation', () => {
294303
'reports-2',
295304
'reports-2-content'
296305
];
297-
306+
298307
expect(nextKeys).toEqual(expectedKeys);
299308

300309
let prevKeys: Key[] = [];

packages/react-aria-components/test/Treeble.test.js

Lines changed: 138 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export function Cell(props) {
2020
return (
2121
<AriaCell {...props}>
2222
{composeRenderProps(props.children, (children, {hasChildItems, isTreeColumn}) => (<>
23-
{isTreeColumn && hasChildItems &&
23+
{isTreeColumn && hasChildItems &&
2424
<Button slot="chevron">&gt;</Button>
2525
}
2626
{children}
@@ -93,6 +93,76 @@ function Example(props) {
9393
);
9494
}
9595

96+
function ReorderableTreeble(props) {
97+
let tree = useTreeData({
98+
initialItems: [
99+
{id: '1', title: 'Documents', type: 'Directory', date: '10/20/2025', children: [
100+
{id: '2', title: 'Project', type: 'Directory', date: '8/2/2025', children: [
101+
{id: '3', title: 'Weekly Report', type: 'File', date: '7/10/2025', children: []},
102+
{id: '4', title: 'Budget', type: 'File', date: '8/20/2025', children: []}
103+
]}
104+
]},
105+
{id: '5', title: 'Photos', type: 'Directory', date: '2/3/2026', children: [
106+
{id: '6', title: 'Image 1', type: 'File', date: '1/23/2026', children: []},
107+
{id: '7', title: 'Image 2', type: 'File', date: '2/3/2026', children: []}
108+
]}
109+
]
110+
});
111+
112+
let {dragAndDropHooks} = useDragAndDrop({
113+
getItems: (keys, items) => items.map(item => ({'text/plain': item.value.title})),
114+
onMove(e) {
115+
if (e.target.dropPosition === 'before') {
116+
tree.moveBefore(e.target.key, e.keys);
117+
} else if (e.target.dropPosition === 'after') {
118+
tree.moveAfter(e.target.key, e.keys);
119+
} else if (e.target.dropPosition === 'on') {
120+
// Move items to become children of the target
121+
let targetNode = tree.getItem(e.target.key);
122+
if (targetNode) {
123+
let targetIndex = targetNode.children ? targetNode.children.length : 0;
124+
let keyArray = Array.from(e.keys);
125+
for (let i = 0; i < keyArray.length; i++) {
126+
tree.move(keyArray[i], e.target.key, targetIndex + i);
127+
}
128+
}
129+
}
130+
}
131+
});
132+
133+
return (
134+
<Table
135+
aria-label="Files"
136+
selectionMode="multiple"
137+
treeColumn="name"
138+
defaultExpandedKeys={['5']}
139+
dragAndDropHooks={dragAndDropHooks}
140+
{...props}>
141+
<TableHeader>
142+
<Column />
143+
<Column id="name" isRowHeader>Name</Column>
144+
<Column id="type">Type</Column>
145+
<Column id="date">Date Modified</Column>
146+
</TableHeader>
147+
<TableBody items={tree.items}>
148+
{function renderItem(item) {
149+
return (
150+
<Row id={item.key} textValue={item.value.title}>
151+
<Cell><Button slot="drag" /></Cell>
152+
<Cell>{item.value.title}</Cell>
153+
<Cell>{item.value.type}</Cell>
154+
<Cell>{item.value.date}</Cell>
155+
{item.children && <Collection items={item.children}>
156+
{renderItem}
157+
</Collection>}
158+
</Row>
159+
);
160+
}}
161+
</TableBody>
162+
</Table>
163+
);
164+
}
165+
96166
describe('Treeble', () => {
97167
let utils = new User();
98168
let user;
@@ -105,7 +175,7 @@ describe('Treeble', () => {
105175
it('renders a treegrid', () => {
106176
let tree = render(<Example />);
107177
let tester = utils.createTester('Table', {root: tree.getByTestId('treeble')});
108-
178+
109179
expect(tester.table).toHaveAttribute('role', 'treegrid');
110180

111181
expect(tester.rows).toHaveLength(4);
@@ -413,7 +483,7 @@ describe('Treeble', () => {
413483
await user.tab();
414484
expect(document.activeElement).toBe(tester.rows[0]);
415485
expect(tester.rows[0]).toHaveAttribute('aria-expanded', 'false');
416-
486+
417487
await user.keyboard('{ArrowRight}');
418488
expect(document.activeElement).toBe(tester.rows[0]);
419489
expect(tester.rows[0]).toHaveAttribute('aria-expanded', 'true');
@@ -455,75 +525,6 @@ describe('Treeble', () => {
455525
});
456526

457527
it('should support drag and drop', async () => {
458-
function ReorderableTreeble() {
459-
let tree = useTreeData({
460-
initialItems: [
461-
{id: '1', title: 'Documents', type: 'Directory', date: '10/20/2025', children: [
462-
{id: '2', title: 'Project', type: 'Directory', date: '8/2/2025', children: [
463-
{id: '3', title: 'Weekly Report', type: 'File', date: '7/10/2025', children: []},
464-
{id: '4', title: 'Budget', type: 'File', date: '8/20/2025', children: []}
465-
]}
466-
]},
467-
{id: '5', title: 'Photos', type: 'Directory', date: '2/3/2026', children: [
468-
{id: '6', title: 'Image 1', type: 'File', date: '1/23/2026', children: []},
469-
{id: '7', title: 'Image 2', type: 'File', date: '2/3/2026', children: []}
470-
]}
471-
]
472-
});
473-
474-
let {dragAndDropHooks} = useDragAndDrop({
475-
getItems: (keys, items) => items.map(item => ({'text/plain': item.value.title})),
476-
onMove(e) {
477-
if (e.target.dropPosition === 'before') {
478-
tree.moveBefore(e.target.key, e.keys);
479-
} else if (e.target.dropPosition === 'after') {
480-
tree.moveAfter(e.target.key, e.keys);
481-
} else if (e.target.dropPosition === 'on') {
482-
// Move items to become children of the target
483-
let targetNode = tree.getItem(e.target.key);
484-
if (targetNode) {
485-
let targetIndex = targetNode.children ? targetNode.children.length : 0;
486-
let keyArray = Array.from(e.keys);
487-
for (let i = 0; i < keyArray.length; i++) {
488-
tree.move(keyArray[i], e.target.key, targetIndex + i);
489-
}
490-
}
491-
}
492-
}
493-
});
494-
495-
return (
496-
<Table
497-
aria-label="Files"
498-
selectionMode="multiple"
499-
treeColumn="name"
500-
defaultExpandedKeys={['5']}
501-
dragAndDropHooks={dragAndDropHooks}>
502-
<TableHeader>
503-
<Column />
504-
<Column id="name" isRowHeader>Name</Column>
505-
<Column id="type">Type</Column>
506-
<Column id="date">Date Modified</Column>
507-
</TableHeader>
508-
<TableBody items={tree.items}>
509-
{function renderItem(item) {
510-
return (
511-
<Row id={item.key} textValue={item.value.title}>
512-
<Cell><Button slot="drag" /></Cell>
513-
<Cell>{item.value.title}</Cell>
514-
<Cell>{item.value.type}</Cell>
515-
<Cell>{item.value.date}</Cell>
516-
{item.children && <Collection items={item.children}>
517-
{renderItem}
518-
</Collection>}
519-
</Row>
520-
);
521-
}}
522-
</TableBody>
523-
</Table>
524-
);
525-
}
526-
527528
let tree = render(<ReorderableTreeble />);
528529
let tester = utils.createTester('Table', {root: tree.getByRole('treegrid')});
529530

@@ -572,7 +573,7 @@ describe('Treeble', () => {
572573
'Insert after Image 2',
573574
'Insert after Photos'
574575
]);
575-
576+
576577
await user.keyboard('{Enter}');
577578
act(() => jest.runAllTimers());
578579

@@ -584,4 +585,68 @@ describe('Treeble', () => {
584585
'Image 1'
585586
]);
586587
});
588+
589+
it('should properly walk through nested levels of drop positioning', async () => {
590+
render(<ReorderableTreeble defaultExpandedKeys={['1', '2']} />);
591+
await user.tab();
592+
await user.keyboard('{ArrowDown}');
593+
await user.keyboard('{ArrowDown}');
594+
await user.keyboard('{ArrowDown}');
595+
await user.keyboard('{ArrowRight}');
596+
await user.keyboard('{Enter}');
597+
act(() => jest.runAllTimers());
598+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Budget');
599+
600+
await user.keyboard('{ArrowDown}');
601+
act(() => jest.runAllTimers());
602+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Project');
603+
604+
await user.keyboard('{ArrowDown}');
605+
act(() => jest.runAllTimers());
606+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Documents and Photos');
607+
608+
await user.keyboard('{ArrowDown}');
609+
act(() => jest.runAllTimers());
610+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Photos');
611+
612+
await user.keyboard('{ArrowUp}');
613+
act(() => jest.runAllTimers());
614+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Documents and Photos');
615+
616+
await user.keyboard('{ArrowUp}');
617+
act(() => jest.runAllTimers());
618+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Project');
619+
620+
await user.keyboard('{ArrowUp}');
621+
act(() => jest.runAllTimers());
622+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Budget');
623+
624+
await user.keyboard('{ArrowUp}');
625+
act(() => jest.runAllTimers());
626+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Weekly Report and Budget');
627+
628+
await user.keyboard('{ArrowUp}');
629+
act(() => jest.runAllTimers());
630+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Weekly Report');
631+
632+
await user.keyboard('{ArrowUp}');
633+
act(() => jest.runAllTimers());
634+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Weekly Report');
635+
636+
await user.keyboard('{ArrowUp}');
637+
act(() => jest.runAllTimers());
638+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Project');
639+
640+
await user.keyboard('{ArrowUp}');
641+
act(() => jest.runAllTimers());
642+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Project');
643+
644+
await user.keyboard('{ArrowUp}');
645+
act(() => jest.runAllTimers());
646+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Documents');
647+
648+
await user.keyboard('{ArrowUp}');
649+
act(() => jest.runAllTimers());
650+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Documents');
651+
});
587652
});

0 commit comments

Comments
 (0)