fix: contain attachment previews in chat panel #1841

Merged
hongming merged 1 commits from fix/panel-contained-attachment-preview into main 2026-05-25 08:48:17 +00:00
6 changed files with 51 additions and 18 deletions
@@ -166,11 +166,12 @@ export function AttachmentImage({ workspaceId, attachment, onDownload, tone }: P
open={open}
onClose={() => setOpen(false)}
ariaLabel={`Preview of ${attachment.name}`}
contained
>
<img
src={state.blobUrl}
alt={attachment.name}
className="max-w-[95vw] max-h-[90vh] object-contain"
className="max-w-full max-h-full object-contain"
/>
</AttachmentLightbox>
</>
@@ -1,6 +1,6 @@
"use client";
// AttachmentLightbox — shared fullscreen modal for image / PDF /
// AttachmentLightbox — shared modal for image / PDF /
// (future) any-fullscreen-renderable kind. Owns:
// - Backdrop + centered viewport
// - Esc to close
@@ -14,11 +14,11 @@
//
// Design choices:
//
// 1. Portals — we don't use ReactDOM.createPortal because the canvas
// chat surface already renders at a high z-index and the modal's
// fixed-position layout reaches the viewport regardless. Saves a
// portal mount in the common case + avoids the SSR warning (canvas
// is "use client" but the parent shell is server-rendered).
// 1. Portals — we don't use ReactDOM.createPortal because the chat tab
// already gives us a positioned container and the preview should stay
// inside that panel. Saves a portal mount in the common case + avoids
// the SSR warning (canvas is "use client" but the parent shell is
// server-rendered).
//
// 2. Focus trap — inline implementation (not a 3rd-party dep). The
// chat lightbox needs to trap focus only across two interactive
@@ -41,13 +41,17 @@ interface Props {
* the dialog opens. The caller knows what's inside (image alt
* text, PDF filename) and supplies it. */
ariaLabel: string;
/** Constrain the preview to the nearest positioned ancestor instead
* of the whole browser viewport. ChatTab passes this so previews
* stay inside the active side-panel tab. */
contained?: boolean;
/** The thing being shown in fullscreen — <img>, <embed>, etc.
* Caller is responsible for sizing it to fit the viewport (we
* give it max-w-full max-h-full via CSS). */
children: ReactNode;
}
export function AttachmentLightbox({ open, onClose, ariaLabel, children }: Props) {
export function AttachmentLightbox({ open, onClose, ariaLabel, contained = false, children }: Props) {
const closeButtonRef = useRef<HTMLButtonElement>(null);
const previousFocusRef = useRef<HTMLElement | null>(null);
@@ -90,12 +94,19 @@ export function AttachmentLightbox({ open, onClose, ariaLabel, children }: Props
if (!open) return null;
const rootClass = contained
? "absolute inset-0 z-50 flex items-center justify-center bg-black/85 motion-reduce:transition-none transition-opacity"
: "fixed inset-0 z-50 flex items-center justify-center bg-black/85 motion-reduce:transition-none transition-opacity";
const contentClass = contained
? "h-full w-full p-3 flex items-center justify-center"
: "max-w-[95vw] max-h-[90vh] flex items-center justify-center";
return (
<div
role="dialog"
aria-modal="true"
aria-label={ariaLabel}
className="fixed inset-0 z-50 flex items-center justify-center bg-black/85 motion-reduce:transition-none transition-opacity"
className={rootClass}
onClick={onBackdropClick}
>
{/* Close button — top-right, large hit area, keyboard-focusable.
@@ -112,7 +123,7 @@ export function AttachmentLightbox({ open, onClose, ariaLabel, children }: Props
</svg>
</button>
<div
className="max-w-[95vw] max-h-[90vh] flex items-center justify-center"
className={contentClass}
onClick={(e) => e.stopPropagation()}
>
{children}
@@ -19,8 +19,8 @@
// suppress the toolbar; we keep it on so the user gets standard
// PDF affordances.
//
// Fullscreen: AttachmentLightbox hosts the PDF at viewport size on
// click. Same shared modal as image — third caller justifies the
// Preview: AttachmentLightbox hosts the PDF inside the active chat tab
// on click. Same shared modal as image — third caller justifies the
// abstraction (per RFC #2991 design).
//
// Failure modes:
@@ -144,8 +144,9 @@ export function AttachmentPDF({ workspaceId, attachment, onDownload, tone }: Pro
open={open}
onClose={() => setOpen(false)}
ariaLabel={`Preview of ${attachment.name}`}
contained
>
<div className="h-[90vh] w-[95vw] overflow-hidden rounded-lg border border-white/20 bg-white shadow-2xl">
<div className="h-full w-full overflow-hidden rounded-lg border border-white/20 bg-white shadow-2xl">
<iframe
src={`${state.blobUrl}#view=FitH`}
title={attachment.name}
@@ -1,6 +1,6 @@
// @vitest-environment jsdom
/**
* AttachmentLightbox — fullscreen modal for image / PDF preview.
* AttachmentLightbox — modal for image / PDF preview.
*
* Owns: backdrop + viewport, Esc to close, click-outside to close,
* focus trap (close button focus on open, restore on close),
@@ -135,6 +135,22 @@ describe("AttachmentLightbox — render", () => {
const closeBtn = document.querySelector('button[aria-label="Close preview"]');
expect(closeBtn).toBeTruthy();
});
it("uses absolute positioning when contained=true", () => {
render(
<AttachmentLightbox
open={true}
onClose={vi.fn()}
ariaLabel="Preview"
contained
>
<MockContent />
</AttachmentLightbox>,
);
const dialog = document.querySelector('[role="dialog"]');
expect(dialog?.className).toContain("absolute");
expect(dialog?.className).not.toContain("fixed");
});
});
// ─── Focus management ─────────────────────────────────────────────────────────
@@ -1,12 +1,12 @@
// @vitest-environment jsdom
/**
* AttachmentPDF — inline PDF preview button + click-to-fullscreen lightbox.
* AttachmentPDF — inline PDF preview button + click-to-panel lightbox.
*
* Per RFC #2991 PR-3: platform-auth URIs fetch bytes → Blob → ObjectURL;
* external URIs use the raw URL directly. State machine: idle → loading →
* ready/error. Loading skeleton shown while fetching. Error falls back to
* AttachmentChip. Clicking the preview button opens AttachmentLightbox with
* <embed>. Blob URL cleaned up on unmount.
* a browser PDF iframe. Blob URL cleaned up on unmount.
*
* NOTE: No @testing-library/jest-dom import — use DOM APIs for assertions.
*
@@ -158,10 +158,12 @@ describe("AttachmentPDF — ready", () => {
});
const dialog = document.querySelector('[role="dialog"]');
expect(dialog?.getAttribute("aria-label")).toContain("report.pdf");
expect(dialog?.className).toContain("absolute");
const frame = dialog?.querySelector("iframe") as HTMLIFrameElement | null;
expect(frame).toBeTruthy();
expect(frame?.getAttribute("title")).toBe("report.pdf");
expect(frame?.className).toContain("bg-white");
expect(frame?.parentElement?.className).toContain("w-full");
expect(dialog?.querySelector("embed")).toBeNull();
});
@@ -237,11 +237,13 @@ describe("AttachmentPreview dispatch", () => {
expect(screen.getByLabelText(/Open doc\.pdf preview/i)).toBeTruthy();
});
// Click → lightbox opens with <embed> inside.
// Click → panel-contained lightbox opens with a browser PDF iframe.
fireEvent.click(screen.getByLabelText(/Open doc\.pdf preview/i));
const dialog = await screen.findByRole("dialog");
expect(dialog).toBeTruthy();
expect(dialog.querySelector("embed[type='application/pdf']")).not.toBeNull();
expect(dialog.className).toContain("absolute");
expect(dialog.querySelector("iframe")).not.toBeNull();
expect(dialog.querySelector("embed")).toBeNull();
});
it("kind=pdf fetch fails → falls back to chip", async () => {