Skip to content

Commit 5f8ddf3

Browse files
LFDanLuLFDanLu
authored
fix: focus newly added table row on drop (adobe#9873)
* fix: focus newly added table row on drop * fix test * fix 16/17 tests * actually just need to remove the acts? * fix focus issues when dropping on a collapsed/expanded tree item and select all child rows on drop * lint * fix tests so the original bug is caught when using old useDroppableCollection
1 parent 6726b1e commit 5f8ddf3

File tree

3 files changed

+165
-4
lines changed

3 files changed

+165
-4
lines changed

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,30 @@ describe('Table', () => {
15151515
await user.keyboard('{Escape}');
15161516
act(() => jest.runAllTimers());
15171517
});
1518+
1519+
it('should select dropped item', async () => {
1520+
const DndTableExample = stories.DndTableExample;
1521+
let {getAllByRole} = render(<DndTableExample />);
1522+
let tableTester = testUtilUser.createTester('Table', {root: getAllByRole('grid')[1]});
1523+
expect(tableTester.rows).toHaveLength(7);
1524+
expect(tableTester.selectedRows).toHaveLength(0);
1525+
await user.tab();
1526+
await user.keyboard('{ArrowRight}');
1527+
await user.keyboard('{Enter}');
1528+
act(() => jest.runAllTimers());
1529+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Adobe Photoshop and Adobe XD');
1530+
await user.tab();
1531+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
1532+
await user.keyboard('{ArrowDown}');
1533+
expect(document.activeElement).toHaveAttribute('aria-label', 'Insert before Pictures');
1534+
fireEvent.keyDown(document.activeElement, {key: 'Enter'});
1535+
fireEvent.keyUp(document.activeElement, {key: 'Enter'});
1536+
// run onInsert promise in DnDTableExample first, otherwise updateFocusAfterDrop doesn't run properly
1537+
await act(async () => {});
1538+
act(() => jest.runAllTimers());
1539+
expect(tableTester.rows).toHaveLength(8);
1540+
expect(tableTester.selectedRows).toHaveLength(1);
1541+
});
15181542
});
15191543

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

