Skip to content

fix: Update Tree drag and drop focus#9751

Open
chirokas wants to merge 1 commit intoadobe:mainfrom
chirokas:tree-focus-after-drop
Open

fix: Update Tree drag and drop focus#9751
chirokas wants to merge 1 commit intoadobe:mainfrom
chirokas:tree-focus-after-drop

Conversation

@chirokas
Copy link
Contributor

@chirokas chirokas commented Mar 6, 2026

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Open the RAC TreeWithDragAndDrop story, drop "Projects" into the second tree (After the insert, focus should be on the newly inserted item), and attempt to keyboard drag "Projects" again into the second tree. Previously, this caused a bug where focus shifted to the collection, but now it should work correctly.

🧢 Your Project:

@github-actions github-actions bot added the RAC label Mar 6, 2026
@chirokas chirokas force-pushed the tree-focus-after-drop branch 2 times, most recently from fe93cbe to db03626 Compare March 6, 2026 14:43
@chirokas chirokas marked this pull request as ready for review March 6, 2026 14:52
@chirokas chirokas marked this pull request as draft March 7, 2026 06:46
@chirokas chirokas force-pushed the tree-focus-after-drop branch from db03626 to 45c5f1c Compare March 7, 2026 12:56
@chirokas chirokas marked this pull request as ready for review March 7, 2026 13:05
@chirokas chirokas force-pushed the tree-focus-after-drop branch from 45c5f1c to b7620f4 Compare March 12, 2026 16:34
@chirokas chirokas changed the title fix: correctly update focus to the first inserted item after a drop in Tree fix: Update Tree drag and drop focus Mar 12, 2026
@andreaaazo
Copy link

hi

first = item.parentKey;
}

if (item?.type === 'content') {
Copy link
Member

Choose a reason for hiding this comment

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

would you mind commenting the code here?

parentKey = parent?.parentKey;
continue;
}
let item = getItemElement(ref, parentKey);
Copy link
Member

Choose a reason for hiding this comment

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

is this just a check that there is a dom node being rendered? can we just set the collection to being focused and then let the selectable item focus itself? I'm assuming not, but I don't understand why.
Is it in case you drop an item inside another?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In short, if an item does not exist in the collection, it means its parent is collapsed. In this case, we should not focus that item, but rather focus its parent, which is consistent with our previous behavior. This applies not only to the case of dropping an item inside another item, but also to the case of dropping a parent item with children into another tree: the newly inserted item is collapsed by default, so we should focus the top-level parent.

@chirokas chirokas force-pushed the tree-focus-after-drop branch from b7620f4 to 50ee395 Compare March 13, 2026 16:06
@chirokas chirokas force-pushed the tree-focus-after-drop branch from 50ee395 to c14db17 Compare March 14, 2026 07:43
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and updates. This way seems like a good start. I'm still mulling over if we want to add something to the collection or somewhere so that droppable collection doesn't need to know about the structure or the rendered items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants