fix: preserve shape across id changes for cross-container sort animations#1925
fix: preserve shape across id changes for cross-container sort animations#1925
Conversation
…ions When a sortable item moves between containers, its droppable identity changes and the previous shape reference is lost before the animation can compute the correct delta. This introduces a shape cache (shapeStore) that captures the droppable shape at drag start and updates it after each animation frame, so the animate method always has a valid previous shape to diff against — even after cross-container reparenting. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@dnd-kit/abstract
@dnd-kit/collision
@dnd-kit/dom
@dnd-kit/geometry
@dnd-kit/helpers
@dnd-kit/react
@dnd-kit/solid
@dnd-kit/state
@dnd-kit/svelte
@dnd-kit/vue
commit: |
clauderic
left a comment
There was a problem hiding this comment.
Summary
This PR fixes cross-container sort animations by introducing a shapeStore cache that preserves the droppable shape across identity changes (which happen when items move between containers). The cached shape is used when computing animation deltas after idChanged is true, ensuring smooth transitions even after the sortable's group/container changes.
Feedback
Blocking issues
- Missing changeset: The
changeset-botalready flagged this, but there's no.changeset/*.mdfile for the change topackages/dom/src/sortable/sortable.ts. A patch-level changeset for@dnd-kit/domis needed before this can be released.
Suggestions
sortable.ts:190-196— The new effect triggersanimate(true)viaqueueMicrotaskwheneverstatus.draggingis true and a cached shape exists. Consider whether this could fire unnecessarily on each reactive update during a drag (e.g., every position change), potentially causing redundant animation calls. It might be worth guarding it more precisely (e.g., tracking when the id actually changed rather than just checking for a cached shape).- The
shapeStorecache doesn't appear to have an explicit cleanup path at drag end. While the WeakStore prevents leaks when the manager is GC'd, stale shape entries could persist across multiple drag sessions within the same manager. A cleanup on drag idle/end might be worth considering.
Changeset
- Status: missing — a
.changeset/*.mdwith"@dnd-kit/dom": patchis required.
Overall
The approach is sound and addresses a real pain point with cross-container animations. The cache strategy is elegant. Just needs a changeset added before merge.
[claude-review]
Summary
shapeStorecache that captures the droppable shape at drag start and keeps it updated after each animation frameSortable.animate()to accept anidChangedflag, using the cached shape when the sortable's identity has changed due to cross-container reparentingProblem
When a sortable item moves between containers (e.g., from list A to list B), its droppable identity changes and the previous
droppable.shapereference is lost before the animation can compute the correct positional delta. This caused animations to either skip or produce incorrect visual transitions during cross-container sorting.Test plan