Skip to content

Commit 6782848

Browse files
serpentbladeclaude
andcommitted
fix(sortable-list): use draggable-relative indices so Lit slot elements don't shift the reorder
The `#header`/`#footer` slots (added in the 6-20 sortable-list round-out) render as REAL `<slot>` elements inside the list container on Lit (shadow DOM), whereas an empty named slot renders nothing on the 5 light-DOM targets. SortableJS's plain `e.oldIndex`/`e.newIndex` count ALL element children, so the leading `<slot name="header">` offset every index by +1 — but ONLY on Lit. Two sites consumed those polluted indices: - handleStart stashed `current[e.oldIndex]` → captured the row AFTER the one actually grabbed; the identity-based commit then faithfully moved the wrong item (root cause of the off-by-one reorder). - handleCommit placed the moved item at `e.newIndex` → one slot too far. Switch both to `e.oldDraggableIndex`/`e.newDraggableIndex` (`index(dragEl, options.draggable)` — counts only the `.rozie-sortable-item` rows, matching the `draggable: '.rozie-sortable-item'` already set). When no non-draggable children exist the two coincide, so the 5 light-DOM targets are byte-for-behavior unchanged. Fixes the 4 Lit VR specs that regressed during the 6-20 round-out and were masked for ~24h by the VR webServer timeout (058131a): sortable-drag, -cross, -multi, and nested-lit-reset. Verified locally: all 29 sortable specs green across all 6 targets; 6 leaves regenerated; lit/react/solid leaves typecheck. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014ArTPHLhYMwrjvvoF9VXDe
1 parent acf5c92 commit 6782848

7 files changed

Lines changed: 252 additions & 35 deletions

File tree

