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 all 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
1 change: 0 additions & 1 deletion packages/@react-aria/dnd/src/useDrop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export interface DropOptions {
/**
* Handler that is called after a valid drag is held over the drop target for a period of time.
* This typically opens the item so that the user can drop within it.
* @private
Copy link
Member

Choose a reason for hiding this comment

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

Did we find a way to make this accessible on keyboard/screen reader? That's why it was private before.

*/
onDropActivate?: (e: DropActivateEvent) => void,
/** Handler that is called when a valid drag exits the drop target. */
Expand Down
7 changes: 5 additions & 2 deletions packages/@react-aria/dnd/src/useDroppableCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
DropTargetDelegate,
Key,
KeyboardDelegate,
KeyboardEvents,
Node,
RefObject
} from '@react-types/shared';
Expand All @@ -46,7 +47,8 @@ export interface DroppableCollectionOptions extends DroppableCollectionProps {
/** A delegate object that implements behavior for keyboard focus movement. */
keyboardDelegate: KeyboardDelegate,
/** A delegate object that provides drop targets for pointer coordinates within the collection. */
dropTargetDelegate: DropTargetDelegate
dropTargetDelegate: DropTargetDelegate,
onKeyDown?: KeyboardEvents['onKeyDown']
}

export interface DroppableCollectionResult {
Expand Down Expand Up @@ -201,7 +203,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
autoScroll.stop();
},
onDropActivate(e) {
if (state.target?.type === 'item' && state.target?.dropPosition === 'on' && typeof props.onDropActivate === 'function') {
if (state.target?.type === 'item' && typeof props.onDropActivate === 'function') {
props.onDropActivate({
type: 'dropactivate',
x: e.x, // todo
Expand Down Expand Up @@ -741,6 +743,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state:
break;
}
}
localState.props.onKeyDown?.(e as any);
Copy link
Member

Choose a reason for hiding this comment

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

e won't be a keyboard event here. What was this intended for?

}
});
}, [localState, ref, onDrop, direction]);
Expand Down
159 changes: 153 additions & 6 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 @@ -107,6 +107,20 @@ export interface TreeData<T extends object> {
*/
move(key: Key, toParentKey: Key | null, index: number): void,

/**
* Moves one or more items before a given key.
* @param key - The key of the item to move the items before.
* @param keys - The keys of the items to move.
*/
moveBefore(key: Key, keys: Iterable<Key>): void,

/**
* Moves one or more items after a given key.
* @param key - The key of the item to move the items after.
* @param keys - The keys of the items to move.
*/
moveAfter(key: Key, keys: Iterable<Key>): void,

/**
* Updates an item in the tree.
* @param key - The key of the item to update.
Expand All @@ -115,6 +129,11 @@ export interface TreeData<T extends object> {
update(key: Key, newValue: T): void
}

interface TreeDataState<T extends object> {
items: TreeNode<T>[],
nodeMap: Map<Key, TreeNode<T>>
}

/**
* Manages state for an immutable tree data structure, and provides convenience methods to
* update the data over time.
Expand All @@ -128,7 +147,7 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
} = options;

// We only want to compute this on initial render.
let [tree, setItems] = useState<{items: TreeNode<T>[], nodeMap: Map<Key, TreeNode<T>>}>(() => buildTree(initialItems, new Map()));
let [tree, setItems] = useState<TreeDataState<T>>(() => buildTree(initialItems, new Map()));
let {items, nodeMap} = tree;

let [selectedKeys, setSelectedKeys] = useState(new Set<Key>(initialSelectedKeys || []));
Expand All @@ -141,7 +160,7 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
items: initialItems.map(item => {
let node: TreeNode<T> = {
key: getKey(item),
parentKey: parentKey,
parentKey: parentKey ?? null,
value: item,
children: null
};
Expand All @@ -154,9 +173,9 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
};
}

function updateTree(items: TreeNode<T>[], key: Key, update: (node: TreeNode<T>) => TreeNode<T> | null, originalMap: Map<Key, TreeNode<T>>) {
let node = originalMap.get(key);
if (!node) {
function updateTree(items: TreeNode<T>[], key: Key | null, update: (node: TreeNode<T>) => TreeNode<T> | null, originalMap: Map<Key, TreeNode<T>>) {
let node = key == null ? null : originalMap.get(key);
if (node == null) {
return {items, nodeMap: originalMap};
}
let map = new Map<Key, TreeNode<T>>(originalMap);
Expand Down Expand Up @@ -233,7 +252,6 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
}
}
}

return {
items,
selectedKeys,
Expand Down Expand Up @@ -352,6 +370,8 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData

// If parentKey is null, insert into the root.
if (toParentKey == null) {
// safe to reuse the original map since no node was actually removed, so we just need to update the one moved node
newMap = new Map(originalMap);
newMap.set(movedNode.key, movedNode);
return {items: [
...newItems.slice(0, index),
Expand All @@ -373,6 +393,39 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
}), newMap);
});
},
moveBefore(key: Key, keys: Iterable<Key>) {
setItems((prevState) => {
let {items, nodeMap} = prevState;
let node = nodeMap.get(key);
if (!node) {
return prevState;
}
let toParentKey = node.parentKey ?? null;
let parent: null | TreeNode<T> = null;
if (toParentKey != null) {
parent = nodeMap.get(toParentKey) ?? null;
}
let toIndex = parent?.children ? parent.children.indexOf(node) : items.indexOf(node);
return moveItems(prevState, keys, parent, toIndex, updateTree);
});
},
moveAfter(key: Key, keys: Iterable<Key>) {
setItems((prevState) => {
let {items, nodeMap} = prevState;
let node = nodeMap.get(key);
if (!node) {
return prevState;
}
let toParentKey = node.parentKey ?? null;
let parent: null | TreeNode<T> = null;
if (toParentKey != null) {
parent = nodeMap.get(toParentKey) ?? null;
}
let toIndex = parent?.children ? parent.children.indexOf(node) : items.indexOf(node);
toIndex++;
return moveItems(prevState, keys, parent, toIndex, updateTree);
});
},
update(oldKey: Key, newValue: T) {
setItems(({items, nodeMap: originalMap}) => updateTree(items, oldKey, oldNode => {
let node: TreeNode<T> = {
Expand All @@ -389,3 +442,97 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData
}
};
}

function moveItems<T extends object>(
state: TreeDataState<T>,
keys: Iterable<Key>,
toParent: TreeNode<T> | null,
toIndex: number,
updateTree: (
items: TreeNode<T>[],
key: Key,
update: (node: TreeNode<T>) => TreeNode<T> | null,
originalMap: Map<Key, TreeNode<T>>
) => TreeDataState<T>
): TreeDataState<T> {
let {items, nodeMap} = state;

let parent = toParent;
let removeKeys = new Set(keys);
while (parent?.parentKey != null) {
if (removeKeys.has(parent.key)) {
throw new Error('Cannot move an item to be a child of itself.');
}
parent = nodeMap.get(parent.parentKey!) ?? null;
}

let originalToIndex = toIndex;

let keyArray = Array.isArray(keys) ? keys : [...keys];
// depth first search to put keys in order
let inOrderKeys: Map<Key, number> = new Map();
let removedItems: Array<TreeNode<T>> = [];
let newItems = items;
let newMap = nodeMap;
let i = 0;

function traversal(node, {inorder, postorder}) {
inorder?.(node);
if (node != null) {
for (let child of node.children ?? []) {
traversal(child, {inorder, postorder});
postorder?.(child);
}
}
}

function inorder(child) {
// in-order so we add items as we encounter them in the tree, then we can insert them in expected order later
if (keyArray.includes(child.key)) {
inOrderKeys.set(child.key, i++);
}
}

function postorder(child) {
// remove items and update the tree from the leaves and work upwards toward the root, this way
// we don't copy child node references from parents inadvertently
if (keyArray.includes(child.key)) {
removedItems.push({...newMap.get(child.key)!, parentKey: toParent?.key ?? null});
let {items: nextItems, nodeMap: nextMap} = updateTree(newItems, child.key, () => null, newMap);
newItems = nextItems;
newMap = nextMap;
}
// decrement the index if the child being removed is in the target parent and before the target index
if (child.parentKey === toParent?.key
&& keyArray.includes(child.key)
&& (toParent?.children ? toParent.children.indexOf(child) : items.indexOf(child)) < originalToIndex) {
toIndex--;
}
}

traversal({children: items}, {inorder, postorder});

let inOrderItems = removedItems.sort((a, b) => inOrderKeys.get(a.key)! > inOrderKeys.get(b.key)! ? 1 : -1);
// If parentKey is null, insert into the root.
if (!toParent || toParent.key == null) {
newMap = new Map(nodeMap);
inOrderItems.forEach(movedNode => newMap.set(movedNode.key, movedNode));
return {items: [
...newItems.slice(0, toIndex),
...inOrderItems,
...newItems.slice(toIndex)
], nodeMap: newMap};
}

// Otherwise, update the parent node and its ancestors.
return updateTree(newItems, toParent.key, parentNode => ({
key: parentNode.key,
parentKey: parentNode.parentKey,
value: parentNode.value,
children: [
...parentNode.children!.slice(0, toIndex),
...inOrderItems,
...parentNode.children!.slice(toIndex)
]
}), newMap);
}
Loading