mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
fix(ui): preserve review and download actions for text artifacts
Keep Markdown review documents available alongside explicit attachment tabs. Preserve direct card downloads and separate the preview action. Regressions fail with the text context enabled before the fix; 199 focused tests and both desktop/mobile browser cases pass afterward. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
92867f4173
commit
d13aeb36f4
5 files changed
+97
-11
No files matched your search
@@ -209,6 +209,10 @@ card, or a chat attachment chip to open a named right-side tab. Reopening the
|
||||
same attachment focuses its existing tab. Tabs can be switched and closed.
|
||||
On mobile, the same viewer opens in the task details drawer.
|
||||
|
||||
Markdown work products keep their expandable review document, annotations,
|
||||
revision indicator, and document links. **Open in tab** is a separate action.
|
||||
Work-product cards keep **Download** as a direct original-file download.
|
||||
|
||||
Markdown attachments offer **Rendered** and **Raw** views. Other supported text
|
||||
files display literal text. The viewer provides a download action. Preview reads
|
||||
are limited to 512 KiB; oversized, unsupported, or unavailable files show an
|
||||
|
||||
@@ -23,6 +23,18 @@ for (const mobile of [false, true]) {
|
||||
await json(await request.post(`/api/issues/${issue.id}/comments`, {
|
||||
data: { body: "Review the attached files.", attachmentIds: attachments.map((attachment) => attachment.id) },
|
||||
}));
|
||||
for (const [index, attachment] of attachments.entries()) {
|
||||
await json(await request.post(`/api/issues/${issue.id}/work-products`, {
|
||||
data: {
|
||||
type: "artifact", provider: "paperclip", title: `Delivered ${files[index]!.name}`, status: "ready_for_review",
|
||||
metadata: {
|
||||
attachmentId: attachment.id, contentType: attachment.contentType, byteSize: attachment.byteSize,
|
||||
originalFilename: files[index]!.name, contentPath: attachment.contentPath,
|
||||
openPath: attachment.contentPath, downloadPath: `${attachment.contentPath}?download=1`,
|
||||
},
|
||||
},
|
||||
}));
|
||||
}
|
||||
await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`);
|
||||
await page.getByRole("link", { name: "Open AGENTS.md", exact: true }).click();
|
||||
const panel = mobile ? page.getByTestId("mobile-task-side-panel") : page.locator("aside").filter({ has: page.getByRole("tab", { name: "AGENTS.md", exact: true }) });
|
||||
@@ -50,5 +62,22 @@ for (const mobile of [false, true]) {
|
||||
await panel.getByRole("tab", { name: "AGENTS.md", exact: true }).click();
|
||||
await expect(panel.getByRole("heading", { name: "File charter", exact: true })).toBeVisible();
|
||||
await expect(panel.getByRole("tab", { name: "AGENTS.md", exact: true })).toHaveCount(1);
|
||||
await panel.getByRole("button", { name: "Open a new tab", exact: true }).click();
|
||||
await page.getByRole("option", { name: "Artifacts", exact: true }).click();
|
||||
const review = panel.getByRole("button", { name: /Delivered AGENTS.md.*For review/ });
|
||||
await review.click();
|
||||
await expect(review).toHaveAttribute("aria-expanded", "true");
|
||||
await expect(panel.getByRole("heading", { name: "File charter", exact: true })).toBeVisible();
|
||||
await expect(review).toContainText("Rev 1");
|
||||
await expect(panel.getByRole("link", { name: "Download Delivered AGENTS.md", exact: true })).toBeVisible();
|
||||
const cardDownload = page.waitForEvent("download");
|
||||
await panel.getByRole("link", { name: "Download: Delivered notes.txt", exact: true }).click();
|
||||
expect(await fs.readFile((await (await cardDownload).path())!)).toEqual(files[1]!.buffer);
|
||||
await expect(panel.getByRole("tab", { name: "Artifacts", exact: true })).toHaveAttribute("aria-selected", "true");
|
||||
await panel.getByRole("button", { name: "Open in tab: Delivered notes.txt", exact: true }).click();
|
||||
await expect(panel.getByRole("tab", { name: "notes.txt", exact: true })).toHaveAttribute("aria-selected", "true");
|
||||
await panel.getByRole("tab", { name: "Artifacts", exact: true }).click();
|
||||
await panel.getByRole("button", { name: "Open in tab: Delivered AGENTS.md", exact: true }).click();
|
||||
await expect(panel.getByRole("tab", { name: "AGENTS.md", exact: true })).toHaveAttribute("aria-selected", "true");
|
||||
});
|
||||
}
|
||||
@@ -1,6 +1,5 @@
|
||||
import { TextAttachmentContext } from "@/context/TextAttachmentContext";
|
||||
import { isTextAttachment } from "@/lib/issue-attachments";
|
||||
import { getAttachmentArtifactWorkProductMetadata } from "@paperclipai/shared";
|
||||
import { useContext, useEffect, useMemo, useRef, useState } from "react";
|
||||
import type { CSSProperties } from "react";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
@@ -102,6 +101,7 @@ function MarkdownWorkProductRow({
|
||||
reviewDoc: IssueDocument | undefined;
|
||||
openRequestId?: number;
|
||||
}) {
|
||||
const openTextAttachment = useContext(TextAttachmentContext);
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const [annotationPanelOpen, setAnnotationPanelOpen] = useState(false);
|
||||
const headerRef = useRef<HTMLDivElement | null>(null);
|
||||
@@ -236,6 +236,17 @@ function MarkdownWorkProductRow({
|
||||
onToggle={() => setAnnotationPanelOpen((open) => !open)}
|
||||
/>
|
||||
) : null}
|
||||
{openTextAttachment ? (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => openTextAttachment(metadata.attachmentId, metadata.originalFilename ?? workProduct.title)}
|
||||
aria-label={`Open in tab: ${workProduct.title}`}
|
||||
title="Open in tab"
|
||||
className="shrink-0 px-1.5 py-1.5 text-muted-foreground hover:text-foreground"
|
||||
>
|
||||
<FileText className="h-3 w-3" />
|
||||
</button>
|
||||
) : null}
|
||||
<a
|
||||
href={metadata.openPath}
|
||||
target="_blank"
|
||||
@@ -464,14 +475,6 @@ export function IssuePropertiesArtifactsTab({ issue, documentDeepLink, onOpenDoc
|
||||
{group.rows.map((row) => {
|
||||
if (row.kind === "work_product") {
|
||||
const wp = row.value;
|
||||
const textMetadata = getAttachmentArtifactWorkProductMetadata(wp);
|
||||
if (openTextAttachment && textMetadata && isTextAttachment(textMetadata)) {
|
||||
return <li key={row.id} className="col-span-full min-w-0">
|
||||
<button type="button" className={cn(ROW_CLASS, "w-full text-left hover:bg-accent/50")} onClick={() => openTextAttachment(textMetadata.attachmentId, textMetadata.originalFilename ?? wp.title)}>
|
||||
<FileText className="size-4 shrink-0 text-muted-foreground" /><span className="min-w-0 flex-1 truncate">{wp.title}</span><ChevronRight className="size-4" />
|
||||
</button>
|
||||
</li>;
|
||||
}
|
||||
const markdownMetadata = getMarkdownWorkProductAttachmentMetadata(wp);
|
||||
if (markdownMetadata) {
|
||||
const reviewKey = artifactReviewDocumentKey(wp.id);
|
||||
|
||||
@@ -8,6 +8,8 @@ import type { Issue, IssueAttachment, IssueDocument, IssueWorkProduct } from "@p
|
||||
import { artifactReviewDocumentKey } from "@paperclipai/shared";
|
||||
import { IssuePropertiesArtifactsTab } from "./IssuePropertiesArtifactsTab";
|
||||
import { ApiError } from "@/api/client";
|
||||
import { TextAttachmentContext } from "@/context/TextAttachmentContext";
|
||||
import { RichWorkProductCard } from "@/components/task-chat/RichWorkProductCard";
|
||||
|
||||
const mockIssuesApi = vi.hoisted(() => ({
|
||||
listAttachments: vi.fn(async (): Promise<unknown[]> => []),
|
||||
@@ -159,6 +161,7 @@ async function waitForAssertion(assertion: () => void, attempts = 20) {
|
||||
describe("markdown work product review row", () => {
|
||||
let container: HTMLDivElement;
|
||||
let root: Root | null = null;
|
||||
const openText = vi.fn();
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -193,7 +196,9 @@ describe("markdown work product review row", () => {
|
||||
await act(async () =>
|
||||
currentRoot.render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<IssuePropertiesArtifactsTab issue={issue} {...props} />
|
||||
<TextAttachmentContext.Provider value={openText}>
|
||||
<IssuePropertiesArtifactsTab issue={issue} {...props} />
|
||||
</TextAttachmentContext.Provider>
|
||||
</QueryClientProvider>,
|
||||
),
|
||||
);
|
||||
@@ -234,6 +239,40 @@ describe("markdown work product review row", () => {
|
||||
expect(mockIssuesApi.ensureWorkProductReviewDocument).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("opens the attachment in a tab without replacing its review controls", async () => {
|
||||
mockUseIssueDocuments.mockReturnValue({ data: [makeReviewDocument({ latestRevisionNumber: 3 })] });
|
||||
await renderTab();
|
||||
|
||||
expect(container.textContent).toContain("Rev 3");
|
||||
expect(container.querySelector(`[data-testid="annotation-count-${REVIEW_KEY}"]`)).not.toBeNull();
|
||||
const open = container.querySelector('button[aria-label="Open in tab: Verification report"]') as HTMLButtonElement;
|
||||
expect(open).not.toBeNull();
|
||||
await act(async () => open.click());
|
||||
expect(openText).toHaveBeenCalledWith(ATTACHMENT_ID, "report.md");
|
||||
expect(expandButton().getAttribute("aria-expanded")).toBe("false");
|
||||
await act(async () => expandButton().click());
|
||||
expect(container.querySelector(`[data-testid="annotation-surface-${REVIEW_KEY}"]`)).not.toBeNull();
|
||||
});
|
||||
|
||||
it.each(["compact", "card"] as const)("keeps %s text-card downloads separate from opening a tab", async (variant) => {
|
||||
root = createRoot(container);
|
||||
await act(async () => root!.render(
|
||||
<TextAttachmentContext.Provider value={openText}>
|
||||
<RichWorkProductCard workProduct={makeMarkdownWorkProduct()} href={`/api/attachments/${ATTACHMENT_ID}/content`} variant={variant} />
|
||||
</TextAttachmentContext.Provider>,
|
||||
));
|
||||
const download = container.querySelector('a[aria-label="Download: Verification report"]') as HTMLAnchorElement;
|
||||
expect(download.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content?download=1`);
|
||||
const click = new MouseEvent("click", { bubbles: true, cancelable: true });
|
||||
await act(async () => { download.dispatchEvent(click); });
|
||||
expect(click.defaultPrevented).toBe(false);
|
||||
expect(openText).not.toHaveBeenCalled();
|
||||
const open = container.querySelector('button[aria-label="Open in tab: Verification report"]') as HTMLButtonElement;
|
||||
expect(open).not.toBeNull();
|
||||
await act(async () => open.click());
|
||||
expect(openText).toHaveBeenCalledWith(ATTACHMENT_ID, "report.md");
|
||||
});
|
||||
|
||||
it("materializes the document on first expand", async () => {
|
||||
mockIssuesApi.ensureWorkProductReviewDocument.mockResolvedValue(makeReviewDocument());
|
||||
await renderTab();
|
||||
|
||||
@@ -190,6 +190,7 @@ export function RichWorkProductCard({ workProduct, href, variant = "card" }: Ric
|
||||
|
||||
const openText = useContext(TextAttachmentContext);
|
||||
const textMetadata = getAttachmentArtifactWorkProductMetadata(workProduct);
|
||||
const canOpenText = Boolean(openText && textMetadata && isTextAttachment(textMetadata));
|
||||
const additions = numberMeta(metadata, "additions");
|
||||
const deletions = numberMeta(metadata, "deletions");
|
||||
const files = numberMeta(metadata, "files", "changedFiles");
|
||||
@@ -277,12 +278,22 @@ export function RichWorkProductCard({ workProduct, href, variant = "card" }: Ric
|
||||
</div>
|
||||
<div className={cn("flex shrink-0 items-center", compact ? "gap-1.5" : "gap-2")}>
|
||||
{chip ? <Chip chip={chip} /> : null}
|
||||
{canOpenText ? (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => openText!(textMetadata!.attachmentId, textMetadata!.originalFilename ?? workProduct.title)}
|
||||
aria-label={`Open in tab: ${workProduct.title}`}
|
||||
className="inline-flex items-center gap-1 text-xs font-medium text-foreground after:absolute after:inset-0 after:rounded-md focus-visible:outline-none focus-visible:after:ring-2 focus-visible:after:ring-ring"
|
||||
>
|
||||
{compact ? null : <span className="hidden @sm:inline">Open in tab</span>}<FileText aria-hidden className="h-3 w-3" />
|
||||
</button>
|
||||
) : null}
|
||||
{mediaPath ? (
|
||||
<button type="button" onClick={openGallery} aria-label={`${action}: ${workProduct.title}`} className="inline-flex items-center gap-1 text-xs font-medium text-foreground after:absolute after:inset-0 after:rounded-md focus-visible:outline-none focus-visible:after:ring-2 focus-visible:after:ring-ring">
|
||||
{compact ? null : <span className="hidden @sm:inline">{action}</span>}<Maximize2 aria-hidden className="h-3 w-3" />
|
||||
</button>
|
||||
) : actionHref ? (
|
||||
<a href={actionHref} onClick={(event) => { if (openText && textMetadata && isTextAttachment(textMetadata) && !event.metaKey && !event.ctrlKey && !event.shiftKey && !event.altKey) { event.preventDefault(); openText(textMetadata.attachmentId, textMetadata.originalFilename ?? workProduct.title); } }} aria-label={`${action}: ${workProduct.title}`} className="inline-flex items-center gap-1 text-xs font-medium text-foreground after:absolute after:inset-0 after:rounded-md focus-visible:outline-none focus-visible:after:ring-2 focus-visible:after:ring-ring" target={actionHref.startsWith("http") ? "_blank" : undefined} rel={actionHref.startsWith("http") ? "noreferrer" : undefined}>
|
||||
<a href={actionHref} aria-label={`${action}: ${workProduct.title}`} className={cn("inline-flex items-center gap-1 text-xs font-medium text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", canOpenText ? "relative z-10" : "after:absolute after:inset-0 after:rounded-md focus-visible:after:ring-2 focus-visible:after:ring-ring")} target={actionHref.startsWith("http") ? "_blank" : undefined} rel={actionHref.startsWith("http") ? "noreferrer" : undefined}>
|
||||
{compact ? null : <span className="hidden @sm:inline">{action}</span>}<ExternalLink aria-hidden className="h-3 w-3" />
|
||||
</a>
|
||||
) : null}
|
||||
|
||||
Reference in new issue
Block a user