From ebdf2f72cb444b0e2bd11004eb26939192b867cf Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 10 Nov 2026 12:52:04 +0100 Subject: [PATCH] fix(app): paint a long user message collapsed from its first frame MessagePreview learned it overflowed from onLayout, which arrives after the mount has painted on web and Fabric. Every mount (opening the chat, a prompt arriving, a row the list mounts again) painted the text hard-cut with no fade or Show more, then grew the row by the control a frame later. The expanded choice also lived in the row and reset with each remount. Measure the body in a layout effect so the first commit carries the control, and keep the expanded choice in a per-agent store the timeline provides, as tool calls already do. --- docs/agent-stream-disclosure.md | 13 ++ .../e2e/browser/user-message-preview.spec.ts | 142 ++++++++++++++++++ .../agent-stream/disclosure.browser.test.tsx | 23 ++- packages/app/src/agent-stream/view.tsx | 60 ++++---- packages/app/src/components/message.tsx | 5 +- .../user-message/expansion-context.tsx | 30 ++++ .../components/user-message/expansion.test.ts | 31 ++++ .../src/components/user-message/expansion.ts | 34 +++++ .../user-message/index.browser.test.tsx | 81 ++++++++++ .../app/src/components/user-message/index.tsx | 28 +++- 10 files changed, 398 insertions(+), 49 deletions(-) create mode 100644 packages/app/e2e/browser/user-message-preview.spec.ts create mode 100644 packages/app/src/components/user-message/expansion-context.tsx create mode 100644 packages/app/src/components/user-message/expansion.test.ts create mode 100644 packages/app/src/components/user-message/expansion.ts create mode 100644 packages/app/src/components/user-message/index.browser.test.tsx diff --git a/docs/agent-stream-disclosure.md b/docs/agent-stream-disclosure.md index 1d5caefcf2..d2ad7a454b 100644 --- a/docs/agent-stream-disclosure.md +++ b/docs/agent-stream-disclosure.md @@ -39,6 +39,19 @@ The trigger can only stay put when the list has scroll room for the change. When content fits the viewport, or a collapse at the end would need an offset past the end, the list ends at its end and the trigger moves by the difference. +## State that survives a remount + +The list mounts a row again whenever it opens the chat, brings a web row back into the history +window, or reassigns a native row's LegendList container. State held in the row resets each time. +Keep a disclosure's open state in the per-agent store the timeline provides, keyed by item id: +`ToolCallExpansionContext` for tool calls, `UserMessageExpansionContext` for long user messages. + +Decide a row's first painted shape in the commit that mounts it. `MessagePreview` measures its body +in a layout effect, which commits before paint on web and Fabric. `onLayout` lands after the first +frame: a long message measured there paints capped with no fade or show more, then grows by the +control's height a frame later. `onLayout` still tracks later size changes. A zero height comes from +a hidden panel and is ignored. + ## Web `holdRow` pins the row's trigger edge in the reading anchor and runs `apply`; the next reading diff --git a/packages/app/e2e/browser/user-message-preview.spec.ts b/packages/app/e2e/browser/user-message-preview.spec.ts new file mode 100644 index 0000000000..10ec6cc3c4 --- /dev/null +++ b/packages/app/e2e/browser/user-message-preview.spec.ts @@ -0,0 +1,142 @@ +import type { Page } from "@playwright/test"; +import { expect, test } from "../support/fixtures"; +import { expectAgentIdle } from "../support/helpers/agent-stream"; +import { openAgentRoute, seedMockAgentWorkspace } from "../support/helpers/mock-agent"; + +// A long user message paints capped, with its fade and Show more, in the frame it mounts in. These +// tests sample every painted frame from before the chat opens and require each long message to +// show one state, the settled one, for its whole life on screen. + +interface PreviewFrame { + height: number; + fade: boolean; + control: string | null; +} + +declare global { + interface Window { + __previewFrames?: Record; + } +} + +function longPrompt(name: string) { + return Array.from({ length: 40 }, (_, line) => `${name} line ${line + 1}`).join("\n"); +} + +async function recordPreviewFrames(page: Page) { + await page.addInitScript(() => { + const frames: Record = {}; + window.__previewFrames = frames; + const sample = () => { + for (const row of document.querySelectorAll('[data-testid="user-message"]')) { + const rect = row.getBoundingClientRect(); + if (rect.height === 0) continue; + const text = row.querySelector('[data-message-text="true"]'); + if (!text) continue; + const name = (text.textContent ?? "").split(" line ")[0]; + // The preview is the ancestor that carries the height cap; the fade paints inside it. + let preview = text.parentElement; + while (preview && preview !== row && getComputedStyle(preview).maxHeight === "none") { + preview = preview.parentElement; + } + const control = [...row.querySelectorAll("*")].find( + (node) => node.children.length === 0 && /^Show (more|less)$/.test(node.textContent ?? ""), + ); + const frame = { + height: Math.round(rect.height), + fade: preview !== null && preview !== row && preview.querySelector("svg") !== null, + control: control?.textContent ?? null, + }; + const seen = (frames[name] ??= []); + if (JSON.stringify(seen.at(-1)) !== JSON.stringify(frame)) seen.push(frame); + } + requestAnimationFrame(sample); + }; + requestAnimationFrame(sample); + }); +} + +async function expectPaintedCollapsedFromFirstFrame(page: Page, name: string) { + const message = page.getByTestId("user-message").filter({ hasText: `${name} line 1` }); + await expect(message.getByRole("button", { name: "Show more" })).toBeVisible(); + // Let any late measurement land before reading the frames. + await page.waitForTimeout(500); + const frames = await page.evaluate((key) => window.__previewFrames?.[key] ?? [], name); + expect(frames).toEqual([{ height: expect.any(Number), fade: true, control: "Show more" }]); +} + +test.describe("long user message preview", () => { + test("opening a chat paints its long message collapsed from the first frame", async ({ + page, + }) => { + const agent = await seedMockAgentWorkspace({ + repoPrefix: "user-message-open-", + title: "Preview open", + initialPrompt: longPrompt("Opened prompt"), + }); + try { + await agent.client.waitForFinish(agent.agentId, 30_000); + await recordPreviewFrames(page); + await openAgentRoute(page, agent); + await expectPaintedCollapsedFromFirstFrame(page, "Opened prompt"); + + await page.reload(); + await expectPaintedCollapsedFromFirstFrame(page, "Opened prompt"); + } finally { + await agent.cleanup(); + } + }); + + test("a long message arriving in an open chat paints collapsed from the first frame", async ({ + page, + }) => { + const agent = await seedMockAgentWorkspace({ + repoPrefix: "user-message-arrive-", + title: "Preview arrive", + initialPrompt: "short", + }); + try { + await agent.client.waitForFinish(agent.agentId, 30_000); + await recordPreviewFrames(page); + await openAgentRoute(page, agent); + await expectAgentIdle(page); + + await agent.client.sendAgentMessage(agent.agentId, longPrompt("Arrived prompt")); + await expectPaintedCollapsedFromFirstFrame(page, "Arrived prompt"); + } finally { + await agent.cleanup(); + } + }); + + test("a long message that arrived while its tab was hidden paints collapsed on return", async ({ + page, + }) => { + const agent = await seedMockAgentWorkspace({ + repoPrefix: "user-message-hidden-", + title: "Preview first", + initialPrompt: "short", + }); + try { + await agent.client.createAgent({ + provider: "mock", + cwd: agent.cwd, + workspaceId: agent.workspaceId, + title: "Preview second", + modeId: "load-test", + model: "e2e-fast-stream", + }); + await agent.client.waitForFinish(agent.agentId, 30_000); + await recordPreviewFrames(page); + await openAgentRoute(page, agent); + await expectAgentIdle(page); + + await page.getByRole("button", { name: "Preview second", exact: true }).click(); + await agent.client.sendAgentMessage(agent.agentId, longPrompt("Hidden prompt")); + await agent.client.waitForFinish(agent.agentId, 30_000); + await page.getByRole("button", { name: "Preview first", exact: true }).click(); + await expectPaintedCollapsedFromFirstFrame(page, "Hidden prompt"); + } finally { + await agent.cleanup(); + } + }); +}); diff --git a/packages/app/src/agent-stream/disclosure.browser.test.tsx b/packages/app/src/agent-stream/disclosure.browser.test.tsx index 74c61c4988..83e34621da 100644 --- a/packages/app/src/agent-stream/disclosure.browser.test.tsx +++ b/packages/app/src/agent-stream/disclosure.browser.test.tsx @@ -6,6 +6,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { i18n as testI18n } from "@/i18n/i18next"; import { PlanCard } from "@/components/plan-card"; import { MessagePreview } from "@/components/user-message"; +import { createUserMessageExpansion } from "@/components/user-message/expansion"; +import { UserMessageExpansionContext } from "@/components/user-message/expansion-context"; import { DisclosureRow } from "./disclosure"; import type { DisclosureTriggerEdge, StreamViewportHandle } from "./strategy"; @@ -72,26 +74,19 @@ const view = () => within(container); const preLine = { whiteSpace: "pre-line" } as const; -async function waitForLayout() { - await act(async () => { - for (let frame = 0; frame < 2; frame += 1) { - await new Promise((resolve) => requestAnimationFrame(resolve)); - } - }); -} - describe("timeline disclosures hold their row", () => { - it("routes user message show more and show less through the row's hold", async () => { + it("routes user message show more and show less through the row's hold", () => { const longText = Array.from({ length: 60 }, (_, line) => `Line ${line + 1}`).join("\n"); + const expansion = createUserMessageExpansion(); const holds = mountRow( "message-1", () => view().queryByRole("button", { name: "Show less" }) !== null, - - {longText} - , + + + {longText} + + , ); - // The preview learns it overflows from its first layout. - await waitForLayout(); act(() => view().getByRole("button", { name: "Show more" }).click()); expect(view().getByRole("button", { name: "Show less" })).toBeTruthy(); diff --git a/packages/app/src/agent-stream/view.tsx b/packages/app/src/agent-stream/view.tsx index 761e73bac6..7ea2c7e5d9 100644 --- a/packages/app/src/agent-stream/view.tsx +++ b/packages/app/src/agent-stream/view.tsx @@ -66,6 +66,10 @@ import { useAgentToolCallExpansion, useExpandedToolCallGroups, } from "@/tool-calls/expansion-context"; +import { + UserMessageExpansionContext, + useAgentUserMessageExpansion, +} from "@/components/user-message/expansion-context"; import { ToolCallSheetProvider } from "@/components/tool-call-sheet"; import { createStreamPresentation } from "./presentation"; import { OverviewToolCallGroupView } from "@/tool-calls/detail-level/overview/view"; @@ -382,6 +386,7 @@ const AgentStreamViewComponent = forwardRef>(new Set()); @@ -711,6 +716,7 @@ const AgentStreamViewComponent = forwardRef) => { return ( - {streamRenderStrategy.render({ - readingRowReplacements: presentation.readingRowReplacements, - agentId, - segments: renderModel.segments, - historyRowRevision, - liveHeadRowRevision: expandedToolCallGroups, - boundary, - renderers, - listEmptyComponent, - viewportRef, - routeBottomAnchorRequest, - isAuthoritativeHistoryReady, - isInitialHistoryLoading, - revealInitialHistory, - onNearBottomChange: nearBottom.set, - onReadingPositionChange: handleReadingPositionChange, - onNearHistoryStart: loadOlder, - isLoadingOlderHistory: isLoadingOlder, - hasOlderHistory: hasOlder, - olderHistoryProgressKey: progressKey, - scrollEnabled: streamScrollEnabled, - listStyle: stylesheet.list, - baseListContentContainerStyle: stylesheet.listContentContainer, - forwardListContentContainerStyle: stylesheet.forwardListContentContainer, - contentMaxWidth, - })} + + {streamRenderStrategy.render({ + readingRowReplacements: presentation.readingRowReplacements, + agentId, + segments: renderModel.segments, + historyRowRevision, + liveHeadRowRevision: expandedToolCallGroups, + boundary, + renderers, + listEmptyComponent, + viewportRef, + routeBottomAnchorRequest, + isAuthoritativeHistoryReady, + isInitialHistoryLoading, + revealInitialHistory, + onNearBottomChange: nearBottom.set, + onReadingPositionChange: handleReadingPositionChange, + onNearHistoryStart: loadOlder, + isLoadingOlderHistory: isLoadingOlder, + hasOlderHistory: hasOlder, + olderHistoryProgressKey: progressKey, + scrollEnabled: streamScrollEnabled, + listStyle: stylesheet.list, + baseListContentContainerStyle: stylesheet.listContentContainer, + forwardListContentContainerStyle: stylesheet.forwardListContentContainer, + contentMaxWidth, + })} + {sender ? : null} - + {hasImages ? ( {images.map((image) => ( diff --git a/packages/app/src/components/user-message/expansion-context.tsx b/packages/app/src/components/user-message/expansion-context.tsx new file mode 100644 index 0000000000..978390223d --- /dev/null +++ b/packages/app/src/components/user-message/expansion-context.tsx @@ -0,0 +1,30 @@ +import { createContext, useCallback, useContext, useState, useSyncExternalStore } from "react"; +import { createUserMessageExpansion, type UserMessageExpansion } from "./expansion"; + +export const UserMessageExpansionContext = createContext(null); + +/** One store per viewed agent; switching agents starts with every message collapsed. */ +export function useAgentUserMessageExpansion(agentId: string): UserMessageExpansion { + const [owner, setOwner] = useState(() => ({ + agentId, + expansion: createUserMessageExpansion(), + })); + if (owner.agentId === agentId) return owner.expansion; + const next = { agentId, expansion: createUserMessageExpansion() }; + setOwner(next); + return next.expansion; +} + +/** Subscribes one message to its own expanded state. */ +export function useUserMessageExpanded(itemId: string) { + const expansion = useContext(UserMessageExpansionContext); + if (!expansion) throw new Error("MessagePreview requires a UserMessageExpansionContext"); + const subscribe = useCallback( + (listener: () => void) => expansion.subscribe(itemId, listener), + [expansion, itemId], + ); + const getSnapshot = useCallback(() => expansion.isExpanded(itemId), [expansion, itemId]); + const expanded = useSyncExternalStore(subscribe, getSnapshot, getSnapshot); + const toggle = useCallback(() => expansion.toggle(itemId), [expansion, itemId]); + return { expanded, toggle }; +} diff --git a/packages/app/src/components/user-message/expansion.test.ts b/packages/app/src/components/user-message/expansion.test.ts new file mode 100644 index 0000000000..d77b020773 --- /dev/null +++ b/packages/app/src/components/user-message/expansion.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { createUserMessageExpansion } from "./expansion"; + +describe("user message expansion", () => { + it("toggles one message and notifies only that message", () => { + const expansion = createUserMessageExpansion(); + const notified: string[] = []; + expansion.subscribe("first", () => notified.push("first")); + expansion.subscribe("second", () => notified.push("second")); + + expansion.toggle("first"); + expect(expansion.isExpanded("first")).toBe(true); + expect(expansion.isExpanded("second")).toBe(false); + + expansion.toggle("first"); + expect(expansion.isExpanded("first")).toBe(false); + expect(notified).toEqual(["first", "first"]); + }); + + it("keeps a message's choice after its subscribers leave, as a remounting row does", () => { + const expansion = createUserMessageExpansion(); + const unsubscribe = expansion.subscribe("first", () => undefined); + expansion.toggle("first"); + unsubscribe(); + + const notified: string[] = []; + expansion.subscribe("first", () => notified.push("first")); + expect(expansion.isExpanded("first")).toBe(true); + expect(notified).toEqual([]); + }); +}); diff --git a/packages/app/src/components/user-message/expansion.ts b/packages/app/src/components/user-message/expansion.ts new file mode 100644 index 0000000000..12c0ab890f --- /dev/null +++ b/packages/app/src/components/user-message/expansion.ts @@ -0,0 +1,34 @@ +type Listener = () => void; + +/** + * Which of one agent's long user messages the user expanded past the preview. It lives with the + * timeline, not the row, so a row that remounts (a virtualized row coming back into view, a row + * reassigned to another list container) paints the choice the user made. A toggle notifies only + * that message's subscribers. + */ +export function createUserMessageExpansion() { + const expanded = new Set(); + const listeners = new Map>(); + return { + isExpanded: (itemId: string) => expanded.has(itemId), + subscribe(itemId: string, listener: Listener) { + let itemListeners = listeners.get(itemId); + if (!itemListeners) { + itemListeners = new Set(); + listeners.set(itemId, itemListeners); + } + itemListeners.add(listener); + return () => { + itemListeners.delete(listener); + if (itemListeners.size === 0) listeners.delete(itemId); + }; + }, + toggle(itemId: string) { + if (expanded.has(itemId)) expanded.delete(itemId); + else expanded.add(itemId); + for (const listener of listeners.get(itemId) ?? []) listener(); + }, + }; +} + +export type UserMessageExpansion = ReturnType; diff --git a/packages/app/src/components/user-message/index.browser.test.tsx b/packages/app/src/components/user-message/index.browser.test.tsx new file mode 100644 index 0000000000..81ac638b0e --- /dev/null +++ b/packages/app/src/components/user-message/index.browser.test.tsx @@ -0,0 +1,81 @@ +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { within } from "@testing-library/dom"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { i18n as testI18n } from "@/i18n/i18next"; +import { MessagePreview } from "./index"; +import { createUserMessageExpansion, type UserMessageExpansion } from "./expansion"; +import { UserMessageExpansionContext } from "./expansion-context"; + +// Load translations so the toggle exposes its real accessible name. +void testI18n; + +// App sources compile against the classic JSX runtime, which expects React on the global. +beforeEach(() => vi.stubGlobal("React", React)); + +let root: Root; +let container: HTMLDivElement; + +beforeEach(() => { + container = document.createElement("div"); + container.style.width = "400px"; + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); +}); + +const longText = Array.from({ length: 60 }, (_, line) => `Line ${line + 1}`).join("\n"); +const preLine = { whiteSpace: "pre-line" } as const; +const view = () => within(container); + +function renderMessage(expansion: UserMessageExpansion) { + act(() => + root.render( + + + {longText} + + , + ), + ); +} + +async function waitForLayout() { + await act(async () => { + for (let frame = 0; frame < 2; frame += 1) { + await new Promise((resolve) => requestAnimationFrame(resolve)); + } + }); +} + +describe("long user message preview", () => { + it("commits the control with the mount, so the first frame is the settled one", async () => { + renderMessage(createUserMessageExpansion()); + + // Nothing has painted yet: whatever is in the DOM now is the first frame. + expect(view().getByRole("button", { name: "Show more" })).toBeTruthy(); + const firstFrameHeight = container.getBoundingClientRect().height; + + await waitForLayout(); + expect(view().getByRole("button", { name: "Show more" })).toBeTruthy(); + expect(container.getBoundingClientRect().height).toBe(firstFrameHeight); + }); + + it("paints the user's expanded choice when the row remounts", async () => { + const expansion = createUserMessageExpansion(); + renderMessage(expansion); + act(() => view().getByRole("button", { name: "Show more" }).click()); + await waitForLayout(); + const expandedHeight = container.getBoundingClientRect().height; + + act(() => root.render(null)); + renderMessage(expansion); + + expect(view().getByRole("button", { name: "Show less" })).toBeTruthy(); + expect(container.getBoundingClientRect().height).toBe(expandedHeight); + }); +}); diff --git a/packages/app/src/components/user-message/index.tsx b/packages/app/src/components/user-message/index.tsx index dc73c517ca..098c5b6990 100644 --- a/packages/app/src/components/user-message/index.tsx +++ b/packages/app/src/components/user-message/index.tsx @@ -1,4 +1,4 @@ -import { useState, useCallback, useMemo, type ReactNode } from "react"; +import { useState, useCallback, useLayoutEffect, useMemo, useRef, type ReactNode } from "react"; import { View, type LayoutChangeEvent, type ViewStyle } from "react-native"; import { StyleSheet } from "react-native-unistyles"; import { useTranslation } from "react-i18next"; @@ -10,6 +10,7 @@ import { useSessionStore } from "@/stores/session-store"; import { navigateToAgent } from "@/utils/navigate-to-agent"; import { isNative } from "@/constants/platform"; import { useDisclosureHold } from "@/agent-stream/disclosure"; +import { useUserMessageExpanded } from "./expansion-context"; const PREVIEW_HEIGHT = 240; // Show more and show less sit under the text they open and close. @@ -21,24 +22,35 @@ const previewStyle: ViewStyle = { overflow: isNative ? "scroll" : "hidden", }; -export function MessagePreview({ children }: { children: ReactNode }) { +/** Caps a long message at the preview height, with a control to read the rest. */ +export function MessagePreview({ itemId, children }: { itemId: string; children: ReactNode }) { const { t } = useTranslation(); - const [expanded, setExpanded] = useState(false); + const { expanded, toggle: toggleExpanded } = useUserMessageExpanded(itemId); + const bodyRef = useRef(null); const [overflowing, setOverflowing] = useState(false); + // A hidden panel lays out at zero height, which says nothing about the message. + const applyBodyHeight = useCallback((height: number) => { + if (height > 0) setOverflowing(height > PREVIEW_HEIGHT); + }, []); + // Measure in the commit that mounts the body: an update from a layout effect paints with it, so + // the first frame already shows the fade and the control. `onLayout` only lands after paint. + useLayoutEffect(() => { + applyBodyHeight(bodyRef.current!.getBoundingClientRect().height); + }, [applyBodyHeight]); const measure = useCallback( - (event: LayoutChangeEvent) => setOverflowing(event.nativeEvent.layout.height > PREVIEW_HEIGHT), - [], + (event: LayoutChangeEvent) => applyBodyHeight(event.nativeEvent.layout.height), + [applyBodyHeight], ); const hold = useDisclosureHold(); const toggle = useCallback( - () => hold(() => setExpanded((value) => !value), TRIGGER_BELOW_CONTENT), - [hold], + () => hold(toggleExpanded, TRIGGER_BELOW_CONTENT), + [hold, toggleExpanded], ); const accessibilityState = useMemo(() => ({ expanded }), [expanded]); return ( <> - + {children} {overflowing && !expanded ? (