From 5b408a7b7cd8cf11b1e0428e506d296e25b7ef46 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Sun, 25 Oct 2026 19:19:42 -0700 Subject: [PATCH] refactor(web): chat media and markdown widgets attach through ref callbacks Rich chat content (Markdown code blocks and tables, Mermaid diagrams, HTML renders, MCP apps, the expanded image dialog, zoomable images, HEIC attachments, Pierre file icons) wired its DOM listeners, observers, and copy timers through effects that mirrored refs and state. Two bugs rode along: a window capture-phase Escape listener closed the expanded image dialog even when a popup inside it was open, and Markdown copy silently did nothing where navigator.clipboard is missing (plain-http LAN and tailnet origins). - ExpandedImageDialog: Escape goes through Base UI's onOpenChange, so a nested popup closes first; opener focus is restored in the close handler; blob URLs are revoked by the setter that replaces them. - Chat overlays (ExpandedImageDialog, ChatOverlayHosts) are dropped by a router onResolved listener started in appInit instead of per-host unmount effects. - MarkdownCodeBlock and MarkdownTable copy through useCopyToClipboard, which has the execCommand fallback; the table measures clipping from a ref callback with a MutationObserver plus ResizeObserver. - ZoomableImage reads the viewer size through useSyncExternalStore and anchors zoom with flushSync in the handler. - McpAppFrame reads the tool definition through a query atom (adds mcpAppEnvironment.toolInfoQuery), adopts a long-lived asset URL during render, and owns full screen through the popover's toggle event and a chrome ref callback. - MermaidDiagram, HtmlRenderFrame, PierreEntryIcon mount or observe from ref callbacks or module load. AttachmentImage starts its decode from the img error handler. Effects kept: HtmlRenderFrame full-screen pin release (component lifetime), MarkdownCodeBlock incremental highlighter disposal, two in McpAppFrame, AttachmentImage decode abort and blob URL revoke on unmount. Co-Authored-By: Claude Opus 5.5 --- apps/web/src/appInit.tsx | 23 ++ .../src/components/chat/ChatOverlayHosts.tsx | 24 +- .../chat/ExpandedImageDialog.browser.test.tsx | 43 ++++ .../components/chat/ExpandedImageDialog.tsx | 92 ++++---- .../chat/HtmlRenderFrame.browser.test.tsx | 41 ++++ .../src/components/chat/HtmlRenderFrame.tsx | 18 +- .../src/components/chat/MarkdownCodeBlock.tsx | 94 +++----- .../chat/MarkdownCopy.browser.test.tsx | 72 ++++++ .../web/src/components/chat/MarkdownTable.tsx | 83 +++---- apps/web/src/components/chat/McpAppFrame.tsx | 217 ++++++++---------- .../src/components/chat/MermaidDiagram.tsx | 17 +- .../src/components/chat/PierreEntryIcon.tsx | 6 +- .../web/src/components/chat/ZoomableImage.tsx | 107 ++++----- .../components/chat/threadOverlayRetention.ts | 15 ++ .../src/components/media/AttachmentImage.tsx | 57 ++--- apps/web/src/pierre-icons.ts | 7 +- packages/client-runtime/src/state/mcpApps.ts | 7 +- 17 files changed, 536 insertions(+), 387 deletions(-) create mode 100644 apps/web/src/components/chat/ExpandedImageDialog.browser.test.tsx create mode 100644 apps/web/src/components/chat/HtmlRenderFrame.browser.test.tsx create mode 100644 apps/web/src/components/chat/MarkdownCopy.browser.test.tsx create mode 100644 apps/web/src/components/chat/threadOverlayRetention.ts diff --git a/apps/web/src/appInit.tsx b/apps/web/src/appInit.tsx index 3d29883887a79..884cff29254fb 100644 --- a/apps/web/src/appInit.tsx +++ b/apps/web/src/appInit.tsx @@ -1,12 +1,15 @@ +import { scopedThreadKey, scopeThreadRef } from "@t3tools/client-runtime/environment"; import type { ClientSettings } from "@t3tools/contracts/settings"; import { createBrowserHistory, createHashHistory } from "@tanstack/react-router"; import { applyAppearanceContrast } from "./appearanceContrast"; import { applyAppearanceFontVariables } from "./appearanceFonts"; +import { retainThreadOverlaysForThread } from "./components/chat/threadOverlayRetention"; import { startAgentOpenedPreviews } from "./components/preview/agentOpenedPreviews"; import { APP_BASE_NAME, APP_DISPLAY_NAME, APP_STAGE_LABEL } from "./branding"; import { resolveServerBackedAppDisplayName } from "./branding.logic"; import { isElectron } from "./env"; +import { useComposerDraftStore } from "./composerDraftStore"; import { getClientSettings, subscribeClientSettings } from "./hooks/useSettings"; import { syncDocumentElectronPlatformClasses, @@ -16,6 +19,7 @@ import { prepareProviderAuthDelivery } from "./providerAuthDelivery"; import { getRouter, type AppRouter } from "./router"; import { appAtomRegistry } from "./rpc/atomRegistry"; import { primaryServerConfigAtom } from "./state/server"; +import { resolveThreadRouteTarget } from "./threadRoutes"; /** Writes appearance settings onto the document, touching only the groups that changed. */ function startClientAppearance(): void { @@ -84,6 +88,24 @@ function startDocumentTitle(): void { ); } +/** Chat overlays belong to the thread they were opened in, so leaving its route drops them. */ +function startThreadOverlayRetention(router: AppRouter): void { + router.subscribe("onResolved", () => { + const target = resolveThreadRouteTarget(router.state.matches.at(-1)?.params ?? {}); + const draftSession = + target?.kind === "draft" + ? useComposerDraftStore.getState().getDraftSession(target.draftId) + : null; + const threadRef = + target?.kind === "server" + ? target.threadRef + : draftSession + ? scopeThreadRef(draftSession.environmentId, draftSession.threadId) + : null; + retainThreadOverlaysForThread(threadRef ? scopedThreadKey(threadRef) : null); + }); +} + /** * Starts every app-lifetime service once, before the first render, and returns the router. * They run for the life of the page, so their disposers are dropped here. @@ -103,6 +125,7 @@ export function startAppServices(): AppRouter { startClientAppearance(); startDocumentTitle(); startAgentOpenedPreviews(); + startThreadOverlayRetention(router); return router; } diff --git a/apps/web/src/components/chat/ChatOverlayHosts.tsx b/apps/web/src/components/chat/ChatOverlayHosts.tsx index f8e3721569535..fd5ecb669dccc 100644 --- a/apps/web/src/components/chat/ChatOverlayHosts.tsx +++ b/apps/web/src/components/chat/ChatOverlayHosts.tsx @@ -7,7 +7,6 @@ import type { } from "@t3tools/contracts"; import { useAtomValue } from "@effect/atom-react"; import { Atom } from "effect/reactivity"; -import { useEffect } from "react"; import { appAtomRegistry } from "~/rpc/atomRegistry"; import { useRightPanelStore } from "~/rightPanelStore"; @@ -25,15 +24,26 @@ import { import { Button } from "../ui/button"; import { Dialog } from "../ui/dialog"; import { WizardPopup } from "../ui/wizard"; - -// Atoms isolate overlay renders. Thread keys prevent overlays following thread switches. +import { registerThreadOverlay } from "./threadOverlayRetention"; + +/** + * Open state for overlays the chat view hosts. It lives in atoms rather than + * ChatView state so opening or closing one renders only its host, not the + * whole chat chrome. Each entry carries the thread it was opened for; a host + * shows only its own thread's entry, and leaving that thread's route drops it. + */ type ThreadOverlayAtom = Atom.Writable<{ readonly threadKey: string; readonly value: T } | null>; function makeThreadOverlayAtom(label: string): ThreadOverlayAtom { - return Atom.make<{ readonly threadKey: string; readonly value: T } | null>(null).pipe( + const atom = Atom.make<{ readonly threadKey: string; readonly value: T } | null>(null).pipe( Atom.keepAlive, Atom.withLabel(`chat:${label}`), ); + registerThreadOverlay((threadKey) => { + const entry = appAtomRegistry.get(atom); + if (entry !== null && entry.threadKey !== threadKey) closeThreadOverlay(atom); + }); + return atom; } function closeThreadOverlay(atom: ThreadOverlayAtom): void { @@ -42,12 +52,6 @@ function closeThreadOverlay(atom: ThreadOverlayAtom): void { function useThreadOverlay(atom: ThreadOverlayAtom, threadKey: string | null): T | null { const entry = useAtomValue(atom); - useEffect( - () => () => { - if (appAtomRegistry.get(atom)?.threadKey === threadKey) closeThreadOverlay(atom); - }, - [atom, threadKey], - ); return threadKey !== null && entry?.threadKey === threadKey ? entry.value : null; } diff --git a/apps/web/src/components/chat/ExpandedImageDialog.browser.test.tsx b/apps/web/src/components/chat/ExpandedImageDialog.browser.test.tsx new file mode 100644 index 0000000000000..1ea8d4c558f35 --- /dev/null +++ b/apps/web/src/components/chat/ExpandedImageDialog.browser.test.tsx @@ -0,0 +1,43 @@ +import type { SnapShotSource } from "@t3tools/contracts"; +import { describe, expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; + +import "~/index.css"; +import { render } from "../../../test/render"; +import { ExpandedImageDialog } from "./ExpandedImageDialog"; + +const PIXEL = + "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="; + +const snapShotSource: SnapShotSource = { + kind: "snap-shot", + capturedAt: "2026-01-01T00:00:00.000Z", + appName: "Finder", + windowTitle: "Downloads", +}; + +describe("ExpandedImageDialog", () => { + it("Escape closes a popup opened inside the dialog before the dialog", async () => { + const onClose = vi.fn(); + await using screen = await render( + , + // The dialog renders no links and reads no route. + { router: false }, + ); + + await userEvent.click(screen.getByRole("button", { name: "No accessibility data" })); + await expect.element(screen.getByText("Accessibility data")).toBeVisible(); + + await userEvent.keyboard("{Escape}"); + await expect.element(screen.getByText("Accessibility data")).not.toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + + // Focus returns to the popup's trigger and opens its tooltip; leave it for a plain control. + (screen.getByRole("button", { name: "Close image preview" }).element() as HTMLElement).focus(); + await userEvent.keyboard("{Escape}"); + await vi.waitFor(() => expect(onClose).toHaveBeenCalledOnce()); + }); +}); diff --git a/apps/web/src/components/chat/ExpandedImageDialog.tsx b/apps/web/src/components/chat/ExpandedImageDialog.tsx index f1a191213b54e..aadbcab79b129 100644 --- a/apps/web/src/components/chat/ExpandedImageDialog.tsx +++ b/apps/web/src/components/chat/ExpandedImageDialog.tsx @@ -1,4 +1,5 @@ -import { memo, useEffect, useRef, useState, type ReactNode, type KeyboardEvent } from "react"; +import { memo, useRef, useState, type ReactNode, type KeyboardEvent } from "react"; +import { flushSync } from "react-dom"; import { ChevronLeftIcon, ChevronRightIcon, XIcon } from "lucide-react"; import { Image as ImageGlyph, Text as TextGlyph } from "lucide"; import { Button } from "../ui/button"; @@ -23,6 +24,7 @@ import { useAtomValue } from "@effect/atom-react"; import { Atom } from "effect/reactivity"; import { appAtomRegistry } from "~/rpc/atomRegistry"; import { expandedImageKey } from "./ExpandedImagePreview"; +import { registerThreadOverlay } from "./threadOverlayRetention"; interface ExpandedImageDialogProps { preview: ExpandedImagePreview; @@ -76,6 +78,13 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ const [returnFocusTarget] = useState(() => document.activeElement instanceof HTMLElement ? document.activeElement : null, ); + // The host unmounts the dialog outright, so Base UI never runs finalFocus. Without + // this, focus stays on the removed dialog, and the composer that owned the opener + // reads that as a blur and rests. + const close = () => { + flushSync(onClose); + if (returnFocusTarget?.isConnected) returnFocusTarget.focus({ preventScroll: true }); + }; const closeButtonRef = useRef(null); const imageCount = preview.images.length; const index = @@ -92,7 +101,7 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ ...source, onOpenFile: () => { openFile(); - onClose(); + close(); }, } : source; @@ -101,18 +110,6 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ setImageOffset((current) => current + direction); }; - // Restore opener focus to prevent composer collapse. - const openerRef = useRef(null); - useEffect(() => { - openerRef.current = document.activeElement; - return () => { - const opener = openerRef.current; - if (opener instanceof HTMLElement && opener.isConnected) { - opener.focus({ preventScroll: true }); - } - }; - }, []); - const onKeyDown = (event: KeyboardEvent) => { if (event.defaultPrevented || isContextMenuOpen() || event.target instanceof HTMLVideoElement) return; @@ -134,17 +131,6 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ navigateImage(1); }; - useEffect(() => { - const onEscape = (event: globalThis.KeyboardEvent) => { - if (event.key !== "Escape" || isContextMenuOpen()) return; - event.preventDefault(); - event.stopPropagation(); - onClose(); - }; - window.addEventListener("keydown", onEscape, { capture: true }); - return () => window.removeEventListener("keydown", onEscape, { capture: true }); - }, [onClose]); - if (!item) return null; const mediaLabel = item.type === "video" ? "video" : "image"; const openOriginalLink = @@ -163,8 +149,14 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ return ( { - if (!open) onClose(); + onOpenChange={(open, eventDetails) => { + if (open) return; + // Escape belongs to an open context menu first. + if (eventDetails.reason === "escape-key" && isContextMenuOpen()) { + eventDetails.cancel(); + return; + } + close(); }} > returnFocusTarget} onClick={(event) => { - if (event.target === event.currentTarget) onClose(); + if (event.target === event.currentTarget) close(); }} > Expanded {mediaLabel} preview @@ -187,7 +179,7 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ size="icon-sm" variant="media-close" className="absolute right-0 top-0 z-20" - onClick={onClose} + onClick={close} aria-label={`Close ${mediaLabel} preview`} > @@ -286,35 +278,45 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ }); // Keep preview state outside ChatView to isolate dialog renders. -const expandedImagePreviewAtom = Atom.make<{ +type ExpandedImagePreviewState = { readonly threadKey: string; readonly preview: ExpandedImagePreview; -} | null>(null).pipe(Atom.keepAlive, Atom.withLabel("chat:expanded-image-preview")); +} | null; + +const expandedImagePreviewAtom = Atom.make(null).pipe( + Atom.keepAlive, + Atom.withLabel("chat:expanded-image-preview"), +); + +function ownedSrc(state: ExpandedImagePreviewState): string | null { + const item = state?.preview.images[state.preview.index]; + return item?.ownsSrc === true ? item.src : null; +} + +// The preview owns a blob URL made for it, so whatever replaces it releases that URL. +function setExpandedImagePreview(next: ExpandedImagePreviewState): void { + const previous = ownedSrc(appAtomRegistry.get(expandedImagePreviewAtom)); + appAtomRegistry.set(expandedImagePreviewAtom, next); + if (previous !== null && previous !== ownedSrc(next)) URL.revokeObjectURL(previous); +} export function openExpandedImagePreview(threadKey: string, preview: ExpandedImagePreview): void { - appAtomRegistry.set(expandedImagePreviewAtom, { threadKey, preview }); + setExpandedImagePreview({ threadKey, preview }); } function closeExpandedImagePreview(): void { - appAtomRegistry.set(expandedImagePreviewAtom, null); + setExpandedImagePreview(null); } -function closeExpandedImagePreviewFor(threadKey: string): void { - if (appAtomRegistry.get(expandedImagePreviewAtom)?.threadKey === threadKey) { - closeExpandedImagePreview(); - } -} +// Leaving the thread's route (or the chat view) drops its preview. +registerThreadOverlay((threadKey) => { + const current = appAtomRegistry.get(expandedImagePreviewAtom); + if (current !== null && current.threadKey !== threadKey) closeExpandedImagePreview(); +}); export function ExpandedImageDialogHost({ threadKey }: { readonly threadKey: string }) { const state = useAtomValue(expandedImagePreviewAtom); const preview = state?.threadKey === threadKey ? state.preview : null; - const item = preview?.images[preview.index]; - const ownedSrc = item?.ownsSrc === true ? item.src : null; - useEffect(() => { - if (ownedSrc === null) return; - return () => URL.revokeObjectURL(ownedSrc); - }, [ownedSrc]); - useEffect(() => () => closeExpandedImagePreviewFor(threadKey), [threadKey]); if (preview === null) return null; return ( ({ + useLongLivedAssetUrl: () => ({ + _tag: "Success", + url: "about:blank", + expiresAt: Date.now() + 60 * 60_000, + }), +})); +vi.mock(import("~/assets/assetUrls"), async (importOriginal) => ({ + ...(await importOriginal()), + useAssetUrlRefresh: () => async () => "about:blank", +})); + +import { HtmlRenderFrame } from "./HtmlRenderFrame"; + +describe("HtmlRenderFrame", () => { + it("reports leaving full screen when it unmounts while expanded", async () => { + const onFullscreenChange = vi.fn(); + await using screen = await render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Open full screen" })); + await expect.element(screen.getByRole("dialog")).toBeVisible(); + expect(onFullscreenChange).toHaveBeenLastCalledWith(true); + + await screen.unmount(); + expect(onFullscreenChange).toHaveBeenLastCalledWith(false); + }); +}); diff --git a/apps/web/src/components/chat/HtmlRenderFrame.tsx b/apps/web/src/components/chat/HtmlRenderFrame.tsx index cbf9edaefaa0f..19fbe6108ae54 100644 --- a/apps/web/src/components/chat/HtmlRenderFrame.tsx +++ b/apps/web/src/components/chat/HtmlRenderFrame.tsx @@ -6,7 +6,7 @@ import { type HtmlRenderReference, } from "@t3tools/shared/htmlRender"; import { Maximize2Icon } from "lucide-react"; -import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useEffect, useState } from "react"; import { useAssetUrlRefresh } from "~/assets/assetUrls"; import { useLongLivedAssetUrl } from "~/assets/longLivedAssetUrls"; @@ -27,16 +27,13 @@ export function HtmlRenderFrame(props: { readonly onFullscreenChange?: (fullscreen: boolean) => void; }) { const { attachmentId, title } = props.htmlRender; - const boxRef = useRef(null); const [width, setWidth] = useState(HTML_RENDER_COLUMN_WIDTH); - useLayoutEffect(() => { - const box = boxRef.current; - if (!box) return; + const observeBox = (box: HTMLDivElement) => { setWidth(box.clientWidth); return observeResize(box, ([entry]) => { if (entry) setWidth(entry.contentRect.width); }); - }, []); + }; // Client fonts can exceed the server-measured height and cause nested scrolling. const [contentHeight, setContentHeight] = useState(); const height = htmlRenderFrameHeight(props.htmlRender, width, contentHeight); @@ -59,14 +56,17 @@ export function HtmlRenderFrame(props: { const refresh = useAssetUrlRefresh(props.environmentId, resource); const [expandedSrc, setExpandedSrc] = useState(null); const { onFullscreenChange } = props; + // Tied to the viewer's lifetime: unmounting while expanded must still release + // the timeline's follow pin, which a close handler alone never reaches. + const expanded = expandedSrc !== null; useEffect(() => { - if (expandedSrc === null) return; + if (!expanded) return; onFullscreenChange?.(true); return () => onFullscreenChange?.(false); - }, [expandedSrc, onFullscreenChange]); + }, [expanded, onFullscreenChange]); return ( -
+
{src !== null ? ( <> + reportMarkdownActionFailure( + { + operation: "copy-code-block", + language, + ...(fenceTitle ? { fenceTitle } : {}), + }, + cause, + ), + }); const [wrapped, setWrapped] = useState(() => getClientSettings().wordWrap); - const copiedTimerRef = useRef | null>(null); const wrapLabel = wrapped ? "Disable line wrap" : "Wrap lines"; const copyLabel = copied ? "Copied" : "Copy code"; const command = code.trim(); @@ -167,43 +178,7 @@ export function MarkdownCodeBlock({ // make the rendered command differ from what the terminal would receive. !/[\p{Cc}\p{Cf}]/u.test(code.slice(0, -1)); - const handleCopy = () => { - if (typeof navigator === "undefined" || navigator.clipboard == null) { - return; - } - void navigator.clipboard - .writeText(code) - .then(() => { - if (copiedTimerRef.current != null) { - clearTimeout(copiedTimerRef.current); - } - setCopied(true); - copiedTimerRef.current = setTimeout(() => { - setCopied(false); - copiedTimerRef.current = null; - }, 1200); - }) - .catch((cause) => { - reportMarkdownActionFailure( - { - operation: "copy-code-block", - language, - ...(fenceTitle ? { fenceTitle } : {}), - }, - cause, - ); - }); - }; - - useEffect( - () => () => { - if (copiedTimerRef.current != null) { - clearTimeout(copiedTimerRef.current); - copiedTimerRef.current = null; - } - }, - [], - ); + const handleCopy = () => copyToClipboard(code); const copyButton = ( @@ -358,32 +333,33 @@ function UncachedShikiCodeBlock({ : null; useEffect(() => () => incrementalHighlight?.dispose(), [incrementalHighlight]); const highlighted = (() => { - try { - if (incrementalHighlight) return incrementalHighlight(code); - return preserveLines - ? highlightCodeToHast(highlighter, code, language, themeName) - : highlighter.codeToHtml(code, { lang: language, theme: themeName }); - } catch (error) { - console.warn( - `Code highlighting failed for language "${language}", falling back to plain text.`, - error instanceof Error ? error.message : error, - ); - return preserveLines - ? highlightCodeToHast(highlighter, code, "text", themeName) - : highlighter.codeToHtml(code, { lang: "text", theme: themeName }); - } - })(); - - useEffect(() => { + const result = (() => { + try { + if (incrementalHighlight) return incrementalHighlight(code); + return preserveLines + ? highlightCodeToHast(highlighter, code, language, themeName) + : highlighter.codeToHtml(code, { lang: language, theme: themeName }); + } catch (error) { + console.warn( + `Code highlighting failed for language "${language}", falling back to plain text.`, + error instanceof Error ? error.message : error, + ); + return preserveLines + ? highlightCodeToHast(highlighter, code, "text", themeName) + : highlighter.codeToHtml(code, { lang: "text", theme: themeName }); + } + })(); + // A finished block is cached for the next mount; a streaming one is still changing. if (!isStreaming) { - const highlightedHtml = typeof highlighted === "string" ? highlighted : toHtml(highlighted); + const highlightedHtml = typeof result === "string" ? result : toHtml(result); highlightedCodeCache.set( cacheKey, highlightedHtml, estimateHighlightedSize(highlightedHtml, code), ); } - }, [cacheKey, code, highlighted, isStreaming]); + return result; + })(); return typeof highlighted === "string" ? (
diff --git a/apps/web/src/components/chat/MarkdownCopy.browser.test.tsx b/apps/web/src/components/chat/MarkdownCopy.browser.test.tsx new file mode 100644 index 0000000000000..9b50489828a92 --- /dev/null +++ b/apps/web/src/components/chat/MarkdownCopy.browser.test.tsx @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from "vite-plus/test"; +import { page, userEvent } from "vite-plus/test/browser"; + +import "~/index.css"; +import { render } from "../../../test/render"; +import { MarkdownCodeBlock } from "./MarkdownCodeBlock"; +import { MarkdownTable } from "./MarkdownTable"; + +// Insecure origins (a LAN or tailnet address over plain http) have no async clipboard. +function withoutAsyncClipboard() { + const original = Object.getOwnPropertyDescriptor(Navigator.prototype, "clipboard"); + Object.defineProperty(navigator, "clipboard", { configurable: true, value: undefined }); + const execCommand = vi.spyOn(document, "execCommand").mockReturnValue(true); + return { + execCommand, + [Symbol.dispose]() { + execCommand.mockRestore(); + Reflect.deleteProperty(navigator, "clipboard"); + if (original) Object.defineProperty(Navigator.prototype, "clipboard", original); + }, + }; +} + +describe("markdown copy without the async clipboard", () => { + it("copies a code block through the execCommand fallback", async () => { + await using screen = await render( + + 
+          echo hi
+
+ , + // The block renders no links and reads no route. + { router: false }, + ); + using clipboard = withoutAsyncClipboard(); + + await userEvent.click(screen.getByRole("button", { name: "Copy code" })); + await expect.element(screen.getByRole("button", { name: "Copied" })).toBeInTheDocument(); + expect(clipboard.execCommand).toHaveBeenCalledWith("copy"); + }); + + it("copies a table through the execCommand fallback", async () => { + await using screen = await render( + + + + Name + + + + + Ada + + + , + // The table renders no links and reads no route. + { router: false }, + ); + using clipboard = withoutAsyncClipboard(); + + await userEvent.click(screen.getByRole("button", { name: "Copy table" })); + await userEvent.click(page.getByRole("menuitem", { name: "Copy as Markdown" })); + await expect.element(screen.getByRole("button", { name: "Copied" })).toBeInTheDocument(); + expect(clipboard.execCommand).toHaveBeenCalledWith("copy"); + }); +}); diff --git a/apps/web/src/components/chat/MarkdownTable.tsx b/apps/web/src/components/chat/MarkdownTable.tsx index 4f75a74193cba..dbeafeb5dcc4a 100644 --- a/apps/web/src/components/chat/MarkdownTable.tsx +++ b/apps/web/src/components/chat/MarkdownTable.tsx @@ -1,6 +1,7 @@ import { Check, Copy, Maximize2, Minimize2 } from "lucide"; -import { useEffect, useLayoutEffect, useRef, useState, type ComponentProps } from "react"; +import { useRef, useState, type ComponentProps } from "react"; +import { useCopyToClipboard } from "../../hooks/useCopyToClipboard"; import { getClientSettings } from "../../hooks/useSettings"; import { observeResize } from "../../lib/observeResize"; import { @@ -41,78 +42,54 @@ function tableHasClippedCells(container: HTMLElement, table: HTMLTableElement): export function MarkdownTable({ children, ...props }: ComponentProps<"table">) { const containerRef = useRef(null); - const tableRef = useRef(null); const [expanded, setExpanded] = useState(() => getClientSettings().wordWrap); const [hasClippedCells, setHasClippedCells] = useState(false); - const [copied, setCopied] = useState(false); - const copiedTimerRef = useRef | null>(null); + const { copyToClipboard, isCopied: copied } = useCopyToClipboard<"markdown" | "csv">({ + timeout: 1200, + onError: (cause, format) => + reportMarkdownActionFailure({ operation: "copy-table", format }, cause), + }); const expandLabel = expanded ? "Collapse table cells" : "Expand table cells"; const copyLabel = copied ? "Copied" : "Copy table"; - const measureClippedCells = () => { - const container = containerRef.current; - const table = tableRef.current; + // React calls this with null when a previous attach returned no cleanup. + const attachContainer = (container: HTMLDivElement | null) => { + containerRef.current = container; + const table = container?.querySelector("table"); if (!container || !table) return; - setHasClippedCells(tableHasClippedCells(container, table)); + const measure = () => setHasClippedCells(tableHasClippedCells(container, table)); + measure(); + // A streamed cell can grow until it clips while the table keeps its size, and a + // narrower column can clip a cell without this component rendering. + const mutations = new MutationObserver(measure); + mutations.observe(table, { childList: true, characterData: true, subtree: true }); + const stopResize = observeResize(table, measure); + return () => { + mutations.disconnect(); + stopResize(); + containerRef.current = null; + }; }; - // A streamed cell can grow until it clips while the table keeps its size, - // so remeasure after every render, not only on resize. - useLayoutEffect(measureClippedCells); - - // Column width changes can clip a cell without rendering this table. - useLayoutEffect(() => { - const table = tableRef.current; - if (!table) return; - return observeResize(table, measureClippedCells); - }, [measureClippedCells]); - const handleCopy = (format: "markdown" | "csv") => { const table = containerRef.current?.querySelector("table"); - if (!table || typeof navigator === "undefined" || navigator.clipboard == null) { - return; - } - const text = + if (!table) return; + copyToClipboard( format === "markdown" ? serializeTableElementToMarkdown(table) - : serializeTableElementToCsv(table); - void navigator.clipboard - .writeText(text) - .then(() => { - if (copiedTimerRef.current != null) { - clearTimeout(copiedTimerRef.current); - } - setCopied(true); - copiedTimerRef.current = setTimeout(() => { - setCopied(false); - copiedTimerRef.current = null; - }, 1200); - }) - .catch((cause) => { - reportMarkdownActionFailure({ operation: "copy-table", format }, cause); - }); + : serializeTableElementToCsv(table), + format, + ); }; - useEffect( - () => () => { - if (copiedTimerRef.current != null) { - clearTimeout(copiedTimerRef.current); - copiedTimerRef.current = null; - } - }, - [], - ); - return (
- - {children} -
+ {children}
 {/* Stays within the row's px-1: the chat lane clips sideways overflow. */}
diff --git a/apps/web/src/components/chat/McpAppFrame.tsx b/apps/web/src/components/chat/McpAppFrame.tsx index 3da6b4d3dae3f..430a6b7412640 100644 --- a/apps/web/src/components/chat/McpAppFrame.tsx +++ b/apps/web/src/components/chat/McpAppFrame.tsx @@ -19,16 +19,18 @@ import { type McpAppReference, } from "@t3tools/shared/mcpApp"; import { Minimize2Icon } from "lucide-react"; -import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState, type FocusEvent, type ToggleEvent } from "react"; import { flushSync } from "react-dom"; -import { useAssetUrlRefresh, useAssetUrlState } from "~/assets/assetUrls"; +import { useLongLivedAssetUrl } from "~/assets/longLivedAssetUrls"; import { APP_VERSION } from "~/branding"; import { isConfirmDialogActive, requestConfirmDialog } from "~/confirmDialog"; import { useHtmlRenderTheme } from "~/hooks/useHtmlRenderTheme"; +import { useStableCallback } from "~/hooks/useStableCallback"; import { Button } from "~/components/ui/button"; import { isElectron } from "~/env"; import { cn } from "~/lib/utils"; +import { useEnvironmentQuery } from "~/state/query"; import { useTurnItemDetail } from "~/state/queries"; import { mcpAppEnvironment } from "~/state/mcpApps"; import { useAtomCommand } from "~/state/use-atom-command"; @@ -43,7 +45,6 @@ const commandFailure = (result: { ); }; -const MIN_URL_LIFE_MS = 5 * 60_000; const fullscreenSupported = typeof HTMLElement !== "undefined" && "popover" in HTMLElement.prototype; @@ -51,6 +52,19 @@ const MAX_DOWNLOAD_BYTES = 25 * 1024 * 1024; // Exclude activation left by input elsewhere in T3. const PAGE_ACTIVATION_MS = 5_000; +// Frame input does not bubble here. An app can refocus itself, so focus +// alone cannot exclude activation from input elsewhere in T3. +let lastPageInputAt = Number.NEGATIVE_INFINITY; +if (typeof window !== "undefined") { + const note = () => { + lastPageInputAt = performance.now(); + }; + if (navigator.userActivation?.isActive) note(); + for (const type of ["pointerdown", "pointerup", "touchend", "keydown"] as const) { + window.addEventListener(type, note, true); + } +} + function saveBlob(blob: Blob, name: string) { const url = URL.createObjectURL(blob); const link = document.createElement("a"); @@ -82,15 +96,22 @@ export function McpAppFrame(props: { const [displayMode, setDisplayMode] = useState("inline"); const [closed, setClosed] = useState(false); const [documentGeneration, setDocumentGeneration] = useState(0); - useLayoutEffect(() => { - const box = boxRef.current; - if (!box) return; + // Closing and reopening replaces the box, which re-attaches this. + const attachBox = (box: HTMLDivElement) => { + boxRef.current = box; setWidth(box.clientWidth); - return observeResize(box, ([entry]) => { + const stopResize = observeResize(box, ([entry]) => { if (entry) setWidth(Math.round(entry.contentRect.width)); + // Full screen reports the viewport, which can change height alone. + if (fullscreenSupported && box.matches(":popover-open")) { + hostRef.current?.updateHostContext(); + } }); - // oxlint-disable-next-line react/exhaustive-effect-dependencies -- Closing and reopening replaces the box. - }, [closed]); + return () => { + stopResize(); + boxRef.current = null; + }; + }; const resource = { _tag: "attachment" as const, @@ -100,27 +121,10 @@ export function McpAppFrame(props: { disposition: "inline" as const, renderIntent: "mcp-app" as const, }; - const asset = useAssetUrlState(props.environmentId, resource); - const refreshAsset = useAssetUrlRefresh(props.environmentId, resource); - // Retain the URL to avoid reloads. Re-mint near expiry; frames cannot report failed loads. + // The frame keeps its first URL: a re-minted one would reload the app. const [src, setSrc] = useState(null); - const [mintFailed, setMintFailed] = useState(false); - const minting = useRef(false); - const cachedUrl = asset._tag === "Success" ? asset.url : null; - const cachedExpiresAt = asset._tag === "Success" ? asset.expiresAt : 0; - useEffect(() => { - if (src !== null || cachedUrl === null || minting.current) return; - if (cachedExpiresAt - Date.now() > MIN_URL_LIFE_MS) { - // oxlint-disable-next-line react/set-state-in-effect -- Adopts the cached URL once it is known to last. - setSrc(cachedUrl); - return; - } - minting.current = true; - void refreshAsset().then( - (url) => (url === null ? setMintFailed(true) : setSrc(url)), - () => setMintFailed(true), - ); - }, [src, cachedUrl, cachedExpiresAt, refreshAsset]); + const asset = useLongLivedAssetUrl(src === null ? props.environmentId : null, resource); + if (src === null && asset._tag === "Success") setSrc(asset.url); const detail = useTurnItemDetail({ environmentId: props.environmentId, @@ -142,14 +146,19 @@ export function McpAppFrame(props: { const callTool = useAtomCommand(mcpAppEnvironment.callTool, { reportFailure: false }); const toolInfo = useAtomCommand(mcpAppEnvironment.toolInfo, { reportFailure: false }); + const toolDefinition = useEnvironmentQuery( + mcpAppEnvironment.toolInfoQuery({ + environmentId: props.environmentId, + input: { threadId: props.threadId, itemId: props.itemId, name: app.tool }, + }), + ).data?.tool; const readResource = useAtomCommand(mcpAppEnvironment.readResource, { reportFailure: false }); const updateModelContext = useAtomCommand(mcpAppEnvironment.updateModelContext, { reportFailure: false, }); - const [toolDefinition, setToolDefinition] = useState(undefined); // Read live values without rebuilding the host and losing the app session. - const live = { + const readLive = useStableCallback(() => ({ theme, width, props, @@ -159,80 +168,42 @@ export function McpAppFrame(props: { updateModelContext, displayMode, toolDefinition, - }; + })); const hostRef = useRef(null); - const latest = useRef(live); - useEffect(() => { - latest.current = live; - }); - - // Frame input does not bubble here. An app can refocus itself, so focus - // alone cannot exclude activation from input elsewhere in T3. - const lastPageInputAt = useRef(Number.NEGATIVE_INFINITY); - useEffect(() => { - const note = () => { - lastPageInputAt.current = performance.now(); - }; - if (navigator.userActivation?.isActive) note(); - const types = ["pointerdown", "pointerup", "touchend", "keydown"] as const; - for (const type of types) window.addEventListener(type, note, true); - return () => { - for (const type of types) window.removeEventListener(type, note, true); - }; - }, []); - useEffect(() => { - const box = boxRef.current; - if (box === null || !fullscreenSupported) return; - const shown = box.matches(":popover-open"); - if (displayMode === "fullscreen" && !shown) box.showPopover(); - if (displayMode !== "fullscreen" && shown) box.hidePopover(); - if (displayMode === "fullscreen") { - latest.current.props.onFullscreenChange?.(true); - return () => latest.current.props.onFullscreenChange?.(false); - } - }, [displayMode]); const [stepAsideFor, setStepAsideFor] = useState(props.awaitingUser); if (stepAsideFor !== props.awaitingUser) { setStepAsideFor(props.awaitingUser); if (props.awaitingUser === true) setDisplayMode("inline"); } - useEffect(() => { - if (displayMode !== "fullscreen") return; - const leave = () => setDisplayMode("inline"); - const onFocus = (event: FocusEvent) => { - if (event.target instanceof Node && !boxRef.current?.contains(event.target)) leave(); - }; - const onKey = (event: KeyboardEvent) => { - if (event.key === "Escape") leave(); - }; - const onResize = () => hostRef.current?.updateHostContext(); - document.addEventListener("focusin", onFocus); - window.addEventListener("keydown", onKey); - window.addEventListener("resize", onResize); + // The box is in the top layer exactly while its full-screen chrome is + // mounted, so every way out of full screen (including the step-aside above, + // decided during render) closes the popover and releases the timeline pin. + const attachFullscreenChrome = (chrome: HTMLDivElement) => { + const box = chrome.parentElement; + if (box === null) return; + if (!box.matches(":popover-open")) box.showPopover(); + readLive().props.onFullscreenChange?.(true); return () => { - document.removeEventListener("focusin", onFocus); - window.removeEventListener("keydown", onKey); - window.removeEventListener("resize", onResize); + if (box.matches(":popover-open")) box.hidePopover(); + readLive().props.onFullscreenChange?.(false); }; - }, [displayMode]); + }; + // Light dismiss (Escape) closes the popover without React. + const onBoxToggle = (event: ToggleEvent) => { + if (event.newState === "closed") setDisplayMode("inline"); + }; + const onBoxBlur = (event: FocusEvent) => { + const next = event.relatedTarget; + if ( + displayMode === "fullscreen" && + next instanceof Node && + !event.currentTarget.contains(next) + ) { + setDisplayMode("inline"); + } + }; - useEffect(() => { - let cancelled = false; - void latest.current - .toolInfo({ - environmentId: props.environmentId, - input: { threadId: props.threadId, itemId: props.itemId, name: app.tool }, - }) - .then((info) => { - if (!cancelled && info._tag === "Success" && info.value.tool !== undefined) { - setToolDefinition(info.value.tool); - } - }); - return () => { - cancelled = true; - }; - }, [props.environmentId, props.threadId, props.itemId, app.tool]); // Navigation retains the frame window; reject second loads to prevent impersonation. const loads = useRef({ generation: 0, count: 0 }); const onFrameLoad = () => { @@ -246,10 +217,11 @@ export function McpAppFrame(props: { } }; - useEffect(() => { - if (src === null) return; + // The host belongs to this frame element, which is replaced when the document is reopened. + const attachFrame = (frame: HTMLIFrameElement) => { + frameRef.current = frame; const hostContext = (): McpAppHostContext => { - const current = latest.current; + const current = readLive(); return { theme: current.theme.appearance, styles: { variables: mcpAppStyleVariables(current.theme.variables) }, @@ -287,7 +259,7 @@ export function McpAppFrame(props: { }; const target = () => frameRef.current?.contentWindow ?? null; const scope = () => { - const { environmentId, threadId, itemId } = latest.current.props; + const { environmentId, threadId, itemId } = readLive().props; return { environmentId, input: { threadId, itemId } }; }; const host = makeMcpAppHost({ @@ -298,7 +270,7 @@ export function McpAppFrame(props: { hostContext, callTool: async ({ name, arguments: args }) => { const { environmentId, input } = scope(); - const info = await latest.current.toolInfo({ environmentId, input: { ...input, name } }); + const info = await readLive().toolInfo({ environmentId, input: { ...input, name } }); if (info._tag !== "Success") throw commandFailure(info); if (!info.value.callable) throw new McpAppHostRefusal("This app cannot call that tool."); if (!info.value.readOnly) { @@ -307,7 +279,7 @@ export function McpAppFrame(props: { ); if (approved !== true) throw new McpAppHostRefusal("Declined by the user."); } - const result = await latest.current.callTool({ + const result = await readLive().callTool({ environmentId, input: { ...input, name, arguments: args }, }); @@ -316,7 +288,7 @@ export function McpAppFrame(props: { }, readResource: async ({ uri }) => { const { environmentId, input } = scope(); - const result = await latest.current.readResource({ + const result = await readLive().readResource({ environmentId, input: { ...input, uri }, }); @@ -330,7 +302,7 @@ export function McpAppFrame(props: { window.open(url, "_blank", "noopener,noreferrer"); }, sendMessage: async (text) => { - const send = latest.current.props.onSendMessage; + const send = readLive().props.onSendMessage; if (send === undefined) throw new McpAppHostRefusal("Messages are not available here."); const approved = await ask(`Send this message from ${app.server}?\n${text}`); if (approved !== true) throw new McpAppHostRefusal("Declined by the user."); @@ -338,12 +310,12 @@ export function McpAppFrame(props: { }, updateModelContext: async (context) => { const { environmentId, input } = scope(); - const result = await latest.current.updateModelContext({ + const result = await readLive().updateModelContext({ environmentId, input: { ...input, ...context, - conversationThreadId: latest.current.props.conversationThreadId, + conversationThreadId: readLive().props.conversationThreadId, }, }); if (result._tag !== "Success") throw commandFailure(result); @@ -352,14 +324,15 @@ export function McpAppFrame(props: { const otherFullscreen = document.querySelector("[data-mcp-app-fullscreen]"); if ( mode === "fullscreen" && - (document.activeElement !== frameRef.current || + (!fullscreenSupported || + document.activeElement !== frameRef.current || !navigator.userActivation?.isActive || - performance.now() - lastPageInputAt.current < PAGE_ACTIVATION_MS || - latest.current.props.awaitingUser === true || + performance.now() - lastPageInputAt < PAGE_ACTIVATION_MS || + readLive().props.awaitingUser === true || isConfirmDialogActive() || (otherFullscreen !== null && otherFullscreen !== boxRef.current)) ) { - return latest.current.displayMode; + return readLive().displayMode; } setDisplayMode(mode); return mode; @@ -375,7 +348,7 @@ export function McpAppFrame(props: { bytes = file.bytes; } else { const { environmentId, input } = scope(); - const read = await latest.current.readResource({ + const read = await readLive().readResource({ environmentId, input: { ...input, uri: file.uri }, }); @@ -393,7 +366,7 @@ export function McpAppFrame(props: { } }, onRequestTeardown: () => { - if (latest.current.displayMode === "fullscreen") { + if (readLive().displayMode === "fullscreen") { setDisplayMode("inline"); return; } @@ -401,7 +374,7 @@ export function McpAppFrame(props: { }, onSizeChanged: (size) => { // Ignore fullscreen height when restoring inline sizing. - if (latest.current.displayMode === "fullscreen") return; + if (readLive().displayMode === "fullscreen") return; if (size.height !== undefined) setHeight(clampMcpAppHeight(size.height)); }, }); @@ -414,9 +387,9 @@ export function McpAppFrame(props: { window.removeEventListener("message", receive); void host.teardown(); hostRef.current = null; + frameRef.current = null; }; - // oxlint-disable-next-line react/exhaustive-effect-dependencies -- A reopened document needs a new host. - }, [src, app, documentGeneration]); + }; useEffect(() => { hostRef.current?.updateHostContext(); @@ -436,7 +409,6 @@ export function McpAppFrame(props: { size="xs" variant="ghost" onClick={() => { - minting.current = false; setSrc(null); setDocumentGeneration((value) => value + 1); setClosed(false); @@ -453,9 +425,11 @@ export function McpAppFrame(props: { // Preserve inline row height during fullscreen to prevent timeline jumps.
{fullscreen ? ( -
+
{app.server}