fix: contain attachment previews in chat panel #1841
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user