mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
fix(board): keep non-manual kanban moves smooth by sorting columns client-side
Non-position kanban sorts (priority/date/title) jumped twice on drop: the optimistic move appended the card to its target bucket's tail in the cache while the board trusted that order verbatim, and a settle gate held the card in the source column until the network round-trip — then a settle rebuild dropped it to the column bottom, and the follow-up refetch moved it again to its real slot. Align board with swimlane/gantt: sort each column client-side via sortIssues before building columns, and drop the settle gate on the non-manual drop path so the optimistic patch flows straight through to the correct sorted slot in one frame. The gate (isSettlingRef) is kept for manual reorders, which still need their hand-built local order preserved. The now-dead settleVersion state is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
253
packages/views/issues/components/board-view.test.tsx
Normal file
253
packages/views/issues/components/board-view.test.tsx
Normal file
@@ -0,0 +1,253 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, within, act } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { I18nProvider } from "@multica/core/i18n/react";
|
||||
import type { Issue } from "@multica/core/types";
|
||||
import enCommon from "../../locales/en/common.json";
|
||||
import enIssues from "../../locales/en/issues.json";
|
||||
import { BoardView } from "./board-view";
|
||||
|
||||
const TEST_RESOURCES = { en: { common: enCommon, issues: enIssues } };
|
||||
|
||||
// PRIORITY_ORDER drives sortIssues' "priority" ranking.
|
||||
vi.mock("@multica/core/issues/config", () => ({
|
||||
PRIORITY_ORDER: ["urgent", "high", "medium", "low", "none"],
|
||||
}));
|
||||
|
||||
// MUST be a stable reference: production `useActorName` memoizes its return,
|
||||
// and board-view feeds `getActorName` into a `useMemo` that drives the
|
||||
// columns-rebuild effect. A fresh object per render loops it forever.
|
||||
const { mockActorName } = vi.hoisted(() => ({
|
||||
mockActorName: {
|
||||
getActorName: () => "Mock Actor",
|
||||
getActorInitials: () => "MA",
|
||||
getActorAvatarUrl: () => null,
|
||||
},
|
||||
}));
|
||||
vi.mock("@multica/core/workspace/hooks", () => ({
|
||||
useActorName: () => mockActorName,
|
||||
}));
|
||||
|
||||
// View store — mutable so each test sets the sort it needs.
|
||||
const mockViewState = {
|
||||
grouping: "status" as "status" | "assignee",
|
||||
sortBy: "position" as
|
||||
| "position"
|
||||
| "priority"
|
||||
| "start_date"
|
||||
| "due_date"
|
||||
| "created_at"
|
||||
| "title",
|
||||
sortDirection: "asc" as "asc" | "desc",
|
||||
};
|
||||
vi.mock("@multica/core/issues/stores/view-store-context", () => ({
|
||||
ViewStoreProvider: ({ children }: { children: React.ReactNode }) => children,
|
||||
useViewStore: (selector?: (s: typeof mockViewState) => unknown) =>
|
||||
selector ? selector(mockViewState) : mockViewState,
|
||||
useViewStoreApi: () => ({
|
||||
getState: () => mockViewState,
|
||||
setState: vi.fn(),
|
||||
subscribe: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
const useLoadMoreByStatusMock = vi.fn(() => ({
|
||||
total: 0,
|
||||
loaded: 0,
|
||||
hasMore: false,
|
||||
isLoading: false,
|
||||
loadMore: vi.fn(),
|
||||
}));
|
||||
vi.mock("@multica/core/issues/mutations", () => ({
|
||||
useLoadMoreByStatus: () => useLoadMoreByStatusMock(),
|
||||
useLoadMoreByAssigneeGroup: () => useLoadMoreByStatusMock(),
|
||||
}));
|
||||
|
||||
// Capture the drag handlers so tests can drive drag-end directly.
|
||||
let lastOnDragEnd: ((event: unknown) => void) | null = null;
|
||||
vi.mock("@dnd-kit/core", () => ({
|
||||
DndContext: ({
|
||||
children,
|
||||
onDragEnd,
|
||||
}: {
|
||||
children: React.ReactNode;
|
||||
onDragEnd: (event: unknown) => void;
|
||||
}) => {
|
||||
lastOnDragEnd = onDragEnd;
|
||||
return children;
|
||||
},
|
||||
DragOverlay: () => null,
|
||||
PointerSensor: class {},
|
||||
useSensor: () => ({}),
|
||||
useSensors: () => [],
|
||||
pointerWithin: vi.fn(),
|
||||
closestCenter: vi.fn(),
|
||||
}));
|
||||
|
||||
// Replace heavy column/card rendering with a thin probe that exposes the
|
||||
// per-column ordered issue ids — exactly what BoardView is responsible for.
|
||||
vi.mock("./board-column", () => ({
|
||||
BOARD_CARD_WIDTH: 200,
|
||||
BoardColumn: ({
|
||||
group,
|
||||
issueIds,
|
||||
}: {
|
||||
group: { id: string };
|
||||
issueIds: string[];
|
||||
}) => (
|
||||
<div data-testid={`col-${group.id}`}>
|
||||
{issueIds.map((id) => (
|
||||
<span key={id} data-testid="card">
|
||||
{id}
|
||||
</span>
|
||||
))}
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
vi.mock("./board-card", () => ({ BoardCardContent: () => null }));
|
||||
vi.mock("./hidden-columns-panel", () => ({
|
||||
HiddenColumnsPanel: () => null,
|
||||
HiddenColumnRow: () => null,
|
||||
}));
|
||||
vi.mock("./infinite-scroll-sentinel", () => ({
|
||||
InfiniteScrollSentinel: () => null,
|
||||
}));
|
||||
|
||||
function makeIssue(over: Partial<Issue> & { id: string }): Issue {
|
||||
return {
|
||||
workspace_id: "ws-1",
|
||||
number: 1,
|
||||
identifier: "MUL-1",
|
||||
title: over.id,
|
||||
description: "",
|
||||
status: "todo",
|
||||
priority: "none",
|
||||
assignee_type: null,
|
||||
assignee_id: null,
|
||||
creator_type: "member",
|
||||
creator_id: "user-1",
|
||||
parent_issue_id: null,
|
||||
project_id: null,
|
||||
position: 0,
|
||||
start_date: null,
|
||||
due_date: null,
|
||||
metadata: {},
|
||||
created_at: "2026-01-01T00:00:00Z",
|
||||
updated_at: "2026-01-01T00:00:00Z",
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
function renderBoard(ui: React.ReactNode) {
|
||||
const qc = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false, gcTime: 0 } },
|
||||
});
|
||||
return render(
|
||||
<QueryClientProvider client={qc}>
|
||||
<I18nProvider resources={TEST_RESOURCES} locale="en">
|
||||
{ui}
|
||||
</I18nProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
const VISIBLE = ["todo", "in_progress"] as const;
|
||||
|
||||
describe("BoardView non-manual ordering", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockViewState.grouping = "status";
|
||||
mockViewState.sortBy = "position";
|
||||
mockViewState.sortDirection = "asc";
|
||||
lastOnDragEnd = null;
|
||||
});
|
||||
|
||||
it("orders cards within a column by the active non-position sort", () => {
|
||||
mockViewState.sortBy = "priority";
|
||||
// Out of priority order on purpose; cache/array order is low, urgent, medium.
|
||||
const issues = [
|
||||
makeIssue({ id: "low", status: "todo", priority: "low" }),
|
||||
makeIssue({ id: "urgent", status: "todo", priority: "urgent" }),
|
||||
makeIssue({ id: "medium", status: "todo", priority: "medium" }),
|
||||
];
|
||||
|
||||
const { getByTestId } = renderBoard(
|
||||
<BoardView
|
||||
issues={issues}
|
||||
visibleStatuses={[...VISIBLE]}
|
||||
hiddenStatuses={[]}
|
||||
onMoveIssue={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
const cards = within(getByTestId("col-status:todo")).getAllByTestId("card");
|
||||
expect(cards.map((c) => c.textContent)).toEqual(["urgent", "medium", "low"]);
|
||||
});
|
||||
|
||||
it("places an optimistically-moved card into its sorted slot without waiting for settle", () => {
|
||||
mockViewState.sortBy = "priority";
|
||||
const onMoveIssue = vi.fn();
|
||||
const mover = makeIssue({ id: "mover", status: "todo", priority: "high" });
|
||||
const initial = [
|
||||
mover,
|
||||
makeIssue({ id: "stay-urgent", status: "in_progress", priority: "urgent" }),
|
||||
makeIssue({ id: "stay-low", status: "in_progress", priority: "low" }),
|
||||
];
|
||||
|
||||
const { getByTestId, rerender } = renderBoard(
|
||||
<BoardView
|
||||
issues={initial}
|
||||
visibleStatuses={[...VISIBLE]}
|
||||
hiddenStatuses={[]}
|
||||
onMoveIssue={onMoveIssue}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Drag "mover" from todo into the in_progress column.
|
||||
act(() => {
|
||||
lastOnDragEnd?.({
|
||||
active: { id: "mover" },
|
||||
over: { id: "status:in_progress" },
|
||||
});
|
||||
});
|
||||
|
||||
// The move is requested with no settle callback — the gate is gone.
|
||||
expect(onMoveIssue).toHaveBeenCalledTimes(1);
|
||||
const [movedId, updates, settleCb] = onMoveIssue.mock.calls[0]!;
|
||||
expect(movedId).toBe("mover");
|
||||
expect(updates).toMatchObject({ status: "in_progress" });
|
||||
expect(settleCb).toBeUndefined();
|
||||
|
||||
// Simulate the optimistic cache patch (status flipped) WITHOUT firing any
|
||||
// settle callback. The card must already sit in its sorted slot — between
|
||||
// urgent and low — in the in_progress column.
|
||||
const patched = [
|
||||
makeIssue({ id: "mover", status: "in_progress", priority: "high" }),
|
||||
makeIssue({ id: "stay-urgent", status: "in_progress", priority: "urgent" }),
|
||||
makeIssue({ id: "stay-low", status: "in_progress", priority: "low" }),
|
||||
];
|
||||
rerender(
|
||||
<QueryClientProvider client={new QueryClient()}>
|
||||
<I18nProvider resources={TEST_RESOURCES} locale="en">
|
||||
<BoardView
|
||||
issues={patched}
|
||||
visibleStatuses={[...VISIBLE]}
|
||||
hiddenStatuses={[]}
|
||||
onMoveIssue={onMoveIssue}
|
||||
/>
|
||||
</I18nProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
const todo = within(getByTestId("col-status:todo")).queryAllByTestId("card");
|
||||
expect(todo.map((c) => c.textContent)).toEqual([]);
|
||||
const inProgress = within(getByTestId("col-status:in_progress")).getAllByTestId(
|
||||
"card",
|
||||
);
|
||||
expect(inProgress.map((c) => c.textContent)).toEqual([
|
||||
"stay-urgent",
|
||||
"mover",
|
||||
"stay-low",
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -25,6 +25,7 @@ import { HiddenColumnsPanel, HiddenColumnRow } from "./hidden-columns-panel";
|
||||
import { InfiniteScrollSentinel } from "./infinite-scroll-sentinel";
|
||||
import type { ChildProgress } from "./list-row";
|
||||
import { useT } from "../../i18n";
|
||||
import { sortIssues } from "../utils/sort";
|
||||
import {
|
||||
type DragMoveUpdates,
|
||||
makeKanbanCollision,
|
||||
@@ -141,6 +142,7 @@ export function BoardView({
|
||||
const { t } = useT("issues");
|
||||
const grouping = useViewStore((s) => s.grouping);
|
||||
const sortBy = useViewStore((s) => s.sortBy);
|
||||
const sortDirection = useViewStore((s) => s.sortDirection);
|
||||
const sortFieldKey = sortBy === "created_at" ? "created" : sortBy;
|
||||
const sortLabel = sortBy !== "position"
|
||||
? t(($) => $.board.ordered_by, { field: t(($) => $.display[`sort_${sortFieldKey}` as keyof typeof $.display]) })
|
||||
@@ -156,6 +158,15 @@ export function BoardView({
|
||||
: issues,
|
||||
[assigneeGroups, grouping, issues],
|
||||
);
|
||||
// Client-side sort before building columns, mirroring swimlane/gantt. The
|
||||
// optimistic move appends the issue to its target bucket's tail in the cache;
|
||||
// sorting here drops it straight into its correct slot so non-manual sorts
|
||||
// (priority/date/title) don't jump on settle. For manual sort this is a no-op
|
||||
// ordering (sortIssues "position" branch == the manual order).
|
||||
const sortedGroupedIssues = useMemo(
|
||||
() => sortIssues(groupedIssues, sortBy, sortDirection),
|
||||
[groupedIssues, sortBy, sortDirection],
|
||||
);
|
||||
const hydratedAssigneeGroups = useMemo(() => {
|
||||
if (grouping !== "assignee" || !assigneeGroups) return undefined;
|
||||
const order: Record<string, number> = {
|
||||
@@ -215,22 +226,21 @@ export function BoardView({
|
||||
const [activeIssue, setActiveIssue] = useState<Issue | null>(null);
|
||||
const isDraggingRef = useRef(false);
|
||||
const isSettlingRef = useRef(false);
|
||||
const [settleVersion, setSettleVersion] = useState(0);
|
||||
|
||||
// --- Local columns state ---
|
||||
// Between drags: follows TQ via useEffect.
|
||||
// During drag: local-only, driven by onDragOver/onDragEnd.
|
||||
const [columns, setColumns] = useState<Record<string, string[]>>(() =>
|
||||
buildColumns(groupedIssues, groups, grouping),
|
||||
buildColumns(sortedGroupedIssues, groups, grouping),
|
||||
);
|
||||
const columnsRef = useRef(columns);
|
||||
columnsRef.current = columns;
|
||||
|
||||
useEffect(() => {
|
||||
if (!isDraggingRef.current && !isSettlingRef.current) {
|
||||
setColumns(buildColumns(groupedIssues, groups, grouping));
|
||||
setColumns(buildColumns(sortedGroupedIssues, groups, grouping));
|
||||
}
|
||||
}, [groupedIssues, groups, grouping, settleVersion]);
|
||||
}, [sortedGroupedIssues, groups, grouping]);
|
||||
|
||||
// After a cross-column move, lock for one animation frame so dnd-kit's
|
||||
// collision detection can stabilize before processing the next move.
|
||||
@@ -306,7 +316,7 @@ export function BoardView({
|
||||
setActiveIssue(null);
|
||||
|
||||
const resetColumns = () =>
|
||||
setColumns(buildColumns(groupedIssues, groups, grouping));
|
||||
setColumns(buildColumns(sortedGroupedIssues, groups, grouping));
|
||||
|
||||
if (!over) {
|
||||
resetColumns();
|
||||
@@ -359,11 +369,12 @@ export function BoardView({
|
||||
resetColumns();
|
||||
return;
|
||||
}
|
||||
isSettlingRef.current = true;
|
||||
onMoveIssue(activeId, getMoveUpdates(finalGroup, currentIssue.position), () => {
|
||||
isSettlingRef.current = false;
|
||||
setSettleVersion((v) => v + 1);
|
||||
});
|
||||
// No settle gate here: the optimistic cache patch flows straight through
|
||||
// the (now sorted) buildColumns effect, so the card lands in its correct
|
||||
// sorted slot in the target column in one frame — no "stuck in source
|
||||
// column then jump" lag. The settle gate is only needed for manual
|
||||
// (position) reorders, which keep a hand-built local order below.
|
||||
onMoveIssue(activeId, getMoveUpdates(finalGroup, currentIssue.position));
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -384,7 +395,7 @@ export function BoardView({
|
||||
isSettlingRef.current = false;
|
||||
});
|
||||
},
|
||||
[groupedIssues, groups, grouping, onMoveIssue, groupIds, groupMap, sortBy],
|
||||
[sortedGroupedIssues, groups, grouping, onMoveIssue, groupIds, groupMap, sortBy],
|
||||
);
|
||||
|
||||
return (
|
||||
|
||||
Reference in New Issue
Block a user