Skip to content

Commit 2f1d30e

Browse files
committed
refactor: make handleSortableEnd robust to SortableJS and selector variants
Addresses Copilot review feedback on #110 fix: 1. Fall back to event.newIndex when event.newDraggableIndex is absent — some SortableJS builds / event shims only expose newIndex. 2. Include [data-card-id] in the normalization selector and neighbor query, symmetrical with the data-card-id fallback already used for the dragged card's own id. Without this, cards that only expose data-card-id would silently fall through to (null, null), which the backend treats as "move to bottom". Browser-verified: top drop still lands at top when the event only carries newIndex, and when cards only expose data-card-id.
1 parent 54104e4 commit 2f1d30e

2 files changed

Lines changed: 25 additions & 17 deletions

File tree

resources/dist/flowforge.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

resources/js/flowforge.js

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,29 @@ export default function flowforge({state}) {
2828
handleSortableEnd(event) {
2929
const draggedNode = event.item;
3030
const parentNode = event.to;
31-
const newDraggableIndex = event.newDraggableIndex;
32-
33-
// Filament's shared x-sortable directive re-inserts the dragged node
34-
// after items[newDraggableIndex - 1] to work around filamentphp/filament#17402,
35-
// but that branch is skipped when newDraggableIndex === 0 — leaving the
36-
// DOM stale on top drops. Extend the workaround so the top position is
37-
// normalized before we read neighbors.
38-
if (newDraggableIndex === 0) {
39-
const firstItem = parentNode.querySelector(':scope > [x-sortable-item]');
31+
// SortableJS exposes both newIndex (across all children) and
32+
// newDraggableIndex (draggable children only). Prefer the draggable
33+
// index where available and fall back to newIndex for older or
34+
// alternative SortableJS builds.
35+
const dropIndex = event.newDraggableIndex ?? event.newIndex;
36+
37+
const itemSelector = ':scope > [x-sortable-item], :scope > [data-card-id]';
38+
const readId = (el) => el.getAttribute('x-sortable-item') || el.getAttribute('data-card-id');
39+
40+
// Filament's shared x-sortable directive re-inserts the dragged
41+
// node after items[dropIndex - 1] to work around
42+
// filamentphp/filament#17402, but that branch is skipped when
43+
// dropIndex === 0 — leaving the DOM stale on top drops. Extend
44+
// the workaround so the top position is normalized before we
45+
// read neighbors.
46+
if (dropIndex === 0) {
47+
const firstItem = parentNode.querySelector(itemSelector);
4048
if (firstItem && firstItem !== draggedNode) {
4149
parentNode.insertBefore(draggedNode, firstItem);
4250
}
4351
}
4452

45-
let cardId = draggedNode.getAttribute('x-sortable-item')
46-
|| draggedNode.getAttribute('data-card-id');
53+
const cardId = readId(draggedNode);
4754
if (!cardId) {
4855
console.error('Flowforge: Could not determine card ID for move operation');
4956
return;
@@ -55,13 +62,14 @@ export default function flowforge({state}) {
5562
return;
5663
}
5764

58-
// Derive neighbors from the normalized DOM rather than sortable.toArray(),
59-
// which can return stale order when SortableJS leaves the DOM out of sync.
60-
const items = parentNode.querySelectorAll(':scope > [x-sortable-item]');
65+
// Derive neighbors from the normalized DOM rather than
66+
// sortable.toArray(), which can return stale order when SortableJS
67+
// leaves the DOM out of sync.
68+
const items = parentNode.querySelectorAll(itemSelector);
6169
const index = Array.prototype.indexOf.call(items, draggedNode);
62-
const afterCardId = index > 0 ? items[index - 1].getAttribute('x-sortable-item') : null;
70+
const afterCardId = index > 0 ? readId(items[index - 1]) : null;
6371
const beforeCardId = index >= 0 && index < items.length - 1
64-
? items[index + 1].getAttribute('x-sortable-item')
72+
? readId(items[index + 1])
6573
: null;
6674

6775
this.setCardState(draggedNode, true);

0 commit comments

Comments
 (0)