packages/ui/sortable-list/packages/angular/src/internal/useSortableJS.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,19 @@ export function useSortableJS<T>(
175175
* it on the destination side. */
176176
const handleStart = (e: SortableEvent): void => {
177177
const current = opts.items();
178-
// `e.oldIndex` is the source index in the source list (this list,
179-
// since onStart fires from the source).
180-
const fromIdx = typeof e.oldIndex === 'number' ? e.oldIndex : -1;
178+
// The source index in this list (onStart fires from the source). Prefer the
179+
// DRAGGABLE-relative index: with non-draggable element children present
180+
// (Lit shadow-DOM `#header`/`#footer` `<slot>`s), plain `e.oldIndex` counts
181+
// all children and is offset — stashing `current[oldIndex]` would capture
182+
// the WRONG item (the row after the one actually grabbed), and the
183+
// identity-based commit lookup would then faithfully move that wrong item.
184+
// `oldDraggableIndex` counts only the `.rozie-sortable-item` rows.
185+
const fromIdx =
186+
typeof e.oldDraggableIndex === 'number'
187+
? e.oldDraggableIndex
188+
: typeof e.oldIndex === 'number'
189+
? e.oldIndex
190+
: -1;
181191
if (fromIdx >= 0 && fromIdx < current.length) {
182192
const item = e.item as unknown as Record<string, unknown> | null;
183193
if (item !== null && typeof item === 'object') {
@@ -225,10 +235,31 @@ export function useSortableJS<T>(
225235
// physically moves the same node between lists).
226236
const stashedItem = readStash<T>(e.item);
227237

238+
// Prefer the DRAGGABLE-relative indices. SortableJS computes plain
239+
// `oldIndex`/`newIndex` via `index(dragEl)` — counting ALL element
240+
// siblings — while `oldDraggableIndex`/`newDraggableIndex` use
241+
// `index(dragEl, options.draggable)`, counting only the
242+
// `.rozie-sortable-item` rows. When the list container also holds
243+
// non-draggable element children, the plain indices are offset. This bites
244+
// ONLY on Lit: the `#header` / `#footer` named slots render as real
245+
// `<slot>` ELEMENTS inside the shadow-DOM list container (the 5 light-DOM
246+
// targets render nothing for an empty named slot), so the leading
247+
// `<slot name="header">` shifts every plain `newIndex` by +1 and the
248+
// writeback dropped items one slot too far. The draggable indices stay
249+
// item-relative everywhere; when no non-draggable children exist the two
250+
// coincide, so the 5 light-DOM targets are unchanged.
228251
const oldIndexHint =
229-
typeof e.oldIndex === 'number' ? e.oldIndex : -1;
252+
typeof e.oldDraggableIndex === 'number'
253+
? e.oldDraggableIndex
254+
: typeof e.oldIndex === 'number'
255+
? e.oldIndex
256+
: -1;
230257
const newIndexHint =
231-
typeof e.newIndex === 'number' ? e.newIndex : -1;
258+
typeof e.newDraggableIndex === 'number'
259+
? e.newDraggableIndex
260+
: typeof e.newIndex === 'number'
261+
? e.newIndex
262+
: -1;
232263

233264
let next: T[];
234265
let oldIndex: number;

packages/ui/sortable-list/packages/lit/src/internal/useSortableJS.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,19 @@ export function useSortableJS<T>(
175175
* it on the destination side. */
176176
const handleStart = (e: SortableEvent): void => {
177177
const current = opts.items();
178-
// `e.oldIndex` is the source index in the source list (this list,
179-
// since onStart fires from the source).
180-
const fromIdx = typeof e.oldIndex === 'number' ? e.oldIndex : -1;
178+
// The source index in this list (onStart fires from the source). Prefer the
179+
// DRAGGABLE-relative index: with non-draggable element children present
180+
// (Lit shadow-DOM `#header`/`#footer` `<slot>`s), plain `e.oldIndex` counts
181+
// all children and is offset — stashing `current[oldIndex]` would capture
182+
// the WRONG item (the row after the one actually grabbed), and the
183+
// identity-based commit lookup would then faithfully move that wrong item.
184+
// `oldDraggableIndex` counts only the `.rozie-sortable-item` rows.
185+
const fromIdx =
186+
typeof e.oldDraggableIndex === 'number'
187+
? e.oldDraggableIndex
188+
: typeof e.oldIndex === 'number'
189+
? e.oldIndex
190+
: -1;
181191
if (fromIdx >= 0 && fromIdx < current.length) {
182192
const item = e.item as unknown as Record<string, unknown> | null;
183193
if (item !== null && typeof item === 'object') {
@@ -225,10 +235,31 @@ export function useSortableJS<T>(
225235
// physically moves the same node between lists).
226236
const stashedItem = readStash<T>(e.item);
227237

238+
// Prefer the DRAGGABLE-relative indices. SortableJS computes plain
239+
// `oldIndex`/`newIndex` via `index(dragEl)` — counting ALL element
240+
// siblings — while `oldDraggableIndex`/`newDraggableIndex` use
241+
// `index(dragEl, options.draggable)`, counting only the
242+
// `.rozie-sortable-item` rows. When the list container also holds
243+
// non-draggable element children, the plain indices are offset. This bites
244+
// ONLY on Lit: the `#header` / `#footer` named slots render as real
245+
// `<slot>` ELEMENTS inside the shadow-DOM list container (the 5 light-DOM
246+
// targets render nothing for an empty named slot), so the leading
247+
// `<slot name="header">` shifts every plain `newIndex` by +1 and the
248+
// writeback dropped items one slot too far. The draggable indices stay
249+
// item-relative everywhere; when no non-draggable children exist the two
250+
// coincide, so the 5 light-DOM targets are unchanged.
228251
const oldIndexHint =
229-
typeof e.oldIndex === 'number' ? e.oldIndex : -1;
252+
typeof e.oldDraggableIndex === 'number'
253+
? e.oldDraggableIndex
254+
: typeof e.oldIndex === 'number'
255+
? e.oldIndex
256+
: -1;
230257
const newIndexHint =
231-
typeof e.newIndex === 'number' ? e.newIndex : -1;
258+
typeof e.newDraggableIndex === 'number'
259+
? e.newDraggableIndex
260+
: typeof e.newIndex === 'number'
261+
? e.newIndex
262+
: -1;
232263

233264
let next: T[];
234265
let oldIndex: number;

packages/ui/sortable-list/packages/react/src/internal/useSortableJS.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,19 @@ export function useSortableJS<T>(
175175
* it on the destination side. */
176176
const handleStart = (e: SortableEvent): void => {
177177
const current = opts.items();
178-
// `e.oldIndex` is the source index in the source list (this list,
179-
// since onStart fires from the source).
180-
const fromIdx = typeof e.oldIndex === 'number' ? e.oldIndex : -1;
178+
// The source index in this list (onStart fires from the source). Prefer the
179+
// DRAGGABLE-relative index: with non-draggable element children present
180+
// (Lit shadow-DOM `#header`/`#footer` `<slot>`s), plain `e.oldIndex` counts
181+
// all children and is offset — stashing `current[oldIndex]` would capture
182+
// the WRONG item (the row after the one actually grabbed), and the
183+
// identity-based commit lookup would then faithfully move that wrong item.
184+
// `oldDraggableIndex` counts only the `.rozie-sortable-item` rows.
185+
const fromIdx =
186+
typeof e.oldDraggableIndex === 'number'
187+
? e.oldDraggableIndex
188+
: typeof e.oldIndex === 'number'
189+
? e.oldIndex
190+
: -1;
181191
if (fromIdx >= 0 && fromIdx < current.length) {
182192
const item = e.item as unknown as Record<string, unknown> | null;
183193
if (item !== null && typeof item === 'object') {
@@ -225,10 +235,31 @@ export function useSortableJS<T>(
225235
// physically moves the same node between lists).
226236
const stashedItem = readStash<T>(e.item);
227237

238+
// Prefer the DRAGGABLE-relative indices. SortableJS computes plain
239+
// `oldIndex`/`newIndex` via `index(dragEl)` — counting ALL element
240+
// siblings — while `oldDraggableIndex`/`newDraggableIndex` use
241+
// `index(dragEl, options.draggable)`, counting only the
242+
// `.rozie-sortable-item` rows. When the list container also holds
243+
// non-draggable element children, the plain indices are offset. This bites
244+
// ONLY on Lit: the `#header` / `#footer` named slots render as real
245+
// `<slot>` ELEMENTS inside the shadow-DOM list container (the 5 light-DOM
246+
// targets render nothing for an empty named slot), so the leading
247+
// `<slot name="header">` shifts every plain `newIndex` by +1 and the
248+
// writeback dropped items one slot too far. The draggable indices stay
249+
// item-relative everywhere; when no non-draggable children exist the two
250+
// coincide, so the 5 light-DOM targets are unchanged.
228251
const oldIndexHint =
229-
typeof e.oldIndex === 'number' ? e.oldIndex : -1;
252+
typeof e.oldDraggableIndex === 'number'
253+
? e.oldDraggableIndex
254+
: typeof e.oldIndex === 'number'
255+
? e.oldIndex
256+
: -1;
230257
const newIndexHint =
231-
typeof e.newIndex === 'number' ? e.newIndex : -1;
258+
typeof e.newDraggableIndex === 'number'
259+
? e.newDraggableIndex
260+
: typeof e.newIndex === 'number'
261+
? e.newIndex
262+
: -1;
232263

233264
let next: T[];
234265
let oldIndex: number;

packages/ui/sortable-list/packages/solid/src/internal/useSortableJS.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,19 @@ export function useSortableJS<T>(
175175
* it on the destination side. */
176176
const handleStart = (e: SortableEvent): void => {
177177
const current = opts.items();
178-
// `e.oldIndex` is the source index in the source list (this list,
179-
// since onStart fires from the source).
180-
const fromIdx = typeof e.oldIndex === 'number' ? e.oldIndex : -1;
178+
// The source index in this list (onStart fires from the source). Prefer the
179+
// DRAGGABLE-relative index: with non-draggable element children present
180+
// (Lit shadow-DOM `#header`/`#footer` `<slot>`s), plain `e.oldIndex` counts
181+
// all children and is offset — stashing `current[oldIndex]` would capture
182+
// the WRONG item (the row after the one actually grabbed), and the
183+
// identity-based commit lookup would then faithfully move that wrong item.
184+
// `oldDraggableIndex` counts only the `.rozie-sortable-item` rows.
185+
const fromIdx =
186+
typeof e.oldDraggableIndex === 'number'
187+
? e.oldDraggableIndex
188+
: typeof e.oldIndex === 'number'
189+
? e.oldIndex
190+
: -1;
181191
if (fromIdx >= 0 && fromIdx < current.length) {
182192
const item = e.item as unknown as Record<string, unknown> | null;
183193
if (item !== null && typeof item === 'object') {
@@ -225,10 +235,31 @@ export function useSortableJS<T>(
225235
// physically moves the same node between lists).
226236
const stashedItem = readStash<T>(e.item);
227237

238+
// Prefer the DRAGGABLE-relative indices. SortableJS computes plain
239+
// `oldIndex`/`newIndex` via `index(dragEl)` — counting ALL element
240+
// siblings — while `oldDraggableIndex`/`newDraggableIndex` use
241+
// `index(dragEl, options.draggable)`, counting only the
242+
// `.rozie-sortable-item` rows. When the list container also holds
243+
// non-draggable element children, the plain indices are offset. This bites
244+
// ONLY on Lit: the `#header` / `#footer` named slots render as real
245+
// `<slot>` ELEMENTS inside the shadow-DOM list container (the 5 light-DOM
246+
// targets render nothing for an empty named slot), so the leading
247+
// `<slot name="header">` shifts every plain `newIndex` by +1 and the
248+
// writeback dropped items one slot too far. The draggable indices stay
249+
// item-relative everywhere; when no non-draggable children exist the two
250+
// coincide, so the 5 light-DOM targets are unchanged.
228251
const oldIndexHint =
229-
typeof e.oldIndex === 'number' ? e.oldIndex : -1;
252+
typeof e.oldDraggableIndex === 'number'
253+
? e.oldDraggableIndex
254+
: typeof e.oldIndex === 'number'
255+
? e.oldIndex
256+
: -1;
230257
const newIndexHint =
231-
typeof e.newIndex === 'number' ? e.newIndex : -1;
258+
typeof e.newDraggableIndex === 'number'
259+
? e.newDraggableIndex
260+
: typeof e.newIndex === 'number'
261+
? e.newIndex
262+
: -1;
232263

233264
let next: T[];
234265
let oldIndex: number;

packages/ui/sortable-list/packages/svelte/src/internal/useSortableJS.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,19 @@ export function useSortableJS<T>(
175175
* it on the destination side. */
176176
const handleStart = (e: SortableEvent): void => {
177177
const current = opts.items();
178-
// `e.oldIndex` is the source index in the source list (this list,
179-
// since onStart fires from the source).
180-
const fromIdx = typeof e.oldIndex === 'number' ? e.oldIndex : -1;
178+
// The source index in this list (onStart fires from the source). Prefer the
179+
// DRAGGABLE-relative index: with non-draggable element children present
180+
// (Lit shadow-DOM `#header`/`#footer` `<slot>`s), plain `e.oldIndex` counts
181+
// all children and is offset — stashing `current[oldIndex]` would capture
182+
// the WRONG item (the row after the one actually grabbed), and the
183+
// identity-based commit lookup would then faithfully move that wrong item.
184+
// `oldDraggableIndex` counts only the `.rozie-sortable-item` rows.
185+
const fromIdx =
186+
typeof e.oldDraggableIndex === 'number'
187+
? e.oldDraggableIndex
188+
: typeof e.oldIndex === 'number'
189+
? e.oldIndex
190+
: -1;
181191
if (fromIdx >= 0 && fromIdx < current.length) {
182192
const item = e.item as unknown as Record<string, unknown> | null;
183193
if (item !== null && typeof item === 'object') {
@@ -225,10 +235,31 @@ export function useSortableJS<T>(
225235
// physically moves the same node between lists).
226236
const stashedItem = readStash<T>(e.item);
227237

238+
// Prefer the DRAGGABLE-relative indices. SortableJS computes plain
239+
// `oldIndex`/`newIndex` via `index(dragEl)` — counting ALL element
240+
// siblings — while `oldDraggableIndex`/`newDraggableIndex` use
241+
// `index(dragEl, options.draggable)`, counting only the
242+
// `.rozie-sortable-item` rows. When the list container also holds
243+
// non-draggable element children, the plain indices are offset. This bites
244+
// ONLY on Lit: the `#header` / `#footer` named slots render as real
245+
// `<slot>` ELEMENTS inside the shadow-DOM list container (the 5 light-DOM
246+
// targets render nothing for an empty named slot), so the leading
247+
// `<slot name="header">` shifts every plain `newIndex` by +1 and the
248+
// writeback dropped items one slot too far. The draggable indices stay
249+
// item-relative everywhere; when no non-draggable children exist the two
250+
// coincide, so the 5 light-DOM targets are unchanged.
228251
const oldIndexHint =
229-
typeof e.oldIndex === 'number' ? e.oldIndex : -1;
252+
typeof e.oldDraggableIndex === 'number'
253+
? e.oldDraggableIndex
254+
: typeof e.oldIndex === 'number'
255+
? e.oldIndex
256+
: -1;
230257
const newIndexHint =
231-
typeof e.newIndex === 'number' ? e.newIndex : -1;
258+
typeof e.newDraggableIndex === 'number'
259+
? e.newDraggableIndex
260+
: typeof e.newIndex === 'number'
261+
? e.newIndex
262+
: -1;
232263

233264
let next: T[];
234265
let oldIndex: number;

packages/ui/sortable-list/packages/vue/src/internal/useSortableJS.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,19 @@ export function useSortableJS<T>(
175175
* it on the destination side. */
176176
const handleStart = (e: SortableEvent): void => {
177177
const current = opts.items();
178-
// `e.oldIndex` is the source index in the source list (this list,
179-
// since onStart fires from the source).
180-
const fromIdx = typeof e.oldIndex === 'number' ? e.oldIndex : -1;
178+
// The source index in this list (onStart fires from the source). Prefer the
179+
// DRAGGABLE-relative index: with non-draggable element children present
180+
// (Lit shadow-DOM `#header`/`#footer` `<slot>`s), plain `e.oldIndex` counts
181+
// all children and is offset — stashing `current[oldIndex]` would capture
182+
// the WRONG item (the row after the one actually grabbed), and the
183+
// identity-based commit lookup would then faithfully move that wrong item.
184+
// `oldDraggableIndex` counts only the `.rozie-sortable-item` rows.
185+
const fromIdx =
186+
typeof e.oldDraggableIndex === 'number'
187+
? e.oldDraggableIndex
188+
: typeof e.oldIndex === 'number'
189+
? e.oldIndex
190+
: -1;
181191
if (fromIdx >= 0 && fromIdx < current.length) {
182192
const item = e.item as unknown as Record<string, unknown> | null;
183193
if (item !== null && typeof item === 'object') {
@@ -225,10 +235,31 @@ export function useSortableJS<T>(
225235
// physically moves the same node between lists).
226236
const stashedItem = readStash<T>(e.item);
227237

238+
// Prefer the DRAGGABLE-relative indices. SortableJS computes plain
239+
// `oldIndex`/`newIndex` via `index(dragEl)` — counting ALL element
240+
// siblings — while `oldDraggableIndex`/`newDraggableIndex` use
241+
// `index(dragEl, options.draggable)`, counting only the
242+
// `.rozie-sortable-item` rows. When the list container also holds
243+
// non-draggable element children, the plain indices are offset. This bites
244+
// ONLY on Lit: the `#header` / `#footer` named slots render as real
245+
// `<slot>` ELEMENTS inside the shadow-DOM list container (the 5 light-DOM
246+
// targets render nothing for an empty named slot), so the leading
247+
// `<slot name="header">` shifts every plain `newIndex` by +1 and the
248+
// writeback dropped items one slot too far. The draggable indices stay
249+
// item-relative everywhere; when no non-draggable children exist the two
250+
// coincide, so the 5 light-DOM targets are unchanged.
228251
const oldIndexHint =
229-
typeof e.oldIndex === 'number' ? e.oldIndex : -1;
252+
typeof e.oldDraggableIndex === 'number'
253+
? e.oldDraggableIndex
254+
: typeof e.oldIndex === 'number'
255+
? e.oldIndex
256+
: -1;
230257
const newIndexHint =
231-
typeof e.newIndex === 'number' ? e.newIndex : -1;
258+
typeof e.newDraggableIndex === 'number'
259+
? e.newDraggableIndex
260+
: typeof e.newIndex === 'number'
261+
? e.newIndex
262+
: -1;
232263

233264
let next: T[];
234265
let oldIndex: number;

0 commit comments

Comments
 (0)