fix(kanban): correct reorder and cleanup edge cases

[Mingholy]

Summary

• Convert Kanban column drop gap indexes to final arrayMove indexes to fix forward reorder off-by-one behavior.
• Ignore Enter/Escape while IME composition is active in Kanban add-card and inline title inputs.
• Detach WebSocket handlers during cleanup to avoid stale reconnects after unmount.

Test plan

• bun test web/test/kanbanReorder.test.ts web/test/kanbanKeyboard.test.ts web/test/useKanbanWebSocket.test.ts
• cd web && bunx tsc -b
• bun run build
• Lint: repository has no lint script or ESLint/Biome/Oxlint config.

Screenshots

No static visual delta was captured for this CR; the changed behavior is interaction/runtime-only and covered by focused tests.

[Mingholy]

Code Review: PR #42

Summary

This PR fixes three kanban edge cases:

1. BF2 — Column reorder off-by-one: Forward drag-and-drop of columns landed one position too far right because the raw "gap index" was passed directly to arrayMove without adjusting for the removal of the dragged element from its original position.
2. BF3 — IME composition guard on Escape: The Escape key handler in card-title and add-card inputs was not guarded by isComposing, so pressing Escape during CJK IME composition cancelled the edit instead of cancelling the composition.
3. BF4 — WebSocket cleanup leak: On unmount, close() was called before detaching onclose, so the reconnect setTimeout fired after unmount, creating orphaned WebSocket connections.

Each fix is extracted into a pure, testable utility function with focused unit tests.

Bug Verification

BF2 — off-by-one: Confirmed. With 4 columns [A, B, C, D], dragging A to the right of C (gap index 3):

• Old code: arrayMove([A,B,C,D], 0, 3) = [B, C, D, A] — A ends up after D, not after C.
• New code: gapToColumnFinalIndex(0, 3, 4) = 3 - 1 = 2, arrayMove([A,B,C,D], 0, 2) = [B, C, A, D] — correct.

Backward drags were unaffected (gap index equals arrayMove target), but the fix correctly handles both directions.

BF3 — Escape during composition: Confirmed. The original onKeyDown handler in both KanbanCardView.tsx and KanbanColumn.tsx:

if (e.key === "Enter" && !e.nativeEvent.isComposing) { ... }
if (e.key === "Escape") { /* no composing guard */ }

Escape had no isComposing check. The fix adds an early return when composing, so the IME handles Escape natively (cancelling composition) before the app acts on it.

BF4 — stale reconnect: Confirmed. The original cleanup:

wsRef.current?.close()  // fires onclose → setTimeout(connect, ...)
wsRef.current = null

close() triggers onclose (synchronously or asynchronously), which schedules a reconnect via setTimeout. The new cleanupKanbanWebSocket nulls all event handlers before calling close(), preventing the stale reconnect. For the CONNECTING state, it defers closure to onopen, which is the correct pattern.

Fix Analysis

BF2: gapToColumnFinalIndex(oldIdx, gapIdx, listLength) correctly applies the classic off-by-one adjustment: when dragging forward (oldIdx < gapIdx), removing the source element shifts everything left by one, so the target becomes gapIdx - 1. When dragging backward, no adjustment is needed. The clamp is defensive and correct.

Edge cases verified:

• Adjacent drops (gap right before/after current position) resolve to oldIdx, caught by the if (clampedIdx === oldIdx) return guard.
• The handleDragOver indicator still uses the raw gap index for visual positioning, which is correct — the indicator shows the gap between columns, while gapToColumnFinalIndex converts to arrayMove's element-position semantics.

BF3: The kanbanTextInputKeyAction function is clean and its isComposing parameter, while always false at the call sites (due to the early return in the caller), makes the function self-documenting and independently testable. No concerns.

BF4: The cleanupKanbanWebSocket function handles both CONNECTING and OPEN states correctly. handleKanbanUnexpectedClose extracts the reconnect-with-jitter logic, improving testability via injectable setTimeoutFn and randomFn. The KanbanWebSocketLike interface enables testing without a real WebSocket. Well-structured.

Code Quality

• Good decomposition: all three fixes extract pure functions into separate modules with focused tests.
• Test coverage is thorough: forward/backward moves, clamping, IME composition, WS CONNECTING vs OPEN cleanup, reconnect scheduling.
• Naming is clear: gapToColumnFinalIndex precisely describes the semantic gap-to-arrayMove conversion.
• Minor note: the isComposing parameter on kanbanTextInputKeyAction is effectively dead code at the call sites since all callers do if (e.nativeEvent.isComposing) return before invoking the function with false. This is a reasonable trade-off for testability, but a brief comment at the call site would help future readers understand the two-layer guard.

Security

No security concerns. The changes are purely client-side UI logic (drag indexing, keyboard event handling, WebSocket lifecycle). No auth, data mutation, or injection vectors are affected.

Verdict

LGTM. All three bugs are real, verified in the existing code, and correctly fixed. The extracted utility functions are well-tested and improve maintainability. No regressions introduced.