fix: prevent table row drag from moving an extra adjacent row#2703
Conversation
When a row drag is dropped outside `.bn-block-group` (e.g. into the side menu gutter), `SideMenuView.dispatchSyntheticEvent` re-dispatches the drop with `synthetic = true` so ProseMirror can handle a coordinate inside the editor. The synthetic event AND the original drop both bubble to `pmView.root`, so `TableHandlesView.dropHandler` runs twice for one user gesture; ProseMirror commits the row move on the first call, and the second call moves the now-shifted adjacent row. Make `dropHandler` idempotent by clearing `state.draggingState` once the move logic has the values it needs. The re-dispatched drop hits the existing early return for `draggingState === undefined` and no longer runs the move twice. Adds a Playwright regression test that drags a row to the side menu gutter (the user-facing reproduction in issue TypeCellOS#2691) and asserts only the dragged row moves. Closes TypeCellOS#2691
|
@LimChaeJune is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR fixes a bug where dragging a table row moved two rows instead of one. The ChangesTable Row Drag Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
nperez0111
left a comment
There was a problem hiding this comment.
Thanks for this @LimChaeJune this fixes the issue well!
Summary
Fixes a bug where dragging a single table row could move an extra adjacent row when the drop lands outside
.bn-block-group(e.g. into the side menu gutter). Closes #2691.Rationale
SideMenuView.dispatchSyntheticEventre-dispatchesdropevents assyntheticso ProseMirror can handle drops that fall outside the editor bounds. BothSideMenuViewandTableHandlesViewlisten fordroponpmView.root.SideMenuView's handlers already guard against the synthetic event, butTableHandlesView.dropHandlerdoes not — so a single user gesture invokesdropHandlertwice (once for the synthetic, once for the original). Between the two calls, ProseMirror commits the row move and refreshesstate.block, causing the second call to drag the now-shifted neighbor along with the original target.The fix clears
state.draggingStateoncedropHandlerhas the values it needs, making the handler idempotent. This avoids leaking knowledge ofSideMenuView's synthetic-event convention intoTableHandlesViewand protects against any future path that re-entersdropHandlerfor the same gesture.Changes
packages/core/src/extensions/TableHandles/TableHandles.ts: After destructuringdraggingStatefromthis.state, clearthis.state.draggingState = undefined. The re-dispatched drop now short-circuits at the existingdraggingState === undefinedearly return.tests/src/end-to-end/tables/tables.test.ts: Added a Playwright regression test that drags row 2 of a 5-row table onto the side-menu gutter (blockGroup.x - 50) and asserts the resulting row order.Impact
dragEndstill setsdraggingState = undefined, which is now a no-op afterdropHandlerran but stays correct for drag cancellations that never reachdropHandler.Testing
["R1", "R3", "R4", "R5", "R2"]. Fails without the fix with["R1", "R4", "R5", "R2", "R3"](the exact bug pattern from Table row drag moves two rows instead of one due to duplicate dropHandler invocation #2691). Passes on chromium and webkit; firefox is skipped following the existing dragdrop test convention.Screenshots/Video
Before

After

Checklist
Additional Notes
the bug is intermittent in real use because it only triggers when the drop coordinate falls outside
.bn-block-group. The regression test pins the drop to the side gutter to make this deterministic.Summary by CodeRabbit
Bug Fixes