From c71de18ac11447e076732384e1c14d0e9ec74c9d Mon Sep 17 00:00:00 2001 From: Molecule AI App-FE Date: Tue, 12 May 2026 02:40:47 +0000 Subject: [PATCH 1/2] test(canvas/chat): add AttachmentLightbox coverage (13 cases) Covers: - Does not render when open=false - Renders dialog with role=dialog and aria-modal - Renders with provided aria-label - Close button has aria-label="Close preview" - Click backdrop calls onClose; content click does not - Escape key calls onClose; other keys do not - Focus moves to close button when opened - Focus restores to previous element when closed - Reduced-motion class on backdrop - Renders children inside the modal Per RFC #2991 Phase 2: this is the third-caller justification for the abstraction (image, PDF, future video-fullscreen all want the same modal contract). Co-Authored-By: Claude Opus 4.7 --- .../__tests__/AttachmentLightbox.test.tsx | 214 ++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 canvas/src/components/tabs/chat/__tests__/AttachmentLightbox.test.tsx diff --git a/canvas/src/components/tabs/chat/__tests__/AttachmentLightbox.test.tsx b/canvas/src/components/tabs/chat/__tests__/AttachmentLightbox.test.tsx new file mode 100644 index 00000000..44e24af3 --- /dev/null +++ b/canvas/src/components/tabs/chat/__tests__/AttachmentLightbox.test.tsx @@ -0,0 +1,214 @@ +// @vitest-environment jsdom +/** + * AttachmentLightbox — shared fullscreen modal for image/PDF/preview. + * + * Per RFC #2991 Phase 2, AttachmentLightbox owns: + * - Backdrop + centered viewport + * - Esc to close + * - Click-outside to close (stopPropagation on content) + * - Focus trap: focus enters close button on open, restores on close + * - prefers-reduced-motion respect + * + * NOTE: No @testing-library/jest-dom import — use textContent / className / + * getAttribute / checked / value checks to avoid jest-dom dependency errors. + * + * Covers: + * - Does not render when open=false + * - Renders dialog with role=dialog and aria-modal + * - Renders with provided aria-label + * - Close button has aria-label="Close preview" + * - Clicking backdrop (outside content) calls onClose + * - Clicking content does NOT call onClose (stopPropagation) + * - Escape key calls onClose + * - Focus moves to close button when opened + * - Focus restores to previous element when closed + * - Reduced motion: motion-reduce class on backdrop + * - Renders children inside the modal + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import React from "react"; + +import { AttachmentLightbox } from "../AttachmentLightbox"; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.resetModules(); +}); + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +/** Renders the lightbox open with children and returns close fn */ +function renderOpen(props?: Partial>) { + const onClose = vi.fn(); + const result = render( + + test + , + ); + return { ...result, onClose }; +} + +// ─── Render States ──────────────────────────────────────────────────────────── + +describe("AttachmentLightbox — render", () => { + it("does not render when open=false", () => { + const { container } = render( + +
content
+ , + ); + expect(container.firstChild).toBeNull(); + }); + + it("renders dialog with role=dialog and aria-modal", () => { + renderOpen(); + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog).toBeTruthy(); + expect(dialog?.getAttribute("aria-modal")).toBe("true"); + }); + + it("renders with provided aria-label", () => { + renderOpen({ ariaLabel: "PDF: report-2026.pdf" }); + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog?.getAttribute("aria-label")).toBe("PDF: report-2026.pdf"); + }); + + it("close button has aria-label='Close preview'", () => { + renderOpen(); + const btn = document.querySelector('[aria-label="Close preview"]'); + expect(btn).toBeTruthy(); + expect(btn?.tagName).toBe("BUTTON"); + }); + + it("renders children inside the modal", () => { + renderOpen({ ariaLabel: "Preview" }); + // Children are inside the dialog + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog?.querySelector("img")).toBeTruthy(); + }); + + it("applies reduced-motion class on backdrop", () => { + renderOpen(); + // The div[role="dialog"] IS the fixed backdrop — contains motion-reduce:transition-none + const dialog = document.querySelector('[role="dialog"]') as HTMLElement; + expect(dialog?.className).toContain("motion-reduce"); + }); +}); + +// ─── Interaction ───────────────────────────────────────────────────────────── + +describe("AttachmentLightbox — interaction", () => { + it("calls onClose when close button is clicked", () => { + const onClose = vi.fn(); + renderOpen({ onClose }); + const btn = document.querySelector('[aria-label="Close preview"]') as HTMLButtonElement; + btn.click(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("calls onClose when backdrop (outside content) is clicked", () => { + const onClose = vi.fn(); + renderOpen({ onClose }); + // The div[role="dialog"] IS the backdrop (fixed inset-0). + // Click on it — e.target === e.currentTarget triggers onBackdropClick. + const backdrop = document.querySelector('[role="dialog"]') as HTMLElement; + fireEvent.click(backdrop, { target: backdrop }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("does NOT call onClose when content (inside modal) is clicked", () => { + const onClose = vi.fn(); + renderOpen({ onClose }); + // Click on the img inside the modal + const img = document.querySelector("img") as HTMLElement; + fireEvent.click(img); + expect(onClose).not.toHaveBeenCalled(); + }); + + it("calls onClose on Escape key", () => { + const onClose = vi.fn(); + renderOpen({ onClose }); + fireEvent.keyDown(document, { key: "Escape" }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("does not call onClose for other keys", () => { + const onClose = vi.fn(); + renderOpen({ onClose }); + fireEvent.keyDown(document, { key: "Enter" }); + fireEvent.keyDown(document, { key: "Tab" }); + expect(onClose).not.toHaveBeenCalled(); + }); +}); + +// ─── Focus Management ──────────────────────────────────────────────────────── + +describe("AttachmentLightbox — focus management", () => { + it("moves focus to close button when opened", () => { + renderOpen(); + const btn = document.querySelector('[aria-label="Close preview"]') as HTMLButtonElement; + expect(document.activeElement).toBe(btn); + }); + + it("restores focus to previous element when closed", () => { + // Create a button to hold focus before opening the modal + const outerBtn = document.createElement("button"); + outerBtn.textContent = "Open modal"; + document.body.appendChild(outerBtn); + outerBtn.focus(); + expect(document.activeElement).toBe(outerBtn); + + const onClose = vi.fn(); + const { rerender } = render( + +
content
+ , + ); + + // Open the modal + rerender( + +
content
+ , + ); + + // Focus should now be on close button + const btn = document.querySelector('[aria-label="Close preview"]') as HTMLButtonElement; + expect(document.activeElement).toBe(btn); + + // Close the modal + rerender( + +
content
+ , + ); + + // Focus should be restored to outerBtn + expect(document.activeElement).toBe(outerBtn); + + document.body.removeChild(outerBtn); + }); +}); -- 2.45.2 From 7cf4e74dc654972d99049a32dd5ace1d64dafe70 Mon Sep 17 00:00:00 2001 From: Molecule AI App-FE Date: Tue, 12 May 2026 04:32:25 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(mobile):=20prevent=20pushState=20?= =?UTF-8?q?=E2=86=92=20popstate=20infinite=20loop=20in=20MobileApp=20URL?= =?UTF-8?q?=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit React error #185 (Maximum update depth exceeded) on mobile when navigating to ?m=chat&a=... was caused by a render loop in the URL sync effect. Root cause: when setRoute/setAgentId fires, the URL sync effect calls pushState. Some mobile WebViews dispatch popstate synchronously as a side-effect of pushState, which triggers the popstate handler → setRoute → URL sync effect → pushState → popstate → ... infinite loop. Fix: track what we just pushed in a prevPushedRef. The popstate handler reads the current URL and skips the state update if the URL matches prevPushedRef — that means the popstate was our own pushState's side-effect, not a genuine user back-navigation. Also tracks: react-hooks/exhaustive-props disabled on the URL sync effect since prevPushedRef is updated inside the effect body (intentional self-mutation pattern for cross-effect communication). Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/mobile/MobileApp.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/canvas/src/components/mobile/MobileApp.tsx b/canvas/src/components/mobile/MobileApp.tsx index a527a5ef..bd966755 100644 --- a/canvas/src/components/mobile/MobileApp.tsx +++ b/canvas/src/components/mobile/MobileApp.tsx @@ -6,7 +6,7 @@ // instantiated when MobileApp is active, so no React Flow + heavy // chrome cost on phones. -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useTheme } from "@/lib/theme-provider"; @@ -90,20 +90,30 @@ export function MobileApp() { // the URL is already what we'd produce — that handles the initial // mount (we read FROM the URL) and prevents duplicate history entries // when popstate restores state we just pushed. + // Track what we pushed so the popstate handler can distinguish + // "browser back to our own push" (skip) from "browser back to + // user navigation" (update state). + const prevPushedRef = useRef<{ route: Route; agentId: string | null }>({ route: route as Route, agentId }); useEffect(() => { if (typeof window === "undefined") return; const current = readRouteFromUrl(); if (current.route === route && current.agentId === agentId) return; const url = buildRouteUrl(route, agentId); + prevPushedRef.current = { route, agentId }; window.history.pushState({ route, agentId }, "", url); - }, [route, agentId]); + }, [route, agentId]); // eslint-disable-line react-hooks/exhaustive-props // Sync URL → route state on browser back/forward. The popstate event // fires AFTER the URL has changed, so re-reading is correct. + // Skip: if the URL now reflects the state we just pushed (popstate was + // fired synchronously by our own pushState on some mobile browsers). useEffect(() => { if (typeof window === "undefined") return; const onPop = () => { const next = readRouteFromUrl(); + // If the URL reflects what we just pushed, skip — this popstate + // was a side-effect of our own pushState, not user navigation. + if (next.route === prevPushedRef.current.route && next.agentId === prevPushedRef.current.agentId) return; setRoute(next.route); setAgentId(next.agentId); }; -- 2.45.2