From a3c3bda3fa9fffe4ebdd275b6926ac3c81eba527 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 5 Oct 2026 18:45:23 +0900 Subject: [PATCH] feat: add enlarged previews for console images --- .../ConsoleAttachmentImage.browser.spec.ts | 142 ++++++++++++++++ .../console/ConsoleAttachmentImage.svelte | 160 ++++++++++++++++-- 2 files changed, 290 insertions(+), 12 deletions(-) create mode 100644 web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.browser.spec.ts diff --git a/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.browser.spec.ts b/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.browser.spec.ts new file mode 100644 index 000000000..d28c5991b --- /dev/null +++ b/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.browser.spec.ts @@ -0,0 +1,142 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; +import ConsoleAttachmentImage from "./ConsoleAttachmentImage.svelte"; +import { + ConsoleAttachmentFetchError, + loadConsoleAttachment, +} from "./attachment-loader.ts"; + +vi.mock("./attachment-loader.ts", async (importOriginal) => ({ + ...await importOriginal(), + loadConsoleAttachment: vi.fn(), +})); + +const attachment = { + attachment_id: "image-a", + media_type: "image/png", + byte_len: 3, +}; +const blob = new Blob([new Uint8Array([1, 2, 3])], { type: "image/png" }); + +beforeEach(() => { + // Exercise the loader's no-observer fallback deterministically. + vi.stubGlobal("IntersectionObserver", undefined); + vi.mocked(loadConsoleAttachment).mockReset().mockResolvedValue(blob); + let nextUrl = 0; + vi.spyOn(URL, "createObjectURL").mockImplementation(() => `blob:image-${++nextUrl}`); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +test("clicking a loaded image opens a modal using the same blob, without another fetch", async () => { + const view = render(ConsoleAttachmentImage, { attachment, url: "/image/a" }); + const trigger = await view.findByRole("button", { name: "Enlarge tool output image" }); + expect(trigger.getAttribute("type")).toBe("button"); + expect(trigger.getAttribute("aria-haspopup")).toBe("dialog"); + expect(view.queryByRole("dialog")).toBeNull(); + trigger.focus(); + await fireEvent.click(trigger); + + const dialog = view.getByRole("dialog", { name: "Image preview" }) as HTMLDialogElement; + expect(dialog.open).toBe(true); + expect(view.getByAltText("Enlarged tool output attachment").getAttribute("src")) + .toBe(view.getByAltText("Tool output attachment").getAttribute("src")); + expect(document.activeElement).toBe(view.getByRole("button", { name: "Close" })); + expect(loadConsoleAttachment).toHaveBeenCalledTimes(1); + expect(URL.createObjectURL).toHaveBeenCalledTimes(1); + + await fireEvent.click(view.getByAltText("Enlarged tool output attachment")); + expect(dialog.open).toBe(true); + await fireEvent.click(view.getByRole("button", { name: "Close" })); + expect(dialog.open).toBe(false); + expect(document.activeElement).toBe(trigger); + expect(URL.revokeObjectURL).not.toHaveBeenCalled(); +}); + +test("Escape, native cancel, and background close the preview and return focus", async () => { + const view = render(ConsoleAttachmentImage, { attachment, url: "/image/a" }); + const trigger = await view.findByRole("button", { name: "Enlarge tool output image" }); + await fireEvent.click(trigger); + const dialog = view.getByRole("dialog") as HTMLDialogElement; + const consoleShortcut = vi.fn(); + window.addEventListener("keydown", consoleShortcut); + try { + await fireEvent.keyDown(view.getByRole("button", { name: "Close" }), { key: "Escape" }); + expect(dialog.open).toBe(false); + expect(consoleShortcut).not.toHaveBeenCalled(); + expect(document.activeElement).toBe(trigger); + } finally { + window.removeEventListener("keydown", consoleShortcut); + } + + await fireEvent.click(trigger); + await fireEvent(dialog, new Event("cancel", { cancelable: true })); + expect(dialog.open).toBe(false); + expect(document.activeElement).toBe(trigger); + await fireEvent.click(trigger); + await fireEvent.click(view.getByRole("button", { name: "Dismiss image preview" })); + expect(dialog.open).toBe(false); + expect(document.activeElement).toBe(trigger); + expect(loadConsoleAttachment).toHaveBeenCalledTimes(1); +}); + +test("unavailable or failed images do not offer a preview; Retry retains existing behavior", async () => { + const view = render(ConsoleAttachmentImage, { attachment, url: null }); + expect(view.getByText("Image Session is unavailable")).toBeTruthy(); + expect(view.queryByRole("button")).toBeNull(); + vi.mocked(loadConsoleAttachment).mockRejectedValueOnce(new ConsoleAttachmentFetchError("expired", "HTTP 410")); + await view.rerender({ attachment, url: "/image/a" }); + await view.findByText("Image retention has expired"); + expect(view.queryByRole("button", { name: "Enlarge tool output image" })).toBeNull(); + await fireEvent.click(view.getByRole("button", { name: "Retry" })); + await view.findByRole("button", { name: "Enlarge tool output image" }); + expect(loadConsoleAttachment).toHaveBeenCalledTimes(2); +}); + +test("replacing the attachment URL closes its preview and releases the old blob", async () => { + const view = render(ConsoleAttachmentImage, { attachment, url: "/image/a" }); + await fireEvent.click(await view.findByRole("button", { name: "Enlarge tool output image" })); + const oldDialog = view.getByRole("dialog") as HTMLDialogElement; + await view.rerender({ attachment, url: "/image/b" }); + await waitFor(() => expect(view.getByAltText("Tool output attachment").getAttribute("src")).toBe("blob:image-2")); + expect(oldDialog.open).toBe(false); + expect(view.queryByRole("dialog")).toBeNull(); + expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:image-1"); + await fireEvent.click(view.getByRole("button", { name: "Enlarge tool output image" })); + expect(view.getByAltText("Enlarged tool output attachment").getAttribute("src")).toBe("blob:image-2"); + const currentDialog = view.getByRole("dialog") as HTMLDialogElement; + view.unmount(); + expect(currentDialog.open).toBe(false); + expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:image-2"); +}); + +test("a late response from a replaced attachment cannot replace the current preview", async () => { + let resolveOld!: (value: Blob) => void; + vi.mocked(loadConsoleAttachment).mockImplementationOnce(() => new Promise((resolve) => resolveOld = resolve)); + const view = render(ConsoleAttachmentImage, { attachment, url: "/image/a" }); + await waitFor(() => expect(loadConsoleAttachment).toHaveBeenCalledWith("/image/a")); + await view.rerender({ attachment, url: "/image/b" }); + await view.findByRole("button", { name: "Enlarge tool output image" }); + resolveOld(blob); + await Promise.resolve(); + expect(URL.createObjectURL).toHaveBeenCalledTimes(1); + expect(view.getByAltText("Tool output attachment").getAttribute("src")).toBe("blob:image-1"); +}); + +test("unmounting during loading does not create a leaked blob or preview", async () => { + let resolveLoad!: (value: Blob) => void; + vi.mocked(loadConsoleAttachment).mockImplementationOnce(() => new Promise((resolve) => resolveLoad = resolve)); + const view = render(ConsoleAttachmentImage, { attachment, url: "/image/a" }); + await waitFor(() => expect(loadConsoleAttachment).toHaveBeenCalledOnce()); + view.unmount(); + resolveLoad(blob); + await Promise.resolve(); + expect(URL.createObjectURL).not.toHaveBeenCalled(); +}); diff --git a/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.svelte b/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.svelte index 83f7bef54..743aeeefc 100644 --- a/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.svelte +++ b/web/workspace/src/lib/workspace/console/ConsoleAttachmentImage.svelte @@ -13,20 +13,24 @@ let { attachment, url }: Props = $props(); let root: HTMLElement | null = $state(null); let objectUrl = $state(null); - let disposed = false; + let previewDialog: HTMLDialogElement | null = $state(null); + let previewTrigger: HTMLButtonElement | null = $state(null); + let closeButton: HTMLButtonElement | null = $state(null); + let sourceVersion = 0; let status = $state<"idle" | "loading" | "ready" | "missing" | "expired" | "unauthorized" | "network" | "failed">("idle"); async function load(): Promise { if (!url || status === "loading" || status === "ready") return; + const version = sourceVersion; status = "loading"; try { const blob = await loadConsoleAttachment(url); - if (disposed) return; + if (version !== sourceVersion) return; if (objectUrl) URL.revokeObjectURL(objectUrl); objectUrl = URL.createObjectURL(blob); status = "ready"; } catch (error) { - if (disposed) return; + if (version !== sourceVersion) return; status = error instanceof ConsoleAttachmentFetchError ? error.kind : "failed"; } } @@ -44,6 +48,38 @@ } } + function openPreview(): void { + if (!previewDialog || previewDialog.open || status !== "ready") return; + previewDialog.showModal(); + closeButton?.focus({ preventScroll: true }); + } + + function closePreview(): void { + previewDialog?.close(); + previewTrigger?.focus({ preventScroll: true }); + } + + function previewKeydown(event: KeyboardEvent): void { + // Console shortcuts must not run while the modal owns keyboard input. + event.stopPropagation(); + if (event.key === "Escape") { + event.preventDefault(); + closePreview(); + } + } + + $effect(() => { + // A reused Console row must not retain another attachment's preview. + url; + status = "idle"; + objectUrl = null; + return () => { + sourceVersion += 1; + previewDialog?.close(); + if (objectUrl) URL.revokeObjectURL(objectUrl); + }; + }); + $effect(() => { if (!root || !url || status !== "idle") return; if (typeof IntersectionObserver === "undefined") { @@ -59,20 +95,47 @@ observer.observe(root); return () => observer.disconnect(); }); - - $effect(() => () => { - disposed = true; - if (objectUrl) URL.revokeObjectURL(objectUrl); - });
{#if status === "ready" && objectUrl} - Tool output attachment + + { event.preventDefault(); closePreview(); }} + > + + + Enlarged tool output attachment + {:else} {statusText()} {#if url && ["missing", "expired", "unauthorized", "network", "failed"].includes(status)} - + {/if} {/if}
@@ -83,7 +146,20 @@ color: var(--text-muted); font-size: var(--font-size-compact); } - img { + .image-preview-trigger { + display: block; + max-width: 100%; + padding: 0; + border: 0; + border-radius: 0.5rem; + background: transparent; + cursor: zoom-in; + } + .image-preview-trigger:focus-visible { + outline: 2px solid var(--text-strong); + outline-offset: 3px; + } + .thumbnail { display: block; max-width: min(100%, 52rem); max-height: 42rem; @@ -91,7 +167,67 @@ border: 1px solid var(--line); border-radius: 0.5rem; } - button { + .image-preview { + position: fixed; + inset: 0; + box-sizing: border-box; + width: 100%; + height: 100dvh; + max-width: none; + max-height: none; + margin: 0; + padding: max(1rem, env(safe-area-inset-top)) max(1rem, env(safe-area-inset-right)) + max(1rem, env(safe-area-inset-bottom)) max(1rem, env(safe-area-inset-left)); + border: 0; + background: transparent; + overflow: hidden; + overscroll-behavior: contain; + } + .image-preview[open] { + display: grid; + grid-template-rows: auto minmax(0, 1fr); + gap: var(--space-2); + } + .image-preview::backdrop { + background: rgb(0 0 0 / 85%); + } + .image-preview-backdrop { + position: absolute; + inset: 0; + width: 100%; + height: 100%; + padding: 0; + border: 0; + border-radius: 0; + background: transparent; + cursor: zoom-out; + } + .image-preview-close { + position: relative; + justify-self: end; + padding: 0.5rem 1rem; + color: white; + background: #222; + border: 1px solid #888; + border-radius: 0.5rem; + cursor: pointer; + } + .image-preview-close:focus-visible { + outline: 2px solid white; + outline-offset: 3px; + } + .image-preview-full { + position: relative; + display: block; + align-self: center; + justify-self: center; + min-width: 0; + min-height: 0; + max-width: 100%; + max-height: 100%; + object-fit: contain; + } + .retry { margin-left: var(--space-2); }