From b4fccaa8c46f4e3eede7db5bfca09d3fae8b09b7 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 30 Jun 2026 09:53:40 -0500 Subject: [PATCH] [codex] Unify issue media attachment gallery (#8785) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Issue detail pages and run output cards both render user-facing attachment previews > - Media attachments were handled through separate gallery entry points, so attachments from issue comments and issue output did not share one consistent viewing path > - A shared gallery path makes image browsing more predictable across the issue surface > - This pull request unifies the issue media attachment gallery behavior across attachments, output cards, and issue detail rendering > - The benefit is a more consistent image preview experience with focused regression coverage around the affected UI paths ## Linked Issues or Issue Description Refs #8788 This PR fixes inconsistent issue media preview behavior across attachment and output surfaces. ## What Changed - Unified issue media attachment gallery behavior across issue attachments, issue output, output primary cards, and issue detail rendering. - Added image gallery modal coverage and regression tests for attachments, output image handling, keyboard navigation, and issue detail interactions. - Preserved the primary video output Open action while still allowing gallery browsing when a gallery handler is available. - Updated issue attachment and issue output helper logic to preserve shared image gallery metadata. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/ImageGalleryModal.test.tsx src/components/issue-output/IssueOutputSection.test.tsx src/components/IssueAttachmentsSection.test.tsx src/pages/IssueDetail.test.tsx` - Result: 4 files passed, 47 tests passed. ## Risks Low to medium risk. The change is UI-scoped but touches shared issue attachment/output rendering paths, so regressions would most likely appear as image/video preview ordering, missing gallery entries, or incorrect non-image attachment behavior. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5 coding agent session with shell and git/GitHub CLI tool use. Exact serving revision and context window were not exposed by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ui/src/components/ImageGalleryModal.test.tsx | 134 ++++++++++++++++++ ui/src/components/ImageGalleryModal.tsx | 83 +++++++---- .../IssueAttachmentsSection.test.tsx | 34 +++++ ui/src/components/IssueAttachmentsSection.tsx | 25 +++- .../issue-output/IssueOutputSection.test.tsx | 55 +++++++ .../issue-output/IssueOutputSection.tsx | 118 ++++++++++++++- .../issue-output/OutputPrimaryCard.tsx | 68 ++++++--- ui/src/lib/issue-attachments.ts | 16 +-- ui/src/lib/issue-output.ts | 21 +++ ui/src/pages/IssueDetail.test.tsx | 5 +- ui/src/pages/IssueDetail.tsx | 76 ++++++++-- .../stories/dialogs-modals.stories.tsx | 2 +- 12 files changed, 557 insertions(+), 80 deletions(-) create mode 100644 ui/src/components/ImageGalleryModal.test.tsx diff --git a/ui/src/components/ImageGalleryModal.test.tsx b/ui/src/components/ImageGalleryModal.test.tsx new file mode 100644 index 0000000000..6fbf0b49d2 --- /dev/null +++ b/ui/src/components/ImageGalleryModal.test.tsx @@ -0,0 +1,134 @@ +// @vitest-environment jsdom + +import { createRoot, type Root } from "react-dom/client"; +import { flushSync } from "react-dom"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ImageGalleryModal, type GalleryMediaItem } from "./ImageGalleryModal"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +async function act(callback: () => void | Promise) { + let result: void | Promise = undefined; + flushSync(() => { + result = callback(); + }); + await result; +} + +async function flushReact() { + await act(async () => { + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); + }); +} + +function makeMediaItem(overrides: Partial = {}): GalleryMediaItem { + return { + id: "media-1", + contentPath: "/api/attachments/media-1/content", + openPath: "/api/attachments/media-1/content", + downloadPath: "/api/attachments/media-1/content?download=1", + contentType: "image/png", + originalFilename: "screenshot.png", + ...overrides, + }; +} + +describe("ImageGalleryModal", () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => { + root.unmount(); + }); + document.body.innerHTML = ""; + }); + + it("renders video media with a download link in the gallery", async () => { + const video = makeMediaItem({ + id: "video-1", + contentPath: "/api/attachments/video-1/content", + downloadPath: "/api/attachments/video-1/content?download=1", + contentType: "video/webm", + originalFilename: "demo.webm", + }); + + await act(async () => { + root.render( + undefined} + />, + ); + }); + await flushReact(); + + const renderedVideo = document.body.querySelector("video"); + expect(renderedVideo?.getAttribute("src")).toBe("/api/attachments/video-1/content"); + expect(renderedVideo?.getAttribute("controls")).not.toBeNull(); + expect( + document.body.querySelector('a[aria-label="Download demo.webm"]')?.getAttribute("href"), + ).toBe("/api/attachments/video-1/content?download=1"); + }); + + it("supports keyboard navigation and Escape close", async () => { + const onOpenChange = vi.fn(); + const first = makeMediaItem({ + id: "first", + contentPath: "/api/attachments/first/content", + originalFilename: "first.png", + }); + const second = makeMediaItem({ + id: "second", + contentPath: "/api/attachments/second/content", + originalFilename: "second.png", + }); + + await act(async () => { + root.render( + , + ); + }); + await flushReact(); + + expect(document.body.textContent).toContain("first.png"); + expect(document.body.textContent).toContain("1 / 2"); + + await act(async () => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowRight" })); + }); + await flushReact(); + + expect(document.body.textContent).toContain("second.png"); + expect(document.body.textContent).toContain("2 / 2"); + + await act(async () => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowLeft" })); + }); + await flushReact(); + + expect(document.body.textContent).toContain("first.png"); + expect(document.body.textContent).toContain("1 / 2"); + + await act(async () => { + window.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" })); + }); + + expect(onOpenChange).toHaveBeenCalledWith(false); + }); +}); diff --git a/ui/src/components/ImageGalleryModal.tsx b/ui/src/components/ImageGalleryModal.tsx index b9ae7c8e6c..0d0d1f994b 100644 --- a/ui/src/components/ImageGalleryModal.tsx +++ b/ui/src/components/ImageGalleryModal.tsx @@ -1,35 +1,53 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { Dialog as DialogPrimitive } from "radix-ui"; import { ChevronLeft, ChevronRight, Download, X } from "lucide-react"; -import type { IssueAttachment } from "@paperclipai/shared"; +import { attachmentDownloadPath, attachmentFilename } from "@/lib/issue-attachments"; +import { isVideoLikeOutput } from "@/lib/issue-output"; + +export interface GalleryMediaItem { + id: string; + contentPath: string; + openPath?: string; + downloadPath?: string; + contentType: string; + originalFilename: string | null; +} interface ImageGalleryModalProps { - images: IssueAttachment[]; + items: GalleryMediaItem[]; initialIndex: number; open: boolean; onOpenChange: (open: boolean) => void; } export function ImageGalleryModal({ - images, + items, initialIndex, open, onOpenChange, }: ImageGalleryModalProps) { const [currentIndex, setCurrentIndex] = useState(initialIndex); - const imageRef = useRef(null); + const mediaRef = useRef(null); + const setMediaRef = useCallback((node: HTMLImageElement | HTMLVideoElement | null) => { + mediaRef.current = node; + }, []); useEffect(() => { if (open) setCurrentIndex(initialIndex); }, [open, initialIndex]); const goNext = useCallback(() => { - setCurrentIndex((i) => (i + 1) % images.length); - }, [images.length]); + setCurrentIndex((i) => (i + 1) % items.length); + }, [items.length]); const goPrev = useCallback(() => { - setCurrentIndex((i) => (i - 1 + images.length) % images.length); - }, [images.length]); + setCurrentIndex((i) => (i - 1 + items.length) % items.length); + }, [items.length]); + + useEffect(() => { + if (currentIndex < items.length) return; + setCurrentIndex(0); + }, [currentIndex, items.length]); useEffect(() => { if (!open) return; @@ -49,7 +67,7 @@ export function ImageGalleryModal({ if ( target.closest("button") || target.closest("a") || - target === imageRef.current + target === mediaRef.current ) return; onOpenChange(false); @@ -57,10 +75,12 @@ export function ImageGalleryModal({ [onOpenChange], ); - if (images.length === 0) return null; + if (items.length === 0) return null; - const current = images[currentIndex]; + const current = items[currentIndex]; if (!current) return null; + const filename = attachmentFilename(current); + const isVideo = isVideoLikeOutput(current.contentType, current.originalFilename); return ( @@ -73,18 +93,19 @@ export function ImageGalleryModal({ > {/* Top bar */}
- - {current.originalFilename ?? "Image"} + + {filename}
- {currentIndex + 1} / {images.length} + {currentIndex + 1} / {items.length} e.stopPropagation()} > @@ -104,7 +125,7 @@ export function ImageGalleryModal({
{/* Left nav zone */}
- {images.length > 1 && ( + {items.length > 1 && ( + ) : null}
- +
); @@ -337,6 +357,7 @@ export function IssueAttachmentsSection({ attachment={attachment} onDelete={requestDelete} deletePending={deletePending} + onPreview={onImageClick} /> ))}
diff --git a/ui/src/components/issue-output/IssueOutputSection.test.tsx b/ui/src/components/issue-output/IssueOutputSection.test.tsx index 9ea78581a9..c25cf5a913 100644 --- a/ui/src/components/issue-output/IssueOutputSection.test.tsx +++ b/ui/src/components/issue-output/IssueOutputSection.test.tsx @@ -32,6 +32,8 @@ const UUIDS: Record = { "att-1": "11111111-1111-4111-8111-111111111111", "att-vid": "22222222-2222-4222-8222-222222222222", "att-pdf": "33333333-3333-4333-8333-333333333333", + "att-img": "44444444-4444-4444-8444-444444444444", + "att-webm": "55555555-5555-4555-8555-555555555555", }; function metadata(key: string, contentType: string, filename: string) { @@ -76,6 +78,27 @@ describe("IssueOutputSection", () => { expect(markup).toContain("18.4 MB"); }); + it("keeps the open action for primary videos when gallery browsing is enabled", () => { + const markup = renderToStaticMarkup( + undefined} + />, + ); + + expect(markup).toContain("Browse"); + expect(markup).toContain("Open"); + expect(markup).toContain(`href="/api/attachments/${UUIDS["att-1"]}/content"`); + expect(markup).toContain("Download"); + }); + it("renders nothing when the issue has no artifact outputs (empty state)", () => { const markup = renderToStaticMarkup( { expect(markup).toContain("PDF"); }); + it("renders secondary image and video outputs as preview tiles", () => { + const markup = renderToStaticMarkup( + , + ); + + expect(markup).toContain("Also produced"); + expect(markup).toContain("screenshot.png"); + expect(markup).toContain("clip.webm"); + expect(markup).toContain(" { const markup = renderToStaticMarkup( string | null; + onMediaClick?: (item: IssueOutputItem) => void; +} + +function isMediaOutput(item: IssueOutputItem) { + const meta = item.metadata; + return Boolean(meta && ( + isImageContentType(meta.contentType) || + isVideoLikeOutput(meta.contentType, meta.originalFilename) + )); +} + +function OutputMediaPreview({ + item, + creatorName, + onMediaClick, +}: { + item: IssueOutputItem; + creatorName?: string | null; + onMediaClick?: (item: IssueOutputItem) => void; +}) { + const meta = item.metadata; + if (!meta) return null; + + const filename = outputFilename(item); + const isVideo = isVideoLikeOutput(meta.contentType, meta.originalFilename); + const metaBits = [meta.contentType, formatBytes(meta.byteSize)]; + if (creatorName) metaBits.push(creatorName); + metaBits.push(relativeTime(item.createdAt)); + + const preview = ( + <> + {isVideo ? ( + + {preview} + + ); } /** @@ -19,12 +110,14 @@ interface IssueOutputSectionProps { * omitted entirely when the issue has produced no outputs — we never show a * permanent empty card. */ -export function IssueOutputSection({ workProducts, resolveCreatorName }: IssueOutputSectionProps) { +export function IssueOutputSection({ workProducts, resolveCreatorName, onMediaClick }: IssueOutputSectionProps) { const { primary, rest, count } = getIssueOutputs(workProducts); if (!primary) return null; const creatorFor = (item: IssueOutputItem) => resolveCreatorName?.(item) ?? null; + const mediaRest = rest.filter(isMediaOutput); + const fileRest = rest.filter((item) => !isMediaOutput(item)); return (
@@ -37,19 +130,32 @@ export function IssueOutputSection({ workProducts, resolveCreatorName }: IssueOu {/* Stable anchor target so company Artifacts cards can deep-link to a specific work product inside its issue context (PAP-10359). */}
- +
- {rest.length > 0 && ( + {rest.length > 0 ? (

Also produced

- {rest.map((item) => ( + {mediaRest.length > 0 ? ( +
+ {mediaRest.map((item) => ( +
+ +
+ ))} +
+ ) : null} + {fileRest.map((item) => (
))}
- )} + ) : null}
); } diff --git a/ui/src/components/issue-output/OutputPrimaryCard.tsx b/ui/src/components/issue-output/OutputPrimaryCard.tsx index c0a6ca7969..a4f935a3a1 100644 --- a/ui/src/components/issue-output/OutputPrimaryCard.tsx +++ b/ui/src/components/issue-output/OutputPrimaryCard.tsx @@ -1,11 +1,11 @@ -import { Download, ExternalLink } from "lucide-react"; +import { Download, ExternalLink, Maximize2 } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Badge } from "@/components/ui/badge"; import { cn, relativeTime } from "@/lib/utils"; import { formatBytes, isImageContentType, - isVideoContentType, + isVideoLikeOutput, outputFilename, type IssueOutputItem, } from "@/lib/issue-output"; @@ -15,6 +15,7 @@ import { OutputFileTile } from "./OutputFileTile"; interface OutputPrimaryCardProps { item: IssueOutputItem; creatorName?: string | null; + onMediaClick?: (item: IssueOutputItem) => void; } /** @@ -22,26 +23,42 @@ interface OutputPrimaryCardProps { * over a metadata strip with Open + Download actions. The layout stacks on * mobile and uses a single horizontal meta row on desktop. */ -export function OutputPrimaryCard({ item, creatorName }: OutputPrimaryCardProps) { +export function OutputPrimaryCard({ item, creatorName, onMediaClick }: OutputPrimaryCardProps) { const meta = item.metadata; const filename = outputFilename(item); const contentType = meta?.contentType; + const isMedia = Boolean(meta && ( + isImageContentType(contentType) || + isVideoLikeOutput(contentType, meta.originalFilename) + )); + const isVideo = Boolean(meta && isVideoLikeOutput(contentType, meta.originalFilename)); return (
{/* Media region */} - {meta && isVideoContentType(contentType) ? ( + {isVideo && meta ? ( ) : meta && isImageContentType(contentType) ? ( - - {filename} - + onMediaClick ? ( + + ) : ( + + {filename} + + ) ) : (
@@ -76,12 +93,25 @@ export function OutputPrimaryCard({ item, creatorName }: OutputPrimaryCardProps) {meta ? (
- + {isMedia && onMediaClick ? ( + + ) : null} + {!isMedia || !onMediaClick || isVideo ? ( + + ) : null}