Skip to content

Commit 27edf8f

Browse files
chirokaschirokas
authored
fix: Fix crash in RAC Table DnD keyboard navigation (#8645)
* test * Fix crash on renderAfterDropIndicators in Table * Fix table dnd keyboard navigation
1 parent 54e3f2e commit 27edf8f

File tree

5 files changed

+117
-10
lines changed

5 files changed

+117
-10
lines changed

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,25 @@ function nextDropTarget(
100100
}
101101
case 'after': {
102102
// If this is the last sibling in a level, traverse to the parent.
103-
let targetNode = collection.getItem(target.key);
104-
if (targetNode && targetNode.nextKey == null && targetNode.parentKey != null) {
103+
let targetNode = collection.getItem(target.key);
104+
let nextItemInSameLevel = targetNode?.nextKey != null ? collection.getItem(targetNode.nextKey) : null;
105+
while (nextItemInSameLevel != null && nextItemInSameLevel.type !== 'item') {
106+
nextItemInSameLevel = nextItemInSameLevel.nextKey != null ? collection.getItem(nextItemInSameLevel.nextKey) : null;
107+
}
108+
109+
if (targetNode && nextItemInSameLevel == null && targetNode.parentKey != null) {
105110
// If the parent item has an item after it, use the "before" position.
106111
let parentNode = collection.getItem(targetNode.parentKey);
107-
if (parentNode?.nextKey != null) {
112+
const nextNode = parentNode?.nextKey != null ? collection.getItem(parentNode.nextKey) : null;
113+
if (nextNode?.type === 'item') {
108114
return {
109115
type: 'item',
110-
key: parentNode.nextKey,
116+
key: nextNode.key,
111117
dropPosition: 'before'
112118
};
113119
}
114120

115-
if (parentNode) {
121+
if (parentNode?.type === 'item') {
116122
return {
117123
type: 'item',
118124
key: parentNode.key,
@@ -121,10 +127,10 @@ function nextDropTarget(
121127
}
122128
}
123129

124-
if (targetNode?.nextKey != null) {
130+
if (nextItemInSameLevel) {
125131
return {
126132
type: 'item',
127-
key: targetNode.nextKey,
133+
key: nextItemInSameLevel.key,
128134
dropPosition: 'on'
129135
};
130136
}
@@ -154,8 +160,11 @@ function previousDropTarget(
154160
let prevKey: Key | null = null;
155161
let lastKey = keyboardDelegate.getLastKey?.();
156162
while (lastKey != null) {
157-
prevKey = lastKey;
158163
let node = collection.getItem(lastKey);
164+
if (node?.type !== 'item') {
165+
break;
166+
}
167+
prevKey = lastKey;
159168
lastKey = node?.parentKey;
160169
}
161170

‎packages/@react-spectrum/table/test/TableDnd.test.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,6 +1967,49 @@ describe('TableView', function () {
19671967
});
19681968
});
19691969

1970+
it('support drop target keyboard navigation', async () => {
1971+
render(<ReorderExample />);
1972+
await user.tab();
1973+
await user.keyboard('{ArrowRight}');
1974+
await user.keyboard('{Enter}');
1975+
act(() => jest.runAllTimers());
1976+
1977+
const labels = ['Vin Charlet', 'Lexy Maddison', 'Robbi Persence', 'Dodie Hurworth', 'Audrye Hember', 'Beau Oller', 'Roarke Gration', 'Cathy Lishman', 'Enrika Soitoux', 'Aloise Tuxsell'];
1978+
1979+
for (let i = 0; i < labels.length; i++) {
1980+
if (i === labels.length - 1) {
1981+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert after ${labels[i]}`);
1982+
} else {
1983+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i]} and ${labels[i + 1]}`);
1984+
}
1985+
1986+
fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'});
1987+
fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'});
1988+
}
1989+
1990+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Vin Charlet');
1991+
await user.keyboard('{End}');
1992+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Aloise Tuxsell');
1993+
1994+
for (let i = labels.length - 1; i >= 0; i--) {
1995+
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
1996+
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});
1997+
1998+
if (i === 0) {
1999+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`);
2000+
} else {
2001+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`);
2002+
}
2003+
}
2004+
2005+
await user.keyboard('{ArrowUp}');
2006+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Aloise Tuxsell');
2007+
await user.keyboard('{Home}');
2008+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Vin Charlet');
2009+
await user.keyboard('{Escape}');
2010+
act(() => jest.runAllTimers());
2011+
});
2012+
19702013
describe('using util handlers', function () {
19712014
async function beginDrag(tree) {
19722015
let grids = tree.getAllByRole('grid');

‎packages/react-aria-components/src/Collection.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ export function renderAfterDropIndicators(collection: ICollection<Node<unknown>>
195195
let afterIndicators: ReactNode[] = [];
196196
if (nextItemInSameLevel == null) {
197197
let current: Node<unknown> | null = node;
198-
while (current && (!nextItemInFlattenedCollection || (current.parentKey !== nextItemInFlattenedCollection.parentKey && nextItemInFlattenedCollection.level < current.level))) {
198+
while (current?.type === 'item' && (!nextItemInFlattenedCollection || (current.parentKey !== nextItemInFlattenedCollection.parentKey && nextItemInFlattenedCollection.level < current.level))) {
199199
let indicator = renderDropIndicator({
200200
type: 'item',
201201
key: current.key,

‎packages/react-aria-components/stories/styles.css‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,14 @@
205205
outline: 1px solid slateblue;
206206
}
207207

208-
209208
:global(.react-aria-Table) {
210209
border-collapse: collapse;
210+
211+
&[data-drop-target] {
212+
outline: 2px solid purple;
213+
outline-offset: -2px;
214+
background: rgb(from purple r g b / 20%);
215+
}
211216
}
212217

213218
:global(.react-aria-Cell) {

‎packages/react-aria-components/test/Table.test.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,6 +1310,56 @@ describe('Table', () => {
13101310
expect(checkbox).toBeChecked();
13111311
}
13121312
});
1313+
1314+
it('support drop target keyboard navigation', async () => {
1315+
const DndTableExample = stories.DndTableExample;
1316+
render(<DndTableExample />);
1317+
await user.tab();
1318+
await user.keyboard('{ArrowRight}');
1319+
await user.keyboard('{Enter}');
1320+
act(() => jest.runAllTimers());
1321+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Adobe Photoshop and Adobe XD');
1322+
await user.tab();
1323+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
1324+
1325+
const labels = ['Pictures', 'Adobe Fresco', 'Apps', 'Adobe Illustrator', 'Adobe Lightroom', 'Adobe Dreamweaver'];
1326+
1327+
for (let i = 0; i <= labels.length; i++) {
1328+
fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'});
1329+
fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'});
1330+
1331+
if (i === 0) {
1332+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`);
1333+
} else if (i === labels.length) {
1334+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert after ${labels[i - 1]}`);
1335+
} else {
1336+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`);
1337+
}
1338+
}
1339+
1340+
await user.keyboard('{Home}');
1341+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
1342+
1343+
for (let i = labels.length; i >= 0; i--) {
1344+
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
1345+
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});
1346+
1347+
if (i === 0) {
1348+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`);
1349+
} else if (i === labels.length) {
1350+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert after ${labels[i - 1]}`);
1351+
} else {
1352+
expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`);
1353+
}
1354+
}
1355+
1356+
await user.keyboard('{End}');
1357+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Adobe Dreamweaver');
1358+
await user.keyboard('{ArrowDown}');
1359+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
1360+
await user.keyboard('{Escape}');
1361+
act(() => jest.runAllTimers());
1362+
});
13131363
});
13141364

13151365
describe('column resizing', () => {

0 commit comments

Comments
 (0)