Support scroll adjusted transform#2073
Conversation
🦋 Changeset detectedLatest commit: 6cc8f63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
clauderic
left a comment
There was a problem hiding this comment.
Summary
Adds a new ScrollCompensator core plugin to @dnd-kit/dom and a scrollAdjustment / adjustedTransform field on DragOperation, restoring the scroll-adjusted delta semantics from the legacy @dnd-kit/core. While dragging, the plugin listens for scroll events on the source's owner document, filters to the source's scrollable ancestors, accumulates the delta against the initial scroll offsets, and dispatches a virtual: true move so listeners see the updated transform. Closes #2069.
Feedback
Suggestions
packages/abstract/src/core/manager/operation.ts:182—get adjustedTransform()is decorated@derivedbut has a write side-effect (this.#adjustedTransform = result). The whole purpose of#adjustedTransformis to keepsnapshot()in sync, but it only updates when something reads the getter. ThedisposeAdjustedTransformEffectin ScrollCompensator.ts exists solely to force a read on every reactive change. That's two indirections to achieve what could be one line insnapshot():This would let you drop both theadjustedTransform: { x: this.#transform.x + this.scrollAdjustment.x, y: this.#transform.y + this.scrollAdjustment.y, },
#adjustedTransformcache and the second effect in the plugin. Worth simplifying before this lands — the current shape will be confusing to future readers.packages/dom/src/core/plugins/scrolling/ScrollCompensator.ts:60—manager.actions.move({event, virtual: true})dispatchesdragmovewith the scroll event as thenativeEvent. Consumers that branch onnativeEventtype (e.g. distinguishing pointer/keyboard) will see an unexpectedScrollEvent. Consider documenting this in the JSDoc for thevirtualoption in actions.ts, or passevent: undefinedif the scroll event is not semantically meaningful here.packages/dom/src/core/plugins/scrolling/ScrollCompensator.ts:43—initialis captured the first time the effect runs (whendraggingflips true). If the source'sscrollableAncestorschange during a drag (e.g. the DOM structure shifts), neither the ancestor set norinitialare recomputed. Probably fine in practice — flagging in case it's intentional.- No tests cover
ScrollCompensatororadjustedTransformbehavior. The legacydeltasemantics were subtle (scroll inside the drag would updatedeltaeven without pointer movement) — a bun:test that starts a drag, simulates a scroll event on an ancestor, and asserts the dispatcheddragmovecarries the expectedadjustedTransformwould catch regressions.
Changeset
- Status: present and correct (
'@dnd-kit/abstract': minor,'@dnd-kit/dom': minor). Minor is right for a new public field/plugin export, pre-1.0.
Overall
Thanks @jindong-zhannng — this is a thoughtful port of a feature that legacy users definitely rely on, and the docs update on drag-drop-manager.mdx is a nice touch. The main thing I'd ask for is simplifying the adjustedTransform plumbing so the cache and forcing-effect go away, plus a small test.
[claude-review]
886f679 to
e2abe6b
Compare
|
Simplified |
e2abe6b to
69891d0
Compare
|
Hi @clauderic , can we move forward this PR? |
9954509 to
6cc8f63
Compare
Close #2069
Port scroll-adjusted translate functionality from legacy dnd-kit:
dragOperation.adjustedTransform.