mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-02 18:13:27 +02:00
feat(issues): show the table's own grid while its first page loads, and end its columns like every other surface
Two loading-state gaps, both in how the table's non-issue rows are modelled. A cold load rendered a single "Loading…" line under a full header, which reads as an empty table rather than a loading one. The surface-level skeleton meant for it was unreachable: use-issue-surface-data hardcodes isLoading to false for table, so the `mode === "table"` branch never ran — and it would not have fitted, drawing rounded bars at p-2 where the table draws an edge-to-edge grid, so switching it on would have traded one wrong state for a layout jump. Placeholders are rows now, rendered through the ordinary cell renderer so they inherit the real column widths, pinning and borders. Everything the table knows before its data — header, toolbar, column layout — is up immediately, and the rows swap in without moving anything. The unreachable surface branch is gone. The end of a column was hand-rolled too, leaving the table the one surface where a failed page read as muted body text rather than an error, where the prompt sat left-aligned against three centred ones, and where reaching the end of a paginated branch said nothing at all. The row carries its state rather than a finished label and renders through ListLoadMoreFooter, the footer Board, List and Swimlane already share — which exists, per its own comment, so those states and their wording stay consistent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -34,12 +34,22 @@ export type IssueTableDisplayRow =
|
||||
hasChildren: boolean;
|
||||
collapsed: boolean;
|
||||
}
|
||||
// Stands in for a row that has not arrived yet. Cold loads render these in
|
||||
// the table's own grid rather than swapping the surface for a generic
|
||||
// placeholder: columns, widths and the header are known before any data is,
|
||||
// so only the rows need standing in for, and the layout does not shift when
|
||||
// they are replaced.
|
||||
| { kind: "skeleton"; key: string }
|
||||
// Carries the state rather than a finished label: the row renders through
|
||||
// ListLoadMoreFooter, the same footer Board / List / Swimlane use, which owns
|
||||
// the wording and the styling for each state.
|
||||
| {
|
||||
kind: "load_more";
|
||||
key: string;
|
||||
label: string;
|
||||
loading: boolean;
|
||||
autoLoad?: boolean;
|
||||
state: "loading" | "has_more" | "error" | "end";
|
||||
// Rows past this many mean the branch actually paginated, which is what
|
||||
// decides whether reaching the end is worth marking.
|
||||
total: number;
|
||||
onLoad?: () => void;
|
||||
};
|
||||
|
||||
|
||||
@@ -67,6 +67,7 @@ import {
|
||||
TableCell,
|
||||
TableRow,
|
||||
} from "@multica/ui/components/ui/table";
|
||||
import { Skeleton } from "@multica/ui/components/ui/skeleton";
|
||||
import { cn } from "@multica/ui/lib/utils";
|
||||
import { ApiError } from "@multica/core/api";
|
||||
import { useWorkspaceId } from "@multica/core/hooks";
|
||||
@@ -137,9 +138,13 @@ import {
|
||||
type IssueTableDisplayRow,
|
||||
} from "./table-view-model";
|
||||
import type { ChildProgress } from "./list-row";
|
||||
import { InfiniteScrollSentinel } from "./infinite-scroll-sentinel";
|
||||
import { ListLoadMoreFooter } from "./list-load-more-footer";
|
||||
import { IssueAgentActivityIndicator } from "./issue-agent-activity-indicator";
|
||||
|
||||
// Enough placeholder rows to cover a typical viewport; the virtualizer only
|
||||
// mounts what fits, so overshooting costs nothing.
|
||||
const SKELETON_ROW_COUNT = 12;
|
||||
|
||||
const SELECT_COLUMN_ID = "__select";
|
||||
const ADD_COLUMN_ID = "__add";
|
||||
|
||||
@@ -1072,6 +1077,12 @@ function IssueTableBodyCell({
|
||||
meta.editingCellKey,
|
||||
meta.setEditingCellKey,
|
||||
);
|
||||
// Placeholder rows go through the ordinary cell renderer so they inherit the
|
||||
// real column widths, pinning and borders — the grid is already correct
|
||||
// before any data arrives, so the rows swap in without shifting anything.
|
||||
if (row.original.kind === "skeleton") {
|
||||
return <Skeleton className="h-3.5 w-full" />;
|
||||
}
|
||||
if (row.original.kind !== "issue") return null;
|
||||
const issueRow = row.original;
|
||||
const issue = issueRow.issue;
|
||||
@@ -1772,9 +1783,8 @@ export function TableView({
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: `${registered ? "loading" : "activate"}:${key}`,
|
||||
label: t(($) => $.table.loading_branch),
|
||||
loading: registered,
|
||||
autoLoad: !registered,
|
||||
state: registered ? "loading" : "has_more",
|
||||
total: 0,
|
||||
onLoad: registered
|
||||
? undefined
|
||||
: () => activateServerBranch(groupKey, parentId, ancestorIds),
|
||||
@@ -1785,8 +1795,8 @@ export function TableView({
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: `loading:${key}`,
|
||||
label: t(($) => $.table.loading_branch),
|
||||
loading: true,
|
||||
state: "loading",
|
||||
total: 0,
|
||||
});
|
||||
}
|
||||
for (const row of data.rows) {
|
||||
@@ -1815,8 +1825,8 @@ export function TableView({
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: `retry:${key}`,
|
||||
label: t(($) => $.table.load_more_failed_retry),
|
||||
loading: false,
|
||||
state: "error",
|
||||
total: data.total,
|
||||
onLoad: () => retryServerBranch(key),
|
||||
});
|
||||
} else if (data.nextCursor) {
|
||||
@@ -1824,11 +1834,20 @@ export function TableView({
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: `more:${key}:${nextCursor}`,
|
||||
label: t(($) => $.table.load_more),
|
||||
loading: data.loading,
|
||||
autoLoad: true,
|
||||
state: data.loading ? "loading" : "has_more",
|
||||
total: data.total,
|
||||
onLoad: () => loadNextServerBranchPage(key, nextCursor),
|
||||
});
|
||||
} else if (data.rows.length > 0) {
|
||||
// Reaching the end is only worth marking on a branch that paginated;
|
||||
// the footer applies that rule, so the row is pushed unconditionally
|
||||
// and carries the total for it to judge by.
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: `end:${key}`,
|
||||
state: "end",
|
||||
total: data.total,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1855,27 +1874,40 @@ export function TableView({
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: "loading:groups",
|
||||
label: t(($) => $.table.loading_branch),
|
||||
loading: true,
|
||||
state: "loading",
|
||||
total: 0,
|
||||
});
|
||||
} else if (usesServerGrouping && serverGroupsError) {
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: "retry:groups",
|
||||
label: t(($) => $.table.load_failed_retry),
|
||||
loading: false,
|
||||
state: "error",
|
||||
total: 0,
|
||||
onLoad: () => void refetchServerGroups(),
|
||||
});
|
||||
} else if (usesServerGrouping && hasNextServerGroupPage) {
|
||||
result.push({
|
||||
kind: "load_more",
|
||||
key: "more:groups",
|
||||
label: t(($) => $.table.load_more),
|
||||
loading: fetchingNextServerGroupPage,
|
||||
autoLoad: true,
|
||||
state: fetchingNextServerGroupPage ? "loading" : "has_more",
|
||||
total: 0,
|
||||
onLoad: () => void fetchNextServerGroupPage(),
|
||||
});
|
||||
}
|
||||
|
||||
// Nothing has landed yet and something is still in flight: show the grid
|
||||
// filled with placeholders instead of one "Loading…" line, which reads as
|
||||
// an empty table more than a loading one.
|
||||
const isColdLoad =
|
||||
!result.some((row) => row.kind === "issue") &&
|
||||
result.some((row) => row.kind === "load_more" && row.state === "loading");
|
||||
if (isColdLoad) {
|
||||
return Array.from({ length: SKELETON_ROW_COUNT }, (_, index) => ({
|
||||
kind: "skeleton" as const,
|
||||
key: `skeleton:${index}`,
|
||||
}));
|
||||
}
|
||||
|
||||
return result;
|
||||
}, [
|
||||
collapsedGroupSet,
|
||||
@@ -2391,32 +2423,27 @@ export function TableView({
|
||||
<TableRow className="hover:bg-transparent">
|
||||
<TableCell
|
||||
colSpan={table.getVisibleLeafColumns().length}
|
||||
className="relative h-9 px-4 py-1"
|
||||
className="p-0"
|
||||
>
|
||||
{loadMoreRow.autoLoad &&
|
||||
loadMoreRow.onLoad &&
|
||||
!loadMoreRow.loading && (
|
||||
<InfiniteScrollSentinel
|
||||
onVisible={loadMoreRow.onLoad}
|
||||
loading={false}
|
||||
rootMargin="240px"
|
||||
className="absolute inset-y-0 left-0 w-px"
|
||||
{/* The same footer Board / List / Swimlane end their
|
||||
* columns with. Hand-rolling it here had left the table
|
||||
* as the one surface where a failed page read as muted
|
||||
* body text rather than an error, and where reaching the
|
||||
* end of a paginated branch said nothing at all. The row
|
||||
* only supplies the cell it lives in. */}
|
||||
<div className="sticky left-0 w-full">
|
||||
<ListLoadMoreFooter
|
||||
hasMore={
|
||||
loadMoreRow.state === "loading" ||
|
||||
loadMoreRow.state === "has_more"
|
||||
}
|
||||
isLoading={loadMoreRow.state === "loading"}
|
||||
total={loadMoreRow.total}
|
||||
onLoadMore={() => loadMoreRow.onLoad?.()}
|
||||
isError={loadMoreRow.state === "error"}
|
||||
onRetry={loadMoreRow.onLoad}
|
||||
/>
|
||||
)}
|
||||
<button
|
||||
type="button"
|
||||
disabled={loadMoreRow.loading || !loadMoreRow.onLoad}
|
||||
onClick={(event) => {
|
||||
event.stopPropagation();
|
||||
loadMoreRow.onLoad?.();
|
||||
}}
|
||||
className="sticky left-4 flex items-center gap-2 text-xs text-muted-foreground enabled:hover:text-foreground disabled:cursor-default"
|
||||
>
|
||||
{loadMoreRow.loading && (
|
||||
<Loader2 className="size-3.5 animate-spin" />
|
||||
)}
|
||||
{loadMoreRow.label}
|
||||
</button>
|
||||
</div>
|
||||
</TableCell>
|
||||
</TableRow>
|
||||
);
|
||||
|
||||
@@ -430,6 +430,33 @@ describe("IssueSurface — table pagination ownership", () => {
|
||||
listSquads: vi.fn(() => Promise.resolve([])),
|
||||
} as unknown as ApiClient);
|
||||
|
||||
// Continuation is driven by the shared footer's sentinel, the same one
|
||||
// Board / List / Swimlane use — there is no manual button to press, so the
|
||||
// observer has to actually report the footer as visible.
|
||||
vi.stubGlobal(
|
||||
"IntersectionObserver",
|
||||
class {
|
||||
private readonly callback: IntersectionObserverCallback;
|
||||
constructor(callback: IntersectionObserverCallback) {
|
||||
this.callback = callback;
|
||||
}
|
||||
observe(target: Element) {
|
||||
this.callback(
|
||||
[{ isIntersecting: true, target } as IntersectionObserverEntry],
|
||||
this as unknown as IntersectionObserver,
|
||||
);
|
||||
}
|
||||
unobserve() {}
|
||||
disconnect() {}
|
||||
takeRecords() {
|
||||
return [];
|
||||
}
|
||||
root = null;
|
||||
rootMargin = "0px";
|
||||
thresholds = [0];
|
||||
},
|
||||
);
|
||||
|
||||
render(
|
||||
<QueryClientProvider client={qc}>
|
||||
<IssueSurface
|
||||
@@ -442,12 +469,6 @@ describe("IssueSurface — table pagination ownership", () => {
|
||||
);
|
||||
|
||||
await screen.findByText("First cursor row");
|
||||
const loadMoreButton = document.querySelector<HTMLButtonElement>(
|
||||
"tbody button.sticky",
|
||||
);
|
||||
expect(loadMoreButton).not.toBeNull();
|
||||
fireEvent.click(loadMoreButton!);
|
||||
|
||||
await screen.findByText("Second cursor row");
|
||||
expect(listIssueTableRows).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ page: { limit: 50, cursor: "cursor-2" } }),
|
||||
|
||||
@@ -327,19 +327,16 @@ function IssueSurfaceContent({
|
||||
);
|
||||
}
|
||||
|
||||
// Table is deliberately absent. It owns its own placeholders, drawn as rows
|
||||
// inside its real grid so the header, column widths and toolbar are up before
|
||||
// any data is — a surface-level stand-in would replace all of that with bars
|
||||
// of a different shape and then jump when the rows arrived.
|
||||
function IssueSurfaceSkeleton({ mode }: { mode: string }) {
|
||||
if (mode === "list" || mode === "table") {
|
||||
if (mode === "list") {
|
||||
return (
|
||||
<div className="flex min-h-0 flex-1 flex-col gap-1 overflow-y-auto p-2">
|
||||
{mode === "table" && <Skeleton className="mb-1 h-8 w-full" />}
|
||||
{Array.from({ length: mode === "table" ? 8 : 4 }).map((_, i) => (
|
||||
<Skeleton
|
||||
key={i}
|
||||
className={cn(
|
||||
"w-full",
|
||||
mode === "table" ? "h-9 rounded-sm" : "h-10 rounded-lg",
|
||||
)}
|
||||
/>
|
||||
{Array.from({ length: 4 }).map((_, i) => (
|
||||
<Skeleton key={i} className="h-10 w-full rounded-lg" />
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user