refactor: keep draft identity in workbench

This commit is contained in:
lda
2026-08-10 02:11:26 +07:00 Verified
parent 6b1398e9f4
commit 070cbe6d7e
4 changed files with 31 additions and 102 deletions
@@ -115,11 +115,13 @@ describe("DraftWorkbench", () => {
expect(screen.getByText("Review needs a route.")).toBeInTheDocument();
});
it("reports the controller's current canonical draft to its route owner", () => {
const onDraftChange = vi.fn();
render(<DraftWorkbench draft={workspace} onDraftChange={onDraftChange} />);
it("renders route identity from the controller's canonical draft", () => {
render(<DraftWorkbench draft={workspace} />);
expect(onDraftChange).toHaveBeenCalledWith(workspace);
const header = screen.getByRole("heading", { name: "Review workflow" }).closest("header");
if (header === null) throw new Error("Expected the workbench identity header.");
expect(within(header).getByText("Revision 2")).toBeInTheDocument();
expect(within(header).getByText("Invalid")).toBeInTheDocument();
});
it("keeps the raw draft collapsed and exposes all deferred actions without handlers", () => {
@@ -13,7 +13,6 @@ type DraftWorkbenchProps = {
readonly draft: DraftWorkspace;
readonly capabilities?: ReadonlyArray<CapabilitySummary>;
readonly initialSelection?: WorkbenchSelection;
readonly onDraftChange?: (draft: DraftWorkspace) => void;
readonly onSelectionChange?: (selection: WorkbenchSelection) => void;
readonly enableNavigationProtection?: boolean;
};
@@ -24,6 +23,12 @@ const MOBILE_BREAKPOINT = 850;
const EMPTY_CAPABILITIES: ReadonlyArray<CapabilitySummary> = [];
const titleFor = (workspace: DraftWorkspace): string =>
workspace.title?.trim() || workspace.workspaceId;
const formatStatus = (status: DraftWorkspace["status"]): string =>
status.charAt(0).toUpperCase() + status.slice(1);
const isMobileViewport = (): boolean =>
typeof window !== "undefined" && window.innerWidth <= MOBILE_BREAKPOINT;
@@ -131,7 +136,6 @@ export const DraftWorkbench = ({
draft,
capabilities = EMPTY_CAPABILITIES,
initialSelection = { kind: "canvas" },
onDraftChange,
onSelectionChange,
enableNavigationProtection = false,
}: DraftWorkbenchProps) => {
@@ -140,9 +144,6 @@ export const DraftWorkbench = ({
const [openSheet, setOpenSheet] = useState<MobileSheet | null>(null);
const paletteTriggerRef = useRef<HTMLButtonElement>(null);
const inspectorTriggerRef = useRef<HTMLButtonElement>(null);
useEffect(() => {
onDraftChange?.(controller.draft);
}, [controller.draft, onDraftChange]);
// Resolve capability details from the controller draft so a newly committed
// node can immediately render its edit form before the route-level loader refreshes.
const graph = projectAuthoringGraph(controller.draft.draft);
@@ -170,6 +171,17 @@ export const DraftWorkbench = ({
return (
<>
{enableNavigationProtection && <DirtyNavigationProtection dirty={controller.dirty} />}
<header className="draft-detail__header">
<p className="workspace-route-pending__eyebrow">Draft authoring workbench</p>
<h1>{titleFor(controller.draft)}</h1>
<p className="draft-detail__workspace-id">{controller.draft.workspaceId}</p>
<p className="draft-detail__status-line">
<span className="draft-workspaces__status" data-status={controller.draft.status}>
{formatStatus(controller.draft.status)}
</span>
<span>Revision {controller.draft.revision}</span>
</p>
</header>
<div
className="draft-workbench"
data-dirty={controller.dirty}
@@ -17,7 +17,6 @@ import type { DraftWorkspaceController } from "./useDraftWorkspace.js";
import { useDraftWorkspace } from "./useDraftWorkspace.js";
const capture = vi.hoisted(() => ({
draftChange: null as ((draft: DraftWorkspace) => void) | null,
renderCount: 0,
}));
@@ -29,7 +28,6 @@ vi.mock("../authoring/DraftWorkbench.js", async () => {
return {
...actual,
DraftWorkbench: (props: ComponentProps<typeof RealDraftWorkbench>) => {
capture.draftChange = props.onDraftChange ?? null;
capture.renderCount += 1;
return <RealDraftWorkbench {...props} />;
},
@@ -167,7 +165,6 @@ const routeElement = () => (
beforeEach(() => {
vi.clearAllMocks();
capture.draftChange = null;
capture.renderCount = 0;
loadedReport = workspace();
loaderPhase = "ready";
@@ -206,7 +203,7 @@ afterEach(() => {
});
describe("DraftDetailRoute authoring freshness", () => {
it("uses real controller mutations, loader replacement, and stale-callback rejection without looping", async () => {
it("uses real controller mutations and loader replacement without synchronization loops", async () => {
const user = userEvent.setup();
const committed = workspace({ revision: 2, status: "valid" });
vi.mocked(authoringClient.updateCapabilityStep).mockResolvedValueOnce(committed);
@@ -217,9 +214,6 @@ describe("DraftDetailRoute authoring freshness", () => {
expect(node).not.toBeNull();
fireEvent.click(node as HTMLElement);
await waitFor(() => expect(screen.getByRole("heading", { name: "collect" })).toBeInTheDocument());
const staleCallback = capture.draftChange;
expect(staleCallback).not.toBeNull();
const retry = screen.getByRole("spinbutton", { name: "Retry" });
await user.clear(retry);
await user.type(retry, "2");
@@ -241,9 +235,6 @@ describe("DraftDetailRoute authoring freshness", () => {
view.rerender(routeElement());
await waitFor(() => expect(within(header()).getByText("Revision 3")).toBeInTheDocument());
expect(within(header()).getByText("Invalid")).toBeInTheDocument();
act(() => staleCallback?.({ ...committed, revision: 99, status: "valid" }));
expect(within(header()).getByText("Revision 3")).toBeInTheDocument();
expect(within(header()).queryByText("Revision 99")).toBeNull();
const refreshRenderCount = capture.renderCount;
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
expect(capture.renderCount).toBe(refreshRenderCount);
@@ -254,9 +245,6 @@ describe("DraftDetailRoute authoring freshness", () => {
loaderPhase = "ready";
view.rerender(routeElement());
await waitFor(() => expect(within(header()).getByText("Revision 3")).toBeInTheDocument());
act(() => staleCallback?.({ ...committed, revision: 100, status: "valid" }));
expect(within(header()).getByText("Revision 3")).toBeInTheDocument();
expect(within(header()).queryByText("Revision 100")).toBeNull();
const reconnectRenderCount = capture.renderCount;
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
expect(capture.renderCount).toBe(reconnectRenderCount);
@@ -264,9 +252,6 @@ describe("DraftDetailRoute authoring freshness", () => {
await user.click(screen.getByRole("button", { name: "Navigate to other workspace" }));
await waitFor(() => expect(screen.getByRole("heading", { name: "Other" })).toBeInTheDocument());
expect(within(header()).getByText("Revision 9")).toBeInTheDocument();
act(() => staleCallback?.({ ...committed, revision: 101, status: "valid" }));
expect(within(header()).getByText("Revision 9")).toBeInTheDocument();
expect(within(header()).queryByText("Revision 101")).toBeNull();
const navigationRenderCount = capture.renderCount;
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
expect(capture.renderCount).toBe(navigationRenderCount);
@@ -1,19 +1,9 @@
import { useCallback, useEffect, useRef, useState } from "react";
import { Link, useParams, useSearchParams } from "react-router-dom";
import type {
DraftWorkspace,
} from "../domain/draft-workspace-models.js";
import { DraftWorkbench } from "../authoring/DraftWorkbench.js";
import type { WorkbenchSelection } from "../authoring/authoring-graph.js";
import { useDraftWorkspace } from "./useDraftWorkspace.js";
import { useCapabilityDiscovery } from "./useCapabilityDiscovery.js";
const titleFor = (workspace: DraftWorkspace): string =>
workspace.title?.trim() || workspace.workspaceId;
const formatStatus = (status: DraftWorkspace["status"]): string =>
status.charAt(0).toUpperCase() + status.slice(1);
export type DraftDetailRouteProps = {
readonly enableNavigationProtection?: boolean;
};
@@ -32,51 +22,6 @@ export const DraftDetailRoute = ({
const capabilities = useCapabilityDiscovery({ loadAllPages: true });
const draft =
drafts.selected?.workspaceId === workspaceId ? drafts.selected : null;
const [authoringDraft, setAuthoringDraft] = useState<DraftWorkspace | null>(null);
const loaderSourceRef = useRef<{
readonly draft: DraftWorkspace | null;
readonly detailPhase: typeof drafts.detailPhase;
readonly workspaceId: string | null;
} | null>(null);
const loaderGenerationRef = useRef(0);
const loaderSource = {
draft,
detailPhase: drafts.detailPhase,
workspaceId,
};
const previousLoaderSource = loaderSourceRef.current;
if (
previousLoaderSource === null ||
previousLoaderSource.draft !== loaderSource.draft ||
previousLoaderSource.detailPhase !== loaderSource.detailPhase ||
previousLoaderSource.workspaceId !== loaderSource.workspaceId
) {
loaderSourceRef.current = loaderSource;
loaderGenerationRef.current += 1;
}
const callbackGeneration = loaderGenerationRef.current;
useEffect(() => {
setAuthoringDraft(draft);
}, [draft, drafts.detailPhase, workspaceId]);
// A successful mutation is displayed immediately; a later loader snapshot
// replaces it, and the generation prevents an old callback crossing freshness boundaries.
const displayedDraft =
draft === null
? null
: authoringDraft?.workspaceId === workspaceId
? authoringDraft
: draft;
const handleDraftChange = useCallback(
(nextDraft: DraftWorkspace): void => {
if (
callbackGeneration === loaderGenerationRef.current &&
nextDraft.workspaceId === workspaceId
) setAuthoringDraft(nextDraft);
},
[callbackGeneration, workspaceId],
);
return (
<div className="draft-detail">
@@ -95,28 +40,13 @@ export const DraftDetailRoute = ({
)}
{drafts.detailPhase === "idle" && <p role="status">Select a draft workspace to inspect.</p>}
{draft && displayedDraft && (
<>
<header className="draft-detail__header">
<p className="workspace-route-pending__eyebrow">Draft authoring workbench</p>
<h1>{titleFor(displayedDraft)}</h1>
<p className="draft-detail__workspace-id">{displayedDraft.workspaceId}</p>
<p className="draft-detail__status-line">
<span className="draft-workspaces__status" data-status={displayedDraft.status}>
{formatStatus(displayedDraft.status)}
</span>
<span>Revision {displayedDraft.revision}</span>
</p>
</header>
<DraftWorkbench
capabilities={capabilities.items}
draft={draft}
enableNavigationProtection={enableNavigationProtection}
initialSelection={initialSelection}
onDraftChange={handleDraftChange}
/>
</>
{draft && (
<DraftWorkbench
capabilities={capabilities.items}
draft={draft}
enableNavigationProtection={enableNavigationProtection}
initialSelection={initialSelection}
/>
)}
</div>
);