Compare commits

...

2 Commits

Author SHA1 Message Date
7cf4e74dc6 fix(mobile): prevent pushState → popstate infinite loop in MobileApp URL sync
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 13s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 13s
Harness Replays / detect-changes (pull_request) Successful in 16s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 13s
E2E API Smoke Test / detect-changes (pull_request) Successful in 44s
gate-check-v3 / gate-check (pull_request) Successful in 25s
CI / Detect changes (pull_request) Successful in 50s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 46s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 36s
qa-review / approved (pull_request) Failing after 17s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 49s
security-review / approved (pull_request) Failing after 11s
sop-tier-check / tier-check (pull_request) Successful in 10s
Harness Replays / Harness Replays (pull_request) Successful in 4s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 8s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 18s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 6s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 7s
audit-force-merge / audit (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Failing after 6m33s
CI / Python Lint & Test (pull_request) Successful in 6m59s
CI / Canvas (Next.js) (pull_request) Successful in 7m32s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Failing after 3s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 7m44s
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 <noreply@anthropic.com>
2026-05-12 04:32:25 +00:00
c71de18ac1 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 <noreply@anthropic.com>
2026-05-12 03:54:01 +00:00
2 changed files with 226 additions and 2 deletions

View File

@ -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);
};

View File

@ -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<React.ComponentProps<typeof AttachmentLightbox>>) {
const onClose = vi.fn();
const result = render(
<AttachmentLightbox
open={true}
onClose={onClose}
ariaLabel="Image preview"
{...props}
>
<img alt="test" src="data:image/png;base64,iVBORw0KGgo=" />
</AttachmentLightbox>,
);
return { ...result, onClose };
}
// ─── Render States ────────────────────────────────────────────────────────────
describe("AttachmentLightbox — render", () => {
it("does not render when open=false", () => {
const { container } = render(
<AttachmentLightbox
open={false}
onClose={vi.fn()}
ariaLabel="Preview"
>
<div>content</div>
</AttachmentLightbox>,
);
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(
<AttachmentLightbox
open={false}
onClose={onClose}
ariaLabel="Preview"
>
<div>content</div>
</AttachmentLightbox>,
);
// Open the modal
rerender(
<AttachmentLightbox
open={true}
onClose={onClose}
ariaLabel="Preview"
>
<div>content</div>
</AttachmentLightbox>,
);
// 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(
<AttachmentLightbox
open={false}
onClose={onClose}
ariaLabel="Preview"
>
<div>content</div>
</AttachmentLightbox>,
);
// Focus should be restored to outerBtn
expect(document.activeElement).toBe(outerBtn);
document.body.removeChild(outerBtn);
});
});