Skip to content

Commit 25103ea

Browse files
fix(web): keep submitted annotations through picker cancellation
Once the guest submits an annotation the pick is complete, so a second picker click or tab change during screenshot conversion no longer discards it. Co-Authored-By: Claude Fable 5 <[email protected]>
1 parent 0a9ca78 commit 25103ea

3 files changed

Lines changed: 64 additions & 10 deletions

File tree

‎apps/web/src/components/ChatView.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4542,6 +4542,9 @@ export default function ChatView(props: ChatViewProps) {
45424542
(surfaces: readonly RightPanelSurface[]) => {
45434543
if (!activeThreadRef) return;
45444544
for (const surface of surfaces) {
4545+
// Without preview access only the local surface goes away. The
4546+
// server record is in-memory bookkeeping, not a live view, and stays
4547+
// listed until a client with the scope closes it or the server restarts.
45454548
if (canOperatePreview && surface.kind === "preview" && surface.resourceId) {
45464549
void closePreviewSession({
45474550
closePreview,

‎apps/web/src/components/preview/PreviewView.test.tsx‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -580,12 +580,11 @@ describe("PreviewView navigation", () => {
580580
createdAt: "2026-09-05T00:00:00.000Z",
581581
};
582582
const onSendAnnotation = vi.fn();
583-
mocks.pickElement.mockResolvedValue({ annotation, submission: "send" });
584-
let finishCapture: (() => void) | undefined;
585-
mocks.capturePreviewAnnotationScreenshot.mockImplementationOnce(
583+
let submitPick: (() => void) | undefined;
584+
mocks.pickElement.mockImplementationOnce(
586585
() =>
587586
new Promise((resolve) => {
588-
finishCapture = () => resolve({ status: "none" });
587+
submitPick = () => resolve({ annotation, submission: "send" });
589588
}),
590589
);
591590
const document = installTestDom();
@@ -602,19 +601,20 @@ describe("PreviewView navigation", () => {
602601
try {
603602
await act(async () => root.render(view()));
604603
await act(async () => mocks.toggleAnnotation?.());
605-
expect(finishCapture).toBeDefined();
604+
expect(submitPick).toBeDefined();
606605
expect(mocks.setAnnotationSendEnabled).toHaveBeenLastCalledWith(TEST_RUNTIME_TAB_ID, true);
607606

608607
mocks.readEnvironmentScope.mockImplementation(
609608
(environmentId?: unknown) => environmentId !== TEST_THREAD_REF.environmentId,
610609
);
611610
await act(async () => root.render(view()));
612611
expect(mocks.setAnnotationSendEnabled).toHaveBeenLastCalledWith(TEST_RUNTIME_TAB_ID, false);
613-
await act(async () => finishCapture!());
612+
await act(async () => submitPick!());
614613
expect(mocks.addPreviewAnnotation).toHaveBeenCalledWith(TEST_THREAD_REF, annotation);
615614
expect(onSendAnnotation).not.toHaveBeenCalled();
616615

617616
mocks.readEnvironmentScope.mockReturnValue(true);
617+
mocks.pickElement.mockResolvedValue({ annotation, submission: "send" });
618618
await act(async () => root.render(view()));
619619
await act(async () => mocks.toggleAnnotation?.());
620620
expect(mocks.setAnnotationSendEnabled).toHaveBeenLastCalledWith(TEST_RUNTIME_TAB_ID, true);
@@ -625,6 +625,48 @@ describe("PreviewView navigation", () => {
625625
}
626626
});
627627

628+
it("keeps a submitted annotation when the picker is toggled during capture", async () => {
629+
const annotation = {
630+
id: "annotation-submitted",
631+
pageUrl: "https://example.com/dashboard",
632+
pageTitle: "Dashboard",
633+
comment: "Tighten this spacing",
634+
elements: [],
635+
regions: [],
636+
strokes: [],
637+
styleChanges: [],
638+
screenshot: null,
639+
createdAt: "2026-09-05T00:00:00.000Z",
640+
};
641+
mocks.pickElement.mockResolvedValue({ annotation, submission: "attach" });
642+
let finishCapture: (() => void) | undefined;
643+
mocks.capturePreviewAnnotationScreenshot.mockImplementationOnce(
644+
() =>
645+
new Promise((resolve) => {
646+
finishCapture = () => resolve({ status: "none" });
647+
}),
648+
);
649+
const document = installTestDom();
650+
const { createRoot } = await import("react-dom/client");
651+
const root = createRoot(document.createElement("div") as unknown as Element);
652+
try {
653+
await act(async () =>
654+
root.render(<PreviewView threadRef={TEST_THREAD_REF} tabId="tab-1" visible />),
655+
);
656+
await act(async () => mocks.toggleAnnotation?.());
657+
expect(finishCapture).toBeDefined();
658+
// The pick is no longer active once the guest has submitted, so this
659+
// starts a new pick instead of cancelling the submitted one.
660+
await act(async () => mocks.toggleAnnotation?.());
661+
expect(mocks.cancelPickElement).not.toHaveBeenCalled();
662+
await act(async () => finishCapture!());
663+
expect(mocks.addPreviewAnnotation).toHaveBeenCalledWith(TEST_THREAD_REF, annotation);
664+
} finally {
665+
await act(async () => root.unmount());
666+
vi.unstubAllGlobals();
667+
}
668+
});
669+
628670
it("does not reopen a cancelled picker after the permission update finishes", async () => {
629671
let finishUpdate: (() => void) | undefined;
630672
mocks.setAnnotationSendEnabled.mockImplementationOnce(

‎apps/web/src/components/preview/PreviewView.tsx‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,7 @@ export function PreviewView({
594594
const previouslyFocused =
595595
typeof document !== "undefined" ? (document.activeElement as HTMLElement | null) : null;
596596
const pickRequest = { cancelled: false };
597+
let submitted = false;
597598
pickActiveRef.current = pickRequest;
598599
setPickActive(true);
599600
void (async () => {
@@ -606,14 +607,21 @@ export function PreviewView({
606607
if (pickRequest.cancelled) return;
607608
const result = await previewBridge.pickElement(runtimeTabId);
608609
if (!result || pickRequest.cancelled) return;
610+
// The user has submitted. Nothing that happens after this point (a
611+
// second picker click, a tab change, unmount) may discard it, so the
612+
// pick stops being cancellable here rather than in `finally`.
613+
if (pickActiveRef.current === pickRequest) {
614+
pickActiveRef.current = null;
615+
if (isMountedRef.current) setPickActive(false);
616+
submitted = true;
617+
}
609618
const { annotation: picked, submission, screenshotFailed = false } = result;
610619
// The structured annotation is still sendable when its optional crop
611620
// stalls or fails, so tell the user what they lost and keep going
612621
// instead of holding the composer for an attachment that never lands.
613622
// The stored copy drops the screenshot on failure, otherwise the prompt
614623
// would tell the agent a crop is attached when none was sent.
615624
const capture = await capturePreviewAnnotationScreenshot(picked);
616-
if (pickRequest.cancelled) return;
617625
// Main reports a crop that failed or timed out on its side; the local
618626
// conversion can fail too. Either way the user should hear about it.
619627
const cropDropped = screenshotFailed || capture.status === "failed";
@@ -655,9 +663,10 @@ export function PreviewView({
655663
} catch {
656664
// Picker failed (e.g. webview navigated). Treat as silent cancel.
657665
} finally {
666+
// A submitted pick already released itself above; a cancelled or
667+
// failed one releases here. Avoid `setState on unmounted component`
668+
// if the panel/thread closed while the pick was in flight.
658669
const isCurrentPick = pickActiveRef.current === pickRequest;
659-
// Avoid `setState on unmounted component` if the panel/thread closed
660-
// while the pick was in flight.
661670
if (isCurrentPick) {
662671
pickActiveRef.current = null;
663672
if (isMountedRef.current) setPickActive(false);
@@ -666,7 +675,7 @@ export function PreviewView({
666675
// pick stole it into the guest webContents. Skip if the previously-
667676
// focused element was unmounted or is no longer focusable.
668677
if (
669-
isCurrentPick &&
678+
(isCurrentPick || submitted) &&
670679
previouslyFocused &&
671680
previouslyFocused.isConnected &&
672681
typeof previouslyFocused.focus === "function"

0 commit comments

Comments
 (0)