Tree Drag & Drop: Child item can't be moved to root if only one root item

[rothsandro]

Hi,

I'm using the RAC Tree component with drag & drop support. If the tree contains a single root item with children, I can't move the child item to the root before the current root item.

> Root
  > Child

// Try to move child before root to get:

> Child
> Root

The drag indicator doesn't appear but dropping the item at the ~right position calls onMove with an invalid target key react-aria-1. The child can be moved to the root after the root item and from there (with two items in the root) it can be moved before the root item.

I opened the Tree - Moving items demo in Stackblitz and changed the data to have a single root item. It can be reproduced here.

rac-tree-dnd.mov

Repro: https://stackblitz.com/edit/csdzuzrl?file=src%2FExample.tsx

Is my code (and the demo in the docs) wrong or is this a bug in React Aria?

Regards,
Sandro

[snowystinger]

I spent a bit of time looking at this today. I see the issue as well, following your instructions, but I'm not sure where the issue is yet.

Because it works if there are other items, it doesn't seem to be related to the drop above being too thin because that space is the same in both cases.

In other places I can drop before a parent key, so it's likely not related to that.

[rothsandro]

I'm not familiar with the implementation but used AI to debug the problem 🙈

The TreeDropTargetDelegate.getPotentialTargets() has some code to "handle converting 'after' to 'before next' for non-ambiguous cases" which is triggered in my use case. Moving the child before the parent has dropPosition before so I'm not sure if this should actually happen (should the if statement check the target's drop position?). The code then accesses the nextNode and uses it as beforeTarget without checking its type. The next node is of type content and here's where the invalid id I mentioned is coming from (I assume it should only use item nodes?).

react-spectrum/packages/react-aria-components/src/TreeDropTargetDelegate.ts

Lines 193 to 207 in 7ba11ce

	// Handle converting "after" to "before next" for non-ambiguous cases
	if (potentialTargets.length === 1) {
	let nextKey = collection.getKeyAfter(target.key);
	let nextNode = nextKey ? collection.getItem(nextKey) : null;
	if (nextKey != null && nextNode && currentItem && nextNode.level != null && currentItem.level != null && nextNode.level > currentItem.level) {
	let beforeTarget = {
	type: 'item',
	key: nextKey,
	dropPosition: 'before'
	} as const;
	if (isValidDropTarget(beforeTarget)) {
	return [beforeTarget];
	}
	}
	}

[snowystinger]

Ah, that sounds like it could be on the right track, we have a few issues with the node types right now and this seems like one of those cases. You could try checking the node type and create a PR for further discussion. Thanks for diving into that, even with AI :)

[rothsandro]

I created a draft PR: #9841

[MILLERMARRU]

I spent some time in TreeDropTargetDelegate.ts (react-aria-components) chasing this, and I think I found the specific asymmetry that causes it, though I haven't gotten a full fix together.

In resolveDropTarget, a before position on the target gets normalized to after of the previous item first. When the target is the very first item in the whole tree (your single root), getKeyBefore(target.key) returns null, so that normalization is skipped and target stays as { type: 'item', key: Root, dropPosition: 'before' }.

Then in getPotentialTargets, this hits:

if (currentItem?.nextKey != null) {
  return [originalTarget];
}

Root's nextKey is its own child (since a node's nextKey walks the flattened collection, not just siblings), so this branch fires and returns [originalTarget] immediately, without ever running it through isValidDropTarget.

Compare that to the other two paths in the same function: the "before first child" path (isValidDropTarget(beforeFirstChildTarget) check before returning) and the ancestor-walk path (.filter(isValidDropTarget) at the very end). Both of those validate before returning a target; the nextKey != null early return doesn't.

So the single-root case is the only shape where a tree hits that specific unvalidated early return with a before position on the very first item. If isValidDropTarget would have rejected that particular target (unclear to me without instrumenting a real DOM tree, since I can't tell from source alone what your getDropOperation/onMove callback is doing with react-aria-1, which looks like a fallback/root collection key rather than an actual item key), the missing check would explain why the indicator never renders and why the eventual onMove call gets a target it was never meant to receive.

Not confident this is the whole story since I haven't run it against your Stackblitz repro, but adding isValidDropTarget(originalTarget) to that early return (mirroring the other two branches) looks like the natural next thing to try.