fix(studio): address storyboard review feedback

This commit is contained in:
James
2026-07-16 14:56:11 -04:00
parent 73dd38f93e
commit ded443647d
10 changed files with 291 additions and 53 deletions
@@ -149,14 +149,13 @@ const VIEW_MODE_OPTIONS: Array<{ mode: StudioViewMode; label: string }> = [
];
/** Segmented control switching the main stage between storyboard and preview. */
function ViewModeToggle() {
export function ViewModeToggle() {
const { viewMode, setViewMode } = useViewMode();
const tabRefs = useRef<Array<HTMLButtonElement | null>>([]);
const selectMode = (mode: StudioViewMode) => {
if (mode === viewMode) return;
trackStudioEvent("view_mode_toggle", { mode });
setViewMode(mode);
if (setViewMode(mode)) trackStudioEvent("view_mode_toggle", { mode });
};
// Complete APG tabs pattern: roving tabIndex + arrow-key navigation.
@@ -0,0 +1,47 @@
// @vitest-environment happy-dom
import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import { AgentChatMessageButton } from "./AgentChatMessageButton";
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
afterEach(() => {
document.body.innerHTML = "";
vi.useRealTimers();
vi.restoreAllMocks();
});
describe("AgentChatMessageButton", () => {
it("reports a successful copy and resets to an explicit re-copy action", async () => {
vi.useFakeTimers();
const writeText = vi.fn().mockResolvedValue(undefined);
Object.defineProperty(navigator, "clipboard", {
configurable: true,
value: { writeText },
});
const onCopied = vi.fn();
const host = document.createElement("div");
document.body.append(host);
const root = createRoot(host);
act(() => {
root.render(<AgentChatMessageButton message="handoff" onCopied={onCopied} />);
});
const button = host.querySelector("button");
if (!button) throw new Error("copy button not rendered");
await act(async () => {
button.dispatchEvent(new MouseEvent("click", { bubbles: true }));
await Promise.resolve();
});
expect(writeText).toHaveBeenCalledWith("handoff");
expect(onCopied).toHaveBeenCalledOnce();
expect(button.textContent).toBe("Copied — paste in your agent chat");
act(() => vi.advanceTimersByTime(3000));
expect(button.textContent).toBe("Copy again");
act(() => root.unmount());
});
});
@@ -1,21 +1,31 @@
import { useState } from "react";
import { useEffect, useState } from "react";
import { Button } from "../ui/Button";
export const APPLY_STORYBOARD_FEEDBACK_MESSAGE = "Apply my saved storyboard feedback.";
export const APPLY_STORYBOARD_FEEDBACK_MESSAGE =
"Read the storyboard feedback I saved in .hyperframes/frame-comments.json and revise the frames.";
export function AgentChatMessageButton({
message,
label = "Copy agent message",
label = "Copy prompt for agent",
onCopied,
}: {
message: string;
label?: string;
onCopied?: () => void;
}) {
const [copyState, setCopyState] = useState<"idle" | "copied" | "failed">("idle");
const [copyState, setCopyState] = useState<"idle" | "copied" | "again" | "failed">("idle");
useEffect(() => {
if (copyState !== "copied") return;
const timeout = window.setTimeout(() => setCopyState("again"), 3000);
return () => window.clearTimeout(timeout);
}, [copyState]);
const copyMessage = async () => {
try {
await navigator.clipboard.writeText(message);
setCopyState("copied");
onCopied?.();
} catch {
setCopyState("failed");
}
@@ -24,10 +34,12 @@ export function AgentChatMessageButton({
return (
<Button size="sm" variant="secondary" onClick={() => void copyMessage()}>
{copyState === "copied"
? "Copied — paste in agent chat"
: copyState === "failed"
? "Copy failed"
: label}
? "Copied — paste in your agent chat"
: copyState === "again"
? "Copy again"
: copyState === "failed"
? "Copy failed"
: label}
</Button>
);
}
@@ -35,6 +35,7 @@ export interface StoryboardFrameFocusProps {
commentsSubmitState: CommentsSubmitState;
commentsSubmitError: string | null;
feedbackMessageCopied: boolean;
onFeedbackMessageCopied: () => void;
onSaveFeedback: () => void;
/** Project signature the board was loaded with (busts the poster cache). */
posterVersion?: string;
@@ -67,11 +68,12 @@ export function StoryboardFrameFocus({
commentsSubmitState,
commentsSubmitError,
feedbackMessageCopied,
onFeedbackMessageCopied,
onSaveFeedback,
posterVersion,
}: StoryboardFrameFocusProps) {
const { readProjectFile, writeProjectFile } = useFileManagerContext();
const { setViewMode } = useViewMode();
const { setViewMode, registerViewModeGuard } = useViewMode();
const [draft, setDraft] = useState(frame.voiceover ?? "");
const [savedVoiceover, setSavedVoiceover] = useState(frame.voiceover ?? "");
const [busy, setBusy] = useState(false);
@@ -121,7 +123,12 @@ export function StoryboardFrameFocus({
// dirty. An in-flight save does NOT count as safe: if it fails after unmount
// the error lands on an unmounted component and the draft is silently lost,
// so keep confirming until the save actually lands (dirty clears on success).
const confirmLeave = () => !dirty || window.confirm("Discard unsaved voiceover changes?");
const confirmLeave = useCallback(
() => !dirty || window.confirm("Discard unsaved voiceover changes?"),
[dirty],
);
useEffect(() => registerViewModeGuard(confirmLeave), [confirmLeave, registerViewModeGuard]);
const handleBack = () => {
if (confirmLeave()) onBack();
};
@@ -144,8 +151,8 @@ export function StoryboardFrameFocus({
});
const openInPreview = () => {
if (!setViewMode("timeline")) return;
if (frame.src) onSelectComposition(frame.src);
setViewMode("timeline");
};
return (
@@ -185,7 +192,7 @@ export function StoryboardFrameFocus({
{commentDraftCount > 0
? "Save this batch and copy the message for your agent."
: feedbackMessageCopied
? "Message copied — paste it in agent chat to continue."
? "Message copied — paste it in your terminal or IDE agent chat."
: "The agent has not been notified yet."}
</div>
</div>
@@ -201,7 +208,8 @@ export function StoryboardFrameFocus({
) : (
<AgentChatMessageButton
message={APPLY_STORYBOARD_FEEDBACK_MESSAGE}
label={feedbackMessageCopied ? "Copy again" : "Copy agent message"}
label={feedbackMessageCopied ? "Copy again" : "Copy prompt for agent"}
onCopied={onFeedbackMessageCopied}
/>
)}
</div>
@@ -235,7 +243,10 @@ export function StoryboardFrameFocus({
<section>
<div className="mb-1 flex items-center justify-between">
<h3 className="text-xs font-semibold uppercase tracking-wider text-neutral-400">
<h3
className="text-xs font-semibold uppercase tracking-wider text-neutral-400"
title="Storyboard voiceover is a guide; SCRIPT.md is the final TTS source."
>
🎙 Voiceover <span className="font-normal normal-case text-neutral-600">guide</span>
</h3>
<Button
@@ -259,7 +270,7 @@ export function StoryboardFrameFocus({
{dirty
? "Unsaved changes"
: scriptExists
? "Saved to STORYBOARD.md as a guide. SCRIPT.md owns final narration and TTS."
? "Saved. SCRIPT.md drives final TTS."
: "Saved to STORYBOARD.md. This voiceover guides narration for the frame."}
</p>
{error && <p className="mt-1 text-[11px] text-red-400">{error}</p>}
@@ -282,9 +293,12 @@ export function StoryboardFrameFocus({
{pendingCommentCount > 0 ? (
<div className="mt-2 flex flex-wrap items-center justify-between gap-2 rounded-md border border-sky-900/70 bg-sky-950/20 px-2.5 py-2">
<p className="text-[11px] text-sky-200">
Feedback saved. Next: paste the agent message in chat.
Paste the agent prompt in your terminal or IDE chat.
</p>
<AgentChatMessageButton message={APPLY_STORYBOARD_FEEDBACK_MESSAGE} />
<AgentChatMessageButton
message={APPLY_STORYBOARD_FEEDBACK_MESSAGE}
onCopied={onFeedbackMessageCopied}
/>
</div>
) : (
<p className="mt-1 text-[11px] text-neutral-600">
@@ -112,6 +112,7 @@ export function StoryboardLoaded({
commentsSubmitState={comments.submitState}
commentsSubmitError={comments.submitError}
feedbackMessageCopied={feedbackMessageCopied}
onFeedbackMessageCopied={() => setFeedbackMessageCopied(true)}
onSaveFeedback={() => void saveFeedbackAndCopyMessage()}
posterVersion={data.signature}
/>
@@ -130,6 +131,7 @@ export function StoryboardLoaded({
submitError={comments.submitError}
messageCopied={feedbackMessageCopied}
onSave={() => void saveFeedbackAndCopyMessage()}
onMessageCopied={() => setFeedbackMessageCopied(true)}
/>
)}
</div>
@@ -141,6 +143,7 @@ export function StoryboardLoaded({
frames={data.frames}
draftCount={comments.draftCount}
pendingCount={comments.pending?.length ?? 0}
onFeedbackMessageCopied={() => setFeedbackMessageCopied(true)}
/>
<StoryboardWarnings
warnings={data.warnings}
@@ -177,6 +180,7 @@ function CommentsSubmitBar({
submitError,
messageCopied,
onSave,
onMessageCopied,
}: {
draftCount: number;
pendingCount: number;
@@ -184,6 +188,7 @@ function CommentsSubmitBar({
submitError: string | null;
messageCopied: boolean;
onSave: () => void;
onMessageCopied: () => void;
}) {
return (
<div className="ml-auto flex min-w-0 flex-1 flex-wrap items-center justify-end gap-2 sm:flex-none">
@@ -191,12 +196,13 @@ function CommentsSubmitBar({
<>
<span className="text-xs text-sky-300">
{messageCopied
? "Feedback saved · Message copied — paste it in agent chat."
? "Feedback saved · Message copied — paste it in your terminal or IDE agent chat."
: "Feedback saved · Agent not notified."}
</span>
<AgentChatMessageButton
message={APPLY_STORYBOARD_FEEDBACK_MESSAGE}
label={messageCopied ? "Copy again" : "Copy agent message"}
label={messageCopied ? "Copy again" : "Copy prompt for agent"}
onCopied={onMessageCopied}
/>
</>
)}
@@ -21,7 +21,7 @@ const GUIDE_COPY: Record<StoryboardReviewStage, { eyebrow: string; title: string
"plan-review": {
eyebrow: "Ready for review",
title: "Review the story plan",
body: "Check the sequence, scene direction, and voiceover before visual work begins. Leave frame comments, save them, then reply in agent chat to request changes or approve the plan.",
body: "Check the sequence, scene direction, and voiceover before visual work begins. Leave frame comments, save them, then reply in your terminal or IDE agent chat.",
},
"sketch-in-progress": {
eyebrow: "Build in progress",
@@ -31,7 +31,7 @@ const GUIDE_COPY: Record<StoryboardReviewStage, { eyebrow: string; title: string
"sketch-review": {
eyebrow: "Ready for review",
title: "Review the visual direction",
body: "Check composition, hierarchy, and copy. Save frame comments, then reply in agent chat to request changes or approve the sketches for animation.",
body: "Check composition, hierarchy, and copy. Save frame comments, then reply in your terminal or IDE agent chat.",
},
"animation-in-progress": {
eyebrow: "Build in progress",
@@ -66,7 +66,7 @@ const REVIEW_ACTION_COPY: Record<
},
"final-review": {
body: "Add comments where you want changes. If everything looks right, approve this pass in agent chat.",
approvalMessage: "Approve this final storyboard review.",
approvalMessage: "Approve this final storyboard review and continue to rendering.",
},
};
@@ -110,6 +110,7 @@ export interface StoryboardReviewGuideProps {
frames: StoryboardFrameView[];
draftCount: number;
pendingCount: number;
onFeedbackMessageCopied: () => void;
}
/** Stage-aware instructions plus the explicit Studio → agent handoff. */
@@ -117,6 +118,7 @@ export function StoryboardReviewGuide({
frames,
draftCount,
pendingCount,
onFeedbackMessageCopied,
}: StoryboardReviewGuideProps) {
const summary = deriveStoryboardReviewStage(frames);
const copy = GUIDE_COPY[summary.stage];
@@ -155,7 +157,12 @@ export function StoryboardReviewGuide({
{summary.frameCount > 0 && (
<div className="mt-3 border-t border-neutral-800 pt-3">
<ReviewSteps current={handoffStep} />
<NextAction stage={summary.stage} step={handoffStep} draftCount={draftCount} />
<NextAction
stage={summary.stage}
step={handoffStep}
draftCount={draftCount}
onFeedbackMessageCopied={onFeedbackMessageCopied}
/>
</div>
)}
</section>
@@ -217,27 +224,34 @@ function NextAction({
stage,
step,
draftCount,
onFeedbackMessageCopied,
}: {
stage: StoryboardReviewStage;
step: StoryboardHandoffStep;
draftCount: number;
onFeedbackMessageCopied: () => void;
}) {
if (step === 3) return <AgentHandoffAction />;
if (step === 3) {
return <AgentHandoffAction onFeedbackMessageCopied={onFeedbackMessageCopied} />;
}
if (step === 2) return <SaveFeedbackAction draftCount={draftCount} />;
return <ReviewFramesAction stage={stage} />;
}
function AgentHandoffAction() {
function AgentHandoffAction({ onFeedbackMessageCopied }: { onFeedbackMessageCopied: () => void }) {
return (
<div className="mt-3 flex flex-col gap-3 rounded-md border border-sky-900/70 bg-sky-950/20 px-3 py-2.5 sm:flex-row sm:items-center sm:justify-between">
<div className="max-w-3xl">
<div className="text-xs font-semibold text-sky-200">Next: return to agent chat</div>
<div className="text-xs font-semibold text-sky-200">Next: return to your agent chat</div>
<p className="mt-0.5 text-[11px] text-neutral-400">
Feedback is saved, but the agent has not been notified. Paste this message in chat:
{APPLY_STORYBOARD_FEEDBACK_MESSAGE}
Feedback is saved, but the agent has not been notified. Paste this prompt in your terminal
or IDE agent chat.
</p>
</div>
<AgentChatMessageButton message={APPLY_STORYBOARD_FEEDBACK_MESSAGE} />
<AgentChatMessageButton
message={APPLY_STORYBOARD_FEEDBACK_MESSAGE}
onCopied={onFeedbackMessageCopied}
/>
</div>
);
}
@@ -0,0 +1,143 @@
// @vitest-environment happy-dom
import React, { act } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { ViewModeProvider, useViewModeState } from "../../contexts/ViewModeContext";
import { ViewModeToggle } from "../StudioHeader";
import { StoryboardFrameFocus } from "./StoryboardFrameFocus";
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
vi.mock("../../contexts/FileManagerContext", () => ({
useFileManagerContext: () => ({
readProjectFile: vi.fn(),
writeProjectFile: vi.fn(),
}),
}));
vi.mock("../../utils/studioTelemetry", () => ({
trackStudioEvent: vi.fn(),
}));
vi.mock("./FramePoster", () => ({
FramePoster: () => <div>poster</div>,
posterTime: () => 0,
}));
const onSelectComposition = vi.fn();
function TestApp() {
const viewMode = useViewModeState();
return (
<ViewModeProvider value={viewMode}>
<span data-view-mode={viewMode.viewMode}>{viewMode.viewMode}</span>
<ViewModeToggle />
{viewMode.viewMode === "storyboard" && (
<StoryboardFrameFocus
projectId="project"
storyboardPath="STORYBOARD.md"
frame={{
index: 1,
number: 1,
title: "Opening",
status: "built",
src: "frames/01-opening.html",
srcExists: true,
voiceover: "Original voiceover",
narrative: "",
extra: {},
}}
frameCount={1}
onBack={vi.fn()}
onNavigate={vi.fn()}
onSaved={vi.fn()}
onSelectComposition={onSelectComposition}
scriptExists={false}
commentDraft=""
onCommentDraftChange={vi.fn()}
pendingComment={null}
pendingCommentCount={0}
commentDraftCount={0}
commentsSubmitState="idle"
commentsSubmitError={null}
feedbackMessageCopied={false}
onFeedbackMessageCopied={vi.fn()}
onSaveFeedback={vi.fn()}
/>
)}
</ViewModeProvider>
);
}
function renderApp(): { host: HTMLDivElement; root: Root } {
const host = document.createElement("div");
document.body.append(host);
const root = createRoot(host);
act(() => root.render(<TestApp />));
return { host, root };
}
function makeVoiceoverDirty(host: HTMLElement): void {
const textarea = host.querySelector<HTMLTextAreaElement>(
'textarea[placeholder^="What the narrator says"]',
);
if (!textarea) throw new Error("voiceover textarea not rendered");
const valueSetter = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")?.set;
act(() => {
valueSetter?.call(textarea, "Changed voiceover");
textarea.dispatchEvent(new Event("input", { bubbles: true }));
});
}
function clickButton(host: HTMLElement, label: string): void {
const button = [...host.querySelectorAll("button")].find(
(candidate) => candidate.textContent?.trim() === label,
);
if (!button) throw new Error(`button not found: ${label}`);
act(() => button.dispatchEvent(new MouseEvent("click", { bubbles: true })));
}
beforeEach(() => {
window.history.replaceState({}, "", "/?view=storyboard");
onSelectComposition.mockReset();
});
afterEach(() => {
document.body.innerHTML = "";
vi.restoreAllMocks();
});
describe("dirty storyboard voiceover view-mode guard", () => {
it("guards the header Preview transition on decline and allows it on accept", () => {
const confirm = vi.spyOn(window, "confirm").mockReturnValue(false);
const { host, root } = renderApp();
makeVoiceoverDirty(host);
clickButton(host, "Preview");
expect(host.querySelector("[data-view-mode]")?.textContent).toBe("storyboard");
expect(confirm).toHaveBeenCalledWith("Discard unsaved voiceover changes?");
confirm.mockReturnValue(true);
clickButton(host, "Preview");
expect(host.querySelector("[data-view-mode]")?.textContent).toBe("timeline");
expect(onSelectComposition).not.toHaveBeenCalled();
act(() => root.unmount());
});
it("guards Open in Preview on decline and selects the frame only after accept", () => {
const confirm = vi.spyOn(window, "confirm").mockReturnValue(false);
const { host, root } = renderApp();
makeVoiceoverDirty(host);
clickButton(host, "Open in Preview →");
expect(host.querySelector("[data-view-mode]")?.textContent).toBe("storyboard");
expect(onSelectComposition).not.toHaveBeenCalled();
confirm.mockReturnValue(true);
clickButton(host, "Open in Preview →");
expect(host.querySelector("[data-view-mode]")?.textContent).toBe("timeline");
expect(onSelectComposition).toHaveBeenCalledWith("frames/01-opening.html");
act(() => root.unmount());
});
});
@@ -1,9 +1,5 @@
import { describe, expect, it } from "vitest";
import {
deriveStoryboardHandoffStep,
deriveStoryboardReviewStage,
isReviewReadyStage,
} from "./storyboardReviewStage";
import { deriveStoryboardHandoffStep, deriveStoryboardReviewStage } from "./storyboardReviewStage";
describe("deriveStoryboardReviewStage", () => {
it.each([
@@ -37,16 +33,8 @@ describe("storyboard review handoff", () => {
[0, 0, 1],
[2, 0, 2],
[0, 3, 3],
[1, 3, 3],
[1, 3, 2],
] as const)("maps %i drafts and %i pending comments to step %i", (drafts, pending, step) => {
expect(deriveStoryboardHandoffStep(drafts, pending)).toBe(step);
});
it("only offers approval when a review pass is complete", () => {
expect(isReviewReadyStage("plan-review")).toBe(true);
expect(isReviewReadyStage("sketch-review")).toBe(true);
expect(isReviewReadyStage("final-review")).toBe(true);
expect(isReviewReadyStage("sketch-in-progress")).toBe(false);
expect(isReviewReadyStage("animation-in-progress")).toBe(false);
});
});
@@ -21,15 +21,11 @@ export function deriveStoryboardHandoffStep(
draftCount: number,
pendingCount: number,
): StoryboardHandoffStep {
if (pendingCount > 0) return 3;
if (draftCount > 0) return 2;
if (pendingCount > 0) return 3;
return 1;
}
export function isReviewReadyStage(stage: StoryboardReviewStage): boolean {
return stage === "plan-review" || stage === "sketch-review" || stage === "final-review";
}
/** Derive the board-level review moment from agent-owned frame statuses. */
export function deriveStoryboardReviewStage(
frames: ReadonlyArray<{ status: FrameStatus }>,
@@ -4,6 +4,7 @@ import {
useContext,
useEffect,
useMemo,
useRef,
useState,
type ReactNode,
} from "react";
@@ -17,6 +18,7 @@ import {
* user straight into the storyboard by navigating the tab to `?view=storyboard`.
*/
export type StudioViewMode = "timeline" | "storyboard";
export type ViewModeGuard = (nextMode: StudioViewMode) => boolean;
const VIEW_QUERY_PARAM = "view";
@@ -40,7 +42,9 @@ function writeViewModeToUrl(mode: StudioViewMode): void {
export interface ViewModeValue {
viewMode: StudioViewMode;
setViewMode: (mode: StudioViewMode) => void;
/** Returns false when an active editor vetoes the transition. */
setViewMode: (mode: StudioViewMode) => boolean;
registerViewModeGuard: (guard: ViewModeGuard) => () => void;
}
/**
@@ -49,6 +53,7 @@ export interface ViewModeValue {
*/
export function useViewModeState(): ViewModeValue {
const [viewMode, setMode] = useState<StudioViewMode>(() => readViewModeFromUrl());
const guardsRef = useRef(new Set<ViewModeGuard>());
// Reflect genuine browser back/forward between history entries with a different
// `?view=`. Note: our own writes use `replaceState` (below), which does NOT fire
@@ -63,11 +68,25 @@ export function useViewModeState(): ViewModeValue {
}, []);
const setViewMode = useCallback((mode: StudioViewMode) => {
for (const guard of guardsRef.current) {
if (!guard(mode)) return false;
}
setMode(mode);
writeViewModeToUrl(mode);
return true;
}, []);
return useMemo(() => ({ viewMode, setViewMode }), [viewMode, setViewMode]);
const registerViewModeGuard = useCallback((guard: ViewModeGuard) => {
guardsRef.current.add(guard);
return () => {
guardsRef.current.delete(guard);
};
}, []);
return useMemo(
() => ({ viewMode, setViewMode, registerViewModeGuard }),
[viewMode, setViewMode, registerViewModeGuard],
);
}
const ViewModeContext = createContext<ViewModeValue | null>(null);