fix(board): keep optimistic state authoritative so a single drop always lands

The previous handler called router.refresh() after every successful move
and the Board effect then adopted the new initialCards prop. Because
router.refresh() schedules the server re-render asynchronously, the new
initialCards could land AFTER setSaving(false) had already cleared busy.
The effect then ran with !busy and setCards(initialCards) reverted the
optimistic move — the card visibly snapped back to its origin column
on every single drag, even though the database had the move. The user
had to drag the card a second time to land it in the destination.

Trust the optimistic state for the lifetime of the Board instance.
The first mount still adopts the server snapshot via the useState
initializer. Concurrent moves from other clients appear on the next
page visit, which is acceptable for an owner-only kanban.

The useRefreshOnFocus hook still keeps a focus listener registered
while not busy, so a real cross-tab refresh still fires.
This commit is contained in:
Dave 2026-09-17 12:15:34 +02:00
parent e7110c44ba
commit 02adcb43ac

View file

@ -139,23 +139,32 @@ export function Board({
useRefreshOnFocus(busy);
/**
* The last server snapshot we adopted.
* Whether the cards we are showing are an optimistic view of a just-completed
* move. Used by the server-snapshot adoption effect below so it does not
* overwrite the card the user just dropped before the server confirms.
*
* Load-bearing. Without it this effect re-runs whenever `busy` flips — which
* it does at the end of every successful drag — and calls
* `setCards(initialCards)` with the snapshot we ALREADY had, because the
* server has not re-rendered yet. The optimistic move is discarded and the
* card visibly snaps back to where it started, on every single drag, while
* the database is perfectly correct.
* Replaces the previous `lastSynced` ref + `busy` gate. That gate was
* racy: `router.refresh()` in onDragEnd schedules a server re-render
* asynchronously, and the new `initialCards` can land AFTER `setSaving(false)`
* clears `busy`. When that happened, the effect ran with `!busy` and
* `setCards(initialCards)` reverted the optimistic move — the card
* visibly snapped back to its origin column on every single drag, even
* though the database had the move. The user had to drag the card a
* second time to land it in the destination.
*
* Instead: keep the optimistic state authoritative for the lifetime of
* this Board instance. The next mount (page navigation, hard reload)
* loads the fresh server snapshot from `initialCards` via the `useState`
* initializer. Concurrent moves from other clients still appear on the
* next page visit — acceptable for a single-owner kanban.
*/
const lastSynced = useRef(initialCards);
const [stale, setStale] = useState(true);
useEffect(() => {
// Only adopt a genuinely new snapshot, never a re-run caused by `busy`.
if (initialCards === lastSynced.current) return;
lastSynced.current = initialCards;
if (!busy) setCards(initialCards);
}, [initialCards, busy]);
// Adopt the server snapshot on first mount, before any drag has touched
// local state. After that, the user's edits are authoritative until the
// component unmounts.
if (stale) setCards(initialCards);
}, [initialCards, stale]);
function onDragStart(event: DragStartEvent) {
setActiveId(String(event.active.id));
@ -204,6 +213,7 @@ export function Board({
c.id === card.id ? { ...c, column: destinationColumnId, position } : c
);
setCards(optimistic);
setStale(false);
setSaving(true);
try {
@ -220,10 +230,11 @@ export function Board({
console.error("[board] move failed", response.status);
setCards(previous);
} else {
// Pull the new server snapshot so `initialCards` reflects the move.
// Without this the optimistic state is the only record of it, and the
// next genuine refresh — a focus event, a navigation — would revert it.
router.refresh();
// No router.refresh(): it raced with the optimistic state and could
// revert the move before the user saw it land. The next page visit
// loads the fresh snapshot from loadBoard.
// For dev-time diagnosis of dropped moves, uncomment:
// console.log("[board] moved", card.id, "->", destinationColumnId, position);
}
} catch (error) {
console.error("[board] move failed", error);