Skip to content

feat(RAC): Tree drag and drop #7692

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.

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
merged 48 commits into from
Apr 21, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
903ba60
[5574] - add moveBefore and moveAfter to useTreeData
rob-clayburn Jan 30, 2025
8f14593
add docs
rob-clayburn Jan 30, 2025
768e253
remove onlys
rob-clayburn Jan 30, 2025
aa2d362
remove console logs
rob-clayburn Jan 30, 2025
d9b4d57
[wip] add intial tree drag and drop
rob-clayburn Jan 30, 2025
5112433
Merge branch 'main' into tree-dnd
rob-clayburn Jan 30, 2025
e6becc6
Merge branch 'main' into tree-dnd
rob-clayburn Jan 31, 2025
22b88d5
useTreeData - add getDescendantKeys method which is used to determine…
rob-clayburn Jan 31, 2025
121712a
revert packlog json change
rob-clayburn Jan 31, 2025
7c81ea2
Merge remote-tracking branch 'upstream/main' into tree-dnd
rob-clayburn Feb 19, 2025
0e3554a
feat: Add moveBefore and moveAfter to useTreeData
snowystinger Mar 3, 2025
02e07de
updates + story
reidbarber Mar 11, 2025
5be35b6
Merge remote-tracking branch 'origin/main' into tree-dnd
reidbarber Mar 11, 2025
d7506e9
remove RSP TreeView dnd for now
reidbarber Mar 12, 2025
34073ba
cleanup
reidbarber Mar 12, 2025
efd5259
fix story
reidbarber Mar 12, 2025
5b38175
lint
reidbarber Mar 12, 2025
e0731ed
lint
reidbarber Mar 12, 2025
f7272d9
Merge remote-tracking branch 'origin/main' into tree-dnd
reidbarber Mar 12, 2025
2cbccd9
lint
reidbarber Mar 12, 2025
8ba1b42
ts
reidbarber Mar 12, 2025
0504991
fix listMapData destructure
reidbarber Mar 12, 2025
2383104
Merge branch 'main' into tree-data-move-before-and-after
snowystinger Mar 18, 2025
a0aa836
allow expanding during dragging
reidbarber Mar 19, 2025
8be4765
lint
reidbarber Mar 19, 2025
5e54c62
review comments
reidbarber Mar 19, 2025
b9d0746
Merge remote-tracking branch 'upstream/main' into tree-dnd
reidbarber Mar 19, 2025
074a3f6
fixes
reidbarber Mar 19, 2025
bb82173
lint
reidbarber Mar 19, 2025
cef6372
Merge remote-tracking branch 'upstream/main' into tree-dnd
reidbarber Apr 9, 2025
ce5d5a8
Merge remote-tracking branch 'upstream/tree-data-move-before-and-afte…
reidbarber Apr 9, 2025
117446c
update story to use useTreeData
reidbarber Apr 9, 2025
20311a9
handle dropPosition === 'on' case
reidbarber Apr 14, 2025
e01241a
set shouldSelectOnPressUp on item if dragging enabled
reidbarber Apr 14, 2025
896d405
update useTreeData move/moveBefore/moveAfter implementations
reidbarber Apr 15, 2025
8066a96
fix stories
reidbarber Apr 15, 2025
36516f8
typescript
reidbarber Apr 15, 2025
92f3b20
add drag button
reidbarber Apr 16, 2025
7dc4026
fix nested item drop button in story
reidbarber Apr 16, 2025
9237749
prevent dropping items onto themselves or their descendants
reidbarber Apr 16, 2025
163815e
Merge remote-tracking branch 'upstream/main' into tree-dnd
reidbarber Apr 16, 2025
33f12e2
Merge branch 'main' into tree-dnd
snowystinger Apr 17, 2025
7ff84d5
Merge branch 'main' into tree-data-move-before-and-after
snowystinger Apr 17, 2025
42c54f7
add tests and fix broken ones
snowystinger Apr 17, 2025
b01e3fe
remove dead code
snowystinger Apr 17, 2025
1d99e8c
fix lost items when moving to root
snowystinger Apr 17, 2025
315cff8
Merge remote-tracking branch 'upstream/tree-data-move-before-and-afte…
reidbarber Apr 17, 2025
88a4627
fix tests
reidbarber Apr 18, 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
3 changes: 2 additions & 1 deletion packages/@react-aria/gridlist/src/useGridListItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ export function useGridListItem<T>(props: AriaGridListItemOptions, state: ListSt

// let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/gridlist');
let {direction} = useLocale();
let {onAction, linkBehavior, keyboardNavigationBehavior} = listMap.get(state)!;
const listMapData = listMap.get(state);
let {onAction, linkBehavior = 'action', keyboardNavigationBehavior = 'arrow'} = listMapData || {};
let descriptionId = useSlotId();

// We need to track the key of the item at the time it was last focused so that we force
Expand Down
20 changes: 20 additions & 0 deletions packages/@react-stately/data/src/useTreeData.ts
Copy link
Member

Choose a reason for hiding this comment

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

I'll have to track down what exactly is happening but I noticed the following bug:

  1. Using http://localhost:9003/?path=/story/react-aria-components--tree-with-drag-and-drop&args=selectionMode:multiple&providerSwitcher-express=false&strict=true, expand both top level folders and and drag "Project 1" above "Projects"
  2. Drag "Project 1" back somewhere under "Projects" again (e.g. between "Project 3" and "Project 4")
    Note that "Project 1" disappears at this point

Copy link
Member

Choose a reason for hiding this comment

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

Another odd case that I discovered:

  1. Drag "Reports 1" into "Projects 2"
  2. Drag "Project 2" down below "Project 3"
  3. Drag "Project 2" down into "Reports" (Aka between "Reports" and "Reports 1" so that it becomes a child of "Reports"

Project 2 actually becomes a root level folder after this

Copy link
Member

Choose a reason for hiding this comment

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

It might be that the drop operation thinks the drop position is "After Reports" which technically isn't wrong but it should really be "Before Reports 1" or have a greater degree of granularity (aka "between Reports and Reports 1"). This actually works with keyboard DnD (and has the proper "insert between Reports and Reports 1" drop target) but for some reason is targeting the wrong drop insertion target I think for mouse DnD

Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export interface TreeData<T extends object> {
*/
getItem(key: Key): TreeNode<T> | undefined,

getDescendantKeys(node?: TreeNode<T>): Key[],
/**
* Inserts an item into a parent node as a child.
* @param parentKey - The key of the parent item to insert into. `null` for the root.
Expand Down Expand Up @@ -234,10 +235,29 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
}
}

function getDescendantKeys(node?: TreeNode<T>): Key[] {
Copy link
Member

Choose a reason for hiding this comment

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

any reason to have node be optional? can we make it required?

let descendantKeys: Key[] = [];
if (!node) {
return descendantKeys;
}
function recurse(currentNode: TreeNode<T>) {
Copy link
Member

Choose a reason for hiding this comment

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

does the traversal order matter? preorder, in order, postorder

should descendantKeys be a Set if it doesn't matter?

if (currentNode.children) {
for (let child of currentNode.children) {
descendantKeys.push(child.key);
recurse(child);
}
}
}

recurse(node);
return descendantKeys;
}

return {
items,
selectedKeys,
setSelectedKeys,
getDescendantKeys,
getItem(key: Key) {
return nodeMap.get(key);
},
Expand Down
59 changes: 58 additions & 1 deletion packages/@react-stately/data/test/useTreeData.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
*/

import {actHook as act, renderHook} from '@react-spectrum/test-utils-internal';
import React from 'react';
import {useTreeData} from '../src/useTreeData';

const initial = [
Expand Down Expand Up @@ -745,4 +744,62 @@ describe('useTreeData', function () {
expect(result.current.items[1].key).toEqual('Emily');
expect(result.current.items.length).toEqual(2);
});

it('gets the decentants of a node', function () {
const initialItems = [...initial, {name: 'Emily'}, {name: 'Eli'}];
let {result} = renderHook(() =>
useTreeData({initialItems, getChildren, getKey})
);
let decendants;
act(() => {
const top = result.current.getItem('David');
decendants = result.current.getDescendantKeys(top);
});
expect(decendants).toEqual([
'John',
'Suzie',
'Sam',
'Stacy',
'Brad',
'Jane'
]);
});

it('gets the decentants of a child node', function () {
const initialItems = [...initial, {name: 'Emily'}, {name: 'Eli'}];
let {result} = renderHook(() =>
useTreeData({initialItems, getChildren, getKey})
);
let descendants;
act(() => {
const top = result.current.getItem('Sam');
descendants = result.current.getDescendantKeys(top);
});
expect(descendants).toEqual(['Stacy', 'Brad']);
});

it('returns an empty array when getting the decendant keys for a leaf node', function () {
const initialItems = [...initial, {name: 'Emily'}, {name: 'Eli'}];
let {result} = renderHook(() =>
useTreeData({initialItems, getChildren, getKey})
);
let descendants;
act(() => {
const top = result.current.getItem('Eli');
descendants = result.current.getDescendantKeys(top);
});
expect(descendants).toEqual([]);
});

it('returns an empty array when an undefined key is supplied', function () {
const initialItems = [...initial, {name: 'Emily'}, {name: 'Eli'}];
let {result} = renderHook(() =>
useTreeData({initialItems, getChildren, getKey})
);
let descendants;
act(() => {
descendants = result.current.getDescendantKeys(undefined);
});
expect(descendants).toEqual([]);
});
});
Loading