Skip to content

Conversation

@chirokas
Copy link
Contributor

@chirokas chirokas commented Jul 28, 2025

Closes

  • Fix a crash during DnD keyboard navigation in RAC Table with TableLoadMoreItem. see StackBlitz
  • Skip non-item nodes (e.g., tablebody, loader) since they can't be drop targets.

✅ 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:

In the Dnd Table Example/tableview dnd story, start a keyboard drag session and hold ArrowUp to cycle through the table multiple times.

🧢 Your Project:

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.

Apologies for the delay, but looks good to me. Did some extra testing around sections + DnD and tree loaders + DnD but the things I found are unrelated to your changes here so I'll file some issues for the team to handle later. Thanks for the fix!

@@ -195,7 +195,7 @@ export function renderAfterDropIndicators(collection: ICollection<Node<unknown>>
let afterIndicators: ReactNode[] = [];
if (nextItemInSameLevel == null) {
let current: Node<unknown> | null = node;
while (current && (!nextItemInFlattenedCollection || (current.parentKey !== nextItemInFlattenedCollection.parentKey && nextItemInFlattenedCollection.level < current.level))) {
while (current?.type === 'item' && (!nextItemInFlattenedCollection || (current.parentKey !== nextItemInFlattenedCollection.parentKey && nextItemInFlattenedCollection.level < current.level))) {
Copy link
Member

Choose a reason for hiding this comment

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

We might need to tweak to for the eventual tree with sections case? This is fine for now, I'm not really sure if after drop positions would ever become available for sections

@github-project-automation github-project-automation bot moved this from 🩺 To Triage to 👀 In Review in RSP Component Milestones Oct 22, 2025
@snowystinger snowystinger added this pull request to the merge queue Oct 22, 2025
Merged via the queue into adobe:main with commit 27edf8f Oct 22, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in RSP Component Milestones Oct 22, 2025
@chirokas chirokas deleted the dnd-table branch October 23, 2025 05:50
bsmiley1 pushed a commit to bsmiley1/react-spectrum that referenced this pull request Dec 5, 2025
* test

* Fix crash on renderAfterDropIndicators in Table

* Fix table dnd keyboard navigation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants