Skip to content

fix: Update dnd to handle inert #8560

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 1 commit into from
Jul 16, 2025
Merged

fix: Update dnd to handle inert #8560

merged 1 commit into from
Jul 16, 2025

Conversation

devongovett
Copy link
Member

When we updated to use inert in ariaHideOutside, I forgot to update other places in the DragManager code that were looking specifically for aria-hidden.

📝 Test Instructions:

In the Virtualized Tree with dnd story, keyboard navigate to Reports, focus the drag button, and press enter. The drop indicator at the end of the list should scroll into view. This was previously broken in Safari.

@rspbot
Copy link

rspbot commented Jul 16, 2025

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

LGTM, verified the behavior in Safari and that things seem to work fine in other browsers

@nwidynski
Copy link
Contributor

@devongovett Just linking #8317 (comment) here again, in case it was missed. I think it may make sense to add inert to getItemElement as well.

@devongovett
Copy link
Member Author

hmm, I think having duplicate keys in a collection might cause other problems though. For example, useSelectableItem handles focusing its own element when the focusedKey matches its own key. If multiple items have the same key, they'd end up both trying to focus themselves.

@devongovett devongovett added this pull request to the merge queue Jul 16, 2025
Merged via the queue into main with commit 358680d Jul 16, 2025
31 checks passed
@devongovett devongovett deleted the dnd-inert branch July 16, 2025 21:09
@nwidynski
Copy link
Contributor

nwidynski commented Jul 16, 2025

@devongovett In this case the focus of the clone would be suppressed by the inert parent, causing only the "real" node to have focus. You are correct though, its better to make sure data-key doesn’t match. We solved this by passing the render function of the clone a copy of itself with a mutated ref that can be used to attach a suffix on the data-key attribute 👍

In terms of self focus without an inert parent, we marshalled focus from the cloned node to the real node whenever focus is within the clone 😋 Again, implementation is obsolete anyways with the Virtualizer, but thanks for taking the time to take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants