fix: smooth drag-and-drop reordering of relation chips - #1806
webmagic123 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
4e9b19e to
1ae444a
Compare
c08a5fe to
686befe
Compare
4f81be7 to
8e6cdcf
Compare
b-d055
changed the title
b-d055
changed the title
8e6cdcf to
2908459
Compare
2908459 to
93531ed
Compare
b-d055
changed the title
93531ed to
6088280
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the relation-chips drag-and-drop implementation to support smooth reordering for variable-width, flex-wrapped chip lists by migrating that surface to the next-gen dnd-kit packages, while keeping the classic dnd-kit stack for other UI areas.
Changes:
- Replace the classic
@dnd-kit/core+@dnd-kit/sortablestrategy-based implementation with next-gen@dnd-kit/react+@dnd-kit/helpersfor relation-chip reordering. - Extend
LinkableRelationChipto support a callback-ref drag handle API (sortableDragHandleRef) and wire it to the dots button. - Add a Vitest setup stub for
ResizeObserver, and move the add-entity control into the chip list via anafterChipsslot.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| bun.lock | Adds lock entries for next-gen dnd-kit packages and transitive deps. |
| apps/web/package.json | Adds @dnd-kit/react and @dnd-kit/helpers dependencies. |
| apps/web/vitest.setup.ts | Stubs ResizeObserver for node/jsdom test imports. |
| apps/web/vite.config.js | Registers the Vitest setupFiles to load the ResizeObserver stub. |
| apps/web/design-system/reorderable-relation-chips-dnd.tsx | Migrates relation-chip DnD to next-gen dnd-kit with optimistic FLIP reordering and an afterChips slot. |
| apps/web/design-system/chip.tsx | Adds sortableDragHandleRef support and forwards it to the dots-button ref. |
| apps/web/partials/entity-page/editable-entity-page.tsx | Uses afterChips to place the add-entity UI inside the chip list and adjusts layout wrapper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| ref={triggerRef} | ||
| ref={node => { | ||
| triggerRef.current = node; | ||
| sortableDragHandleRef?.(node); | ||
| }} |
There was a problem hiding this comment.
Fixed in ec73f1e — the merged ref is now a stable React.useCallback (dotsButtonRef), so the drag handle no longer detaches/re-attaches on hover/popover re-renders.
Migrate the relation-chip reorder component to the next-gen dnd-kit (@dnd-kit/react + @dnd-kit/helpers). The classic sortable strategies compute transforms assuming uniform item sizes, which stretched, mis-spaced, or snapped variable-width chips in wrapped multi-row lists. The new library measures the real DOM and FLIP-animates optimistic reordering, so wrapped chip rows reflow smoothly and land correctly. Positions are committed on drop, reusing the existing position set. - reorderable-relation-chips-dnd.tsx: rewritten on DragDropProvider/ useSortable; adds afterChips slot so the add-entity button lives inside the flex-wrapped list - chip.tsx: LinkableRelationChip accepts sortableDragHandleRef (the new API marks handles via callback ref); dots button unchanged - editable-entity-page.tsx: renders the add button via afterChips; IMAGE/VIDEO branching normalized to renderableTypeStrict - vite.config.js + vitest.setup.ts: stub ResizeObserver in the test environment; @dnd-kit/dom references it at module scope and jsdom does not implement it Co-authored-by: Juan Mardikian <developermardikian@gmail.com>
6088280 to
ec73f1e
Compare
|
Closing as part of a sweep of the open-PR queue. Not a judgement on the work — reopen if you still want it and I will help get it current. Opened 2026-05-15 and now conflicting with master. At this distance a rebase is usually more work than redoing the change against current code, and the surrounding code has moved a long way underneath "fix: smooth drag-and-drop reordering of relation chips". @webmagic123 — if the idea still stands but the branch does not, a fresh PR or a ticket is probably a better route than reviving this one. Nothing is discarded: the branch and its history remain, and reopening costs a click. Context: 74 PRs were open, 25 older than two months, the oldest from February. The point is to make the queue mean something so genuinely ready work is visible rather than buried — #2449 sat ready for three days this week partly because of the noise. Only non-draft, conflicting PRs are in scope; drafts and anything still mergeable are being left alone. |
|
Reopened — closing this was my mistake. It was caught in a sweep of non-draft PRs older than two months that conflict with master. The criteria were mechanical and this one met them, but the evidence against closing was a click away and I did not look: #2423 is a replay of this branch on master, opened eight days ago precisely because this one's preview is unusable at 378 commits behind, and its body says "Review and merge the original." I checked afterwards, and the bug is still live on master:
Worth deciding, since this branch is still 378 behindThe content here is good and verified, but this branch is not the practical way to land it. #2423 carries the same change on current master — So the realistic options are to rebase this (378 commits, for one component), or to un-draft #2423 and land that instead. The second looks much cheaper, and #2423 only says "not for merge" because it was opened as a preview target rather than because anything is wrong with it. @webmagic123 — your call, and apologies for the noise. |
What
Makes drag-and-drop reordering of relation chips feel smooth, including wrapped multi-row chip lists.
This PR was rebuilt from scratch on current
master(the previous 59-commit history had a duplicated lineage from a bad merge and a 77-commit-stale base). It is now a single commit with a net-negative diff.Why the rewrite
The classic
@dnd-kit/sortablestrategies compute item transforms assuming uniform item sizes. Relation chips are variable-width and wrap across rows, so every classic approach failed in testing:horizontalListSortingStrategybreaks on multi-row lists,rectSortingStrategystretches/mis-spaces chips (its scale/offset math assumes a uniform grid), and a custom insert-gap engine (the earlier revision of this PR) disabled per-chip sliding entirely, so chips snapped instead of animating.How
Migrates this one component to the next-gen dnd-kit:
@dnd-kit/react+@dnd-kit/helpers(0.5.0) — measures the real DOM and FLIP-animates optimistic reordering, so variable-width wrapped chips reflow smoothly and land correctly. Reordering is optimistic during the drag; positions are committed on drop by reassigning the existing position set (same scheme as before).chip.tsx:LinkableRelationChipgains an optionalsortableDragHandleRef(the new API marks drag handles via callback ref instead of spread listeners). The dots button remains the handle; grab UX unchanged.editable-entity-page.tsx: the add-entity button moves into the chip list via the newafterChipsslot; IMAGE/VIDEO branching normalized torenderableTypeStrict(behavior-equivalent).The classic
@dnd-kit/core/@dnd-kit/sortablepackages remain — other surfaces (table rows, tabs, etc.) still use them; the libraries coexist.Scope note
An earlier revision of this branch also migrated the type property-groups editor onto a shared multi-container drag engine. That migration was reverted: review found it introduced data-loss regressions (group deletion removed properties from the type schema, orphaned group values, phantom position edits from a hidden-mounted editor, and dropped entity hydration). The property-groups editor is unchanged from
masterin this PR; making it smooth can be a follow-up on top of the new library.An earlier revision also changed table-block row dragging (
table-block-dnd-items.tsx: collapsing the dragged row in place). That was reverted as out of scope — the collapse fought the classic library's layout measurements and degraded the list/gallery/bulleted-list drag feel. Table blocks are unchanged frommaster.Testing
tsc --noEmitclean against currentmaster