‎packages/react-aria-components/test/Tree.test.tsx‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,6 +2075,134 @@ describe('Tree', () => {
20752075
expect(getItems).toHaveBeenCalledTimes(1);
20762076
expect(getItems).toHaveBeenCalledWith(new Set(['projects', 'reports']));
20772077
});
2078+
2079+
it('should select the parent and all its children when dropped', async () => {
2080+
let {getAllByRole} = render(<TreeWithDragAndDrop selectionMode="multiple" />);
2081+
let trees = getAllByRole('treegrid');
2082+
2083+
let firstTreeTester = testUtilUser.createTester('Tree', {root: trees[0]});
2084+
let secondTreeTester = testUtilUser.createTester('Tree', {root: trees[1]});
2085+
expect(firstTreeTester.rows).toHaveLength(2);
2086+
// has the empty state row
2087+
expect(secondTreeTester.rows).toHaveLength(1);
2088+
await user.tab();
2089+
// selects and drops first row onto second tree
2090+
await user.keyboard('{ArrowRight}');
2091+
await user.keyboard('{ArrowRight}');
2092+
await user.keyboard('{ArrowRight}');
2093+
await user.keyboard('{Enter}');
2094+
act(() => jest.runAllTimers());
2095+
await user.tab();
2096+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
2097+
await act(async () => {
2098+
fireEvent.keyDown(document.activeElement as Element, {key: 'Enter'});
2099+
fireEvent.keyUp(document.activeElement as Element, {key: 'Enter'});
2100+
});
2101+
act(() => jest.runAllTimers());
2102+
expect(secondTreeTester.rows).toHaveLength(1);
2103+
// expands tree row children
2104+
await user.keyboard('{ArrowRight}');
2105+
await user.keyboard('{ArrowDown}');
2106+
await user.keyboard('{ArrowDown}');
2107+
await user.keyboard('{ArrowRight}');
2108+
expect(secondTreeTester.selectedRows).toHaveLength(9);
2109+
});
2110+
2111+
it('should focus the parent row when dropped on if it isnt expanded', async () => {
2112+
let {getAllByRole} = render(<TreeWithDragAndDrop />);
2113+
let trees = getAllByRole('treegrid');
2114+
2115+
let firstTreeTester = testUtilUser.createTester('Tree', {root: trees[0]});
2116+
let secondTreeTester = testUtilUser.createTester('Tree', {root: trees[1]});
2117+
expect(firstTreeTester.rows).toHaveLength(2);
2118+
// has the empty state row
2119+
expect(secondTreeTester.rows).toHaveLength(1);
2120+
await user.tab();
2121+
// selects and drops first row onto second tree
2122+
await user.keyboard('{ArrowRight}');
2123+
await user.keyboard('{ArrowRight}');
2124+
await user.keyboard('{Enter}');
2125+
act(() => jest.runAllTimers());
2126+
await user.tab();
2127+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
2128+
await act(async () => {
2129+
fireEvent.keyDown(document.activeElement as Element, {key: 'Enter'});
2130+
fireEvent.keyUp(document.activeElement as Element, {key: 'Enter'});
2131+
});
2132+
act(() => jest.runAllTimers());
2133+
expect(secondTreeTester.rows).toHaveLength(1);
2134+
await user.keyboard('{ArrowRight}');
2135+
expect(secondTreeTester.rows).toHaveLength(6);
2136+
// tab back to the first tree and drop a new row onto one of the 2nd tree's child rows as it is expanded
2137+
await user.tab({shift: true});
2138+
expect(document.activeElement).toBe(firstTreeTester.rows[0]);
2139+
await user.keyboard('{ArrowRight}');
2140+
await user.keyboard('{Enter}');
2141+
act(() => jest.runAllTimers());
2142+
await user.tab();
2143+
for (let i = 0; i < 7; i++) {
2144+
await user.keyboard('{ArrowDown}');
2145+
}
2146+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Project 2');
2147+
await act(async () => {
2148+
fireEvent.keyDown(document.activeElement as Element, {key: 'Enter'});
2149+
fireEvent.keyUp(document.activeElement as Element, {key: 'Enter'});
2150+
});
2151+
act(() => jest.runAllTimers());
2152+
expect(document.activeElement).toBe(secondTreeTester.rows[2]);
2153+
});
2154+
2155+
it('should focus the dropped row when dropped on a parent that is expanded', async () => {
2156+
let {getAllByRole} = render(<TreeWithDragAndDrop />);
2157+
let trees = getAllByRole('treegrid');
2158+
2159+
let firstTreeTester = testUtilUser.createTester('Tree', {root: trees[0]});
2160+
let secondTreeTester = testUtilUser.createTester('Tree', {root: trees[1]});
2161+
expect(firstTreeTester.rows).toHaveLength(2);
2162+
// has the empty state row
2163+
expect(secondTreeTester.rows).toHaveLength(1);
2164+
await user.tab();
2165+
// selects and drops first row onto second tree
2166+
await user.keyboard('{ArrowRight}');
2167+
await user.keyboard('{ArrowRight}');
2168+
await user.keyboard('{Enter}');
2169+
act(() => jest.runAllTimers());
2170+
2171+
await user.tab();
2172+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on');
2173+
await act(async () => {
2174+
fireEvent.keyDown(document.activeElement as Element, {key: 'Enter'});
2175+
fireEvent.keyUp(document.activeElement as Element, {key: 'Enter'});
2176+
});
2177+
act(() => jest.runAllTimers());
2178+
expect(secondTreeTester.rows).toHaveLength(1);
2179+
// expands tree row children
2180+
await user.keyboard('{ArrowRight}');
2181+
await user.keyboard('{ArrowDown}');
2182+
await user.keyboard('{ArrowDown}');
2183+
await user.keyboard('{ArrowRight}');
2184+
expect(secondTreeTester.rows).toHaveLength(9);
2185+
// tab back to the first tree and drop a new row onto one of the 2nd tree's child rows as it is expanded
2186+
await user.tab({shift: true});
2187+
expect(document.activeElement).toBe(firstTreeTester.rows[0]);
2188+
await user.keyboard('{ArrowRight}');
2189+
await user.keyboard('{Enter}');
2190+
2191+
act(() => jest.runAllTimers());
2192+
await user.tab();
2193+
for (let i = 0; i < 14; i++) {
2194+
await user.keyboard('{ArrowDown}');
2195+
}
2196+
expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Project 2');
2197+
await act(async () => {
2198+
fireEvent.keyDown(document.activeElement as Element, {key: 'Enter'});
2199+
fireEvent.keyUp(document.activeElement as Element, {key: 'Enter'});
2200+
});
2201+
act(() => jest.runAllTimers());
2202+
expect(document.activeElement).toHaveTextContent('Projects');
2203+
expect(document.activeElement).toBe(secondTreeTester.rows[3]);
2204+
2205+
});
20782206
});
20792207

20802208
describe('press events', () => {

‎packages/react-aria/src/dnd/useDroppableCollection.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,18 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
253253
state.selectionManager.isSelectionEqual(prevSelectedKeys)
254254
) {
255255
let newKeys = new Set<Key>();
256-
for (let item of state.collection) {
257-
if (item.type === 'item' && !prevCollection.getItem(item.key)) {
256+
let key = state.collection.getFirstKey();
257+
while (key != null) {
258+
let item = state.collection.getItem(key);
259+
if (item?.type === 'item' && !prevCollection.getItem(item.key)) {
258260
newKeys.add(item.key);
259261
}
262+
263+
if (item?.hasChildNodes && state.collection.getItem(item.lastChildKey!)?.type === 'item') {
264+
key = item.firstChildKey!;
265+
} else {
266+
key = state.collection.getKeyAfter(key);
267+
}
260268
}
261269

262270
state.selectionManager.setSelectedKeys(newKeys);
@@ -268,10 +276,11 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
268276
let first: Key | null | undefined = newKeys.keys().next().value;
269277
if (first != null) {
270278
let item = state.collection.getItem(first);
271-
279+
let dropTarget = droppingState.current.target;
280+
let isParentRowExpanded = state.collection['expandedKeys'] ? state.collection['expandedKeys'].has(item?.parentKey) : false;
272281
// If this is a cell, focus the parent row.
273282
// eslint-disable-next-line max-depth
274-
if (item?.type === 'cell') {
283+
if (item && (item?.type === 'cell' || (dropTarget.type === 'item' && dropTarget.dropPosition === 'on' && !isParentRowExpanded))) {
275284
first = item.parentKey;
276285
}
277286

0 commit comments

Comments
 (0)