Skip to content

Commit fc1502c

Browse files
fix: correct top-drop position to stay at top [4.x] (#114)
* fix: correct top-drop position to stay at top (#110) Dragging a card to the very top of a column made it snap to the bottom. All other drop positions worked. Root cause: Filament's shared x-sortable directive wraps SortableJS and re-inserts the dragged node after items[newDraggableIndex - 1] to work around filamentphp/filament#17402. That branch is skipped when newDraggableIndex === 0, so SortableJS's stale DOM leaks through on top drops. flowforge's handleSortableEnd then read neighbors from event.to.sortable.toArray(), which was still in the old order — producing a payload of (afterCardId = old previous, beforeCardId = null), which the backend correctly interprets as "append to bottom". Fix: extend the insertBefore workaround to newDraggableIndex === 0, then derive neighbors from the normalized DOM (querySelectorAll scoped to parentNode) rather than sortable.toArray(). The backend position math was already correct — confirmed by the new feature test that calls moveCard with a realistic multi-card column and asserts the moved card ends up at the top. Closes #110 * 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. * chore: remove Unreleased section from changelog (release workflow auto-updates)
1 parent f548ff8 commit fc1502c

3 files changed

Lines changed: 64 additions & 18 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: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,35 +26,57 @@ export default function flowforge({state}) {
2626
},
2727

2828
handleSortableEnd(event) {
29-
const newOrder = event.to.sortable.toArray();
30-
let cardId = event.item.getAttribute('x-sortable-item');
29+
const draggedNode = event.item;
30+
const parentNode = event.to;
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);
48+
if (firstItem && firstItem !== draggedNode) {
49+
parentNode.insertBefore(draggedNode, firstItem);
50+
}
51+
}
3152

32-
// Fallback to data-card-id if x-sortable-item is missing (edge case safety)
53+
const cardId = readId(draggedNode);
3354
if (!cardId) {
34-
cardId = event.item.getAttribute('data-card-id');
35-
if (!cardId) {
36-
console.error('Flowforge: Could not determine card ID for move operation');
37-
return;
38-
}
55+
console.error('Flowforge: Could not determine card ID for move operation');
56+
return;
3957
}
4058

41-
const targetColumn = event.to.getAttribute('data-column-id');
59+
const targetColumn = parentNode.getAttribute('data-column-id');
4260
if (!targetColumn) {
4361
console.error('Flowforge: Target column ID is missing');
4462
return;
4563
}
4664

47-
const cardElement = event.item;
48-
49-
this.setCardState(cardElement, true);
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);
69+
const index = Array.prototype.indexOf.call(items, draggedNode);
70+
const afterCardId = index > 0 ? readId(items[index - 1]) : null;
71+
const beforeCardId = index >= 0 && index < items.length - 1
72+
? readId(items[index + 1])
73+
: null;
5074

51-
const cardIndex = newOrder.indexOf(cardId);
52-
const afterCardId = cardIndex > 0 ? newOrder[cardIndex - 1] : null;
53-
const beforeCardId = cardIndex < newOrder.length - 1 ? newOrder[cardIndex + 1] : null;
75+
this.setCardState(draggedNode, true);
5476

5577
this.$wire.moveCard(cardId, targetColumn, afterCardId, beforeCardId)
56-
.then(() => this.setCardState(cardElement, false))
57-
.catch(() => this.setCardState(cardElement, false));
78+
.then(() => this.setCardState(draggedNode, false))
79+
.catch(() => this.setCardState(draggedNode, false));
5880
},
5981

6082
setCardState(cardElement, disabled) {

tests/Feature/LivewireBoardTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,30 @@
6666
->and((float) $movedTask->order_position)->toBeLessThan(65535);
6767
});
6868

69+
test('dragging last card to top places it above first card (regression for #110)', function () {
70+
$first = Task::factory()->inProgress()->withPosition('65535.0000000000')->create(['title' => 'First']);
71+
$second = Task::factory()->inProgress()->withPosition('131070.0000000000')->create(['title' => 'Second']);
72+
$third = Task::factory()->inProgress()->withPosition('196605.0000000000')->create(['title' => 'Third']);
73+
$last = Task::factory()->inProgress()->withPosition('262140.0000000000')->create(['title' => 'Last']);
74+
75+
Livewire::test(TestBoard::class)
76+
->call('moveCard', (string) $last->id, 'in_progress', null, (string) $first->id);
77+
78+
$movedTask = $last->fresh();
79+
expect((float) $movedTask->order_position)->toBeLessThan((float) $first->order_position);
80+
81+
$orderedIds = Task::where('status', 'in_progress')
82+
->orderBy('order_position')
83+
->pluck('id')
84+
->map(fn ($id) => (string) $id)
85+
->toArray();
86+
87+
expect($orderedIds[0])->toBe((string) $last->id)
88+
->and($orderedIds[1])->toBe((string) $first->id)
89+
->and($orderedIds[2])->toBe((string) $second->id)
90+
->and($orderedIds[3])->toBe((string) $third->id);
91+
});
92+
6993
test('moves card to bottom of column', function () {
7094
$existingTask = Task::factory()->inProgress()->withPosition('65535.0000000000')->create();
7195
$taskToMove = Task::factory()->todo()->withPosition('65535.0000000000')->create();

0 commit comments

Comments
 (0)