diff --git a/apps/app/bundle-budget.json b/apps/app/bundle-budget.json index 7032e9dcae..b667739ad9 100644 --- a/apps/app/bundle-budget.json +++ b/apps/app/bundle-budget.json @@ -47,7 +47,7 @@ "to print the static chain that pulled a package into the closure." ], "maxBootBytes": 1723617, - "maxBootBrotliBytes": 479067, + "maxBootBrotliBytes": 429072, "forbiddenBootPackages": [ "@pierre/diffs", "@pierre/theming", diff --git a/apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx b/apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx index 7ec3b26225..3af15d9e79 100644 --- a/apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx +++ b/apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx @@ -108,6 +108,18 @@ function withElementClientHeight( } } +// Waits out one scheduled rAF pass so "the pass ran and did nothing" is +// distinguishable from "the pass has not run yet". +async function flushScheduledViewportPass() { + await act(async () => { + await new Promise((resolve) => { + window.requestAnimationFrame(() => { + window.requestAnimationFrame(() => resolve()); + }); + }); + }); +} + beforeEach(() => { vi.spyOn(window, "scrollTo").mockImplementation(() => {}); }); @@ -310,6 +322,142 @@ describe("useMobileVisualViewportHeight", () => { expect(window.scrollTo).not.toHaveBeenCalled(); }); }); + + it("writes shell geometry only when a pass computes new values", async () => { + const visualViewport = new FakeVisualViewport(); + visualViewport.offsetTop = 0; + await withFakeVisualViewport(visualViewport, async () => { + render(); + const shell = screen.getByTestId("shell"); + const shellHeightRoot = screen.getByTestId("shell-height-root"); + expect(shell.style.height).toBe("500px"); + const setShellHeightProperty = vi.spyOn( + shellHeightRoot.style, + "setProperty", + ); + + // Same geometry again: the pass must return before any style write, or + // every keyboard/URL-bar animation frame invalidates the whole tree. + act(() => { + visualViewport.dispatchEvent(new Event("resize")); + }); + await flushScheduledViewportPass(); + expect(setShellHeightProperty).not.toHaveBeenCalled(); + + act(() => { + visualViewport.height = 480; + visualViewport.dispatchEvent(new Event("resize")); + }); + await waitFor(() => expect(shell.style.height).toBe("480px")); + expect(setShellHeightProperty).toHaveBeenCalledTimes(1); + }); + }); + + it("reads the containing block only when the layout viewport can change", async () => { + const visualViewport = new FakeVisualViewport(); + visualViewport.offsetTop = 0; + let containingBlockReads = 0; + await withElementClientHeight( + document.body, + () => { + containingBlockReads += 1; + return 800; + }, + async () => + withFakeVisualViewport(visualViewport, async () => { + render(); + const shell = screen.getByTestId("shell"); + expect(shell.style.height).toBe("500px"); + const readsAfterMount = containingBlockReads; + + // Visual-viewport ticks pan or resize only the visual viewport; + // they must reuse the cached containing-block height instead of + // forcing a full-document layout per animation frame. + act(() => { + visualViewport.offsetTop = 40; + visualViewport.dispatchEvent(new Event("scroll")); + }); + await waitFor(() => expect(shell.style.top).toBe("40px")); + act(() => { + visualViewport.height = 460; + visualViewport.dispatchEvent(new Event("resize")); + }); + await waitFor(() => expect(shell.style.height).toBe("460px")); + expect(containingBlockReads).toBe(readsAfterMount); + + act(() => { + window.dispatchEvent(new Event("resize")); + }); + await waitFor(() => + expect(containingBlockReads).toBe(readsAfterMount + 1), + ); + }), + ); + }); + + it("runs a geometry pass when an editor is focused programmatically", async () => { + const visualViewport = new FakeVisualViewport(); + visualViewport.offsetTop = 0; + await withElementClientHeight( + document.body, + () => 500, + async () => + withFakeVisualViewport(visualViewport, async () => { + render(); + const shell = screen.getByTestId("shell"); + const editor = screen.getByTestId("editor"); + // Native layout matches the visual viewport: no override applied. + expect(shell.style.height).toBe(""); + + // The keyboard shortens the visual viewport around the same time + // the composer autofocuses, without any window resize; the focus + // pass must pick the change up on its own. + visualViewport.height = 300; + act(() => editor.focus()); + await waitFor(() => expect(shell.style.height).toBe("300px")); + }), + ); + }); + + it("ignores visual viewport pans without a keyboard or an applied override", async () => { + const visualViewport = new FakeVisualViewport(); + visualViewport.offsetTop = 0; + await withElementClientHeight( + document.body, + () => 500, + async () => + withFakeVisualViewport(visualViewport, async () => { + render(); + const shell = screen.getByTestId("shell"); + const editor = screen.getByTestId("editor"); + expect(shell.style.height).toBe(""); + + // A URL-bar pan with no keyboard: nothing to compensate. + act(() => { + visualViewport.offsetTop = 340; + visualViewport.dispatchEvent(new Event("scroll")); + }); + await flushScheduledViewportPass(); + expect(window.scrollTo).not.toHaveBeenCalled(); + expect(shell.style.top).toBe(""); + + // With a keyboard editor focused, the same pan is Safari's + // focus-reveal pan and must still be compensated. Let the pan + // settle and the focus-scheduled pass run first, so that only the + // scroll handler's keyboard branch can produce the compensation. + visualViewport.offsetTop = 0; + act(() => editor.focus()); + await flushScheduledViewportPass(); + expect(shell.style.top).toBe(""); + act(() => { + visualViewport.offsetTop = 340; + visualViewport.dispatchEvent(new Event("scroll")); + }); + await waitFor(() => expect(shell.style.top).toBe("340px")); + expect(window.scrollTo).toHaveBeenCalledWith(0, 0); + }), + ); + }); }); describe("shouldRestoreIOSViewportOnKeyboardDismissal", () => { diff --git a/apps/app/src/components/layout/useMobileVisualViewportHeight.ts b/apps/app/src/components/layout/useMobileVisualViewportHeight.ts index e628e7c05b..a987dee6bb 100644 --- a/apps/app/src/components/layout/useMobileVisualViewportHeight.ts +++ b/apps/app/src/components/layout/useMobileVisualViewportHeight.ts @@ -55,7 +55,22 @@ export function useMobileVisualViewportHeight( if (!shell || !shellHeightRoot || !enabled || !visualViewport) return; let animationFrame: number | null = null; + // The override last written to the shell, or null while none is applied. + // Writing shell `top`/`height` and the inherited `--bb-shell-height` + // invalidates computed style for the whole app tree, and passes run at + // visual-viewport event cadence (keyboard animation, URL-bar collapse), + // so a pass that recomputes unchanged geometry must not write at all. + let appliedOverride: { top: number; height: number } | null = null; + // Reading `document.body.clientHeight` forces a full-document layout. The + // shell's containing block only changes when the layout viewport does, so + // cache the read and mark it stale only on triggers that can resize the + // layout viewport — never on visualViewport ticks, which move or resize + // only the visual viewport. + let shellContainingBlockHeight = 0; + let shellContainingBlockHeightStale = true; const clearViewportOverride = () => { + if (appliedOverride === null) return; + appliedOverride = null; shell.style.removeProperty("top"); shell.style.removeProperty("height"); shellHeightRoot.style.removeProperty("--bb-shell-height"); @@ -68,12 +83,15 @@ export function useMobileVisualViewportHeight( } const visualViewportHeight = Math.round(visualViewport.height); - // `documentElement.clientHeight` is the visible viewport height for the - // root element, even when that root's actual CSS box extends behind an - // Android in-app browser toolbar. The body inherits the root box and - // therefore exposes the containing-block height the app shell really - // receives. - const shellContainingBlockHeight = document.body.clientHeight; + if (shellContainingBlockHeightStale) { + // `documentElement.clientHeight` is the visible viewport height for the + // root element, even when that root's actual CSS box extends behind an + // Android in-app browser toolbar. The body inherits the root box and + // therefore exposes the containing-block height the app shell really + // receives. + shellContainingBlockHeight = document.body.clientHeight; + shellContainingBlockHeightStale = false; + } const hasVisualViewportPan = visualViewport.offsetTop > 1 || window.scrollY > 0; if ( @@ -91,7 +109,16 @@ export function useMobileVisualViewportHeight( // compensation below also handles a visual-viewport-only pan. window.scrollTo(0, 0); } - shell.style.top = `${getVisualViewportPageTop(visualViewport)}px`; + const shellTop = getVisualViewportPageTop(visualViewport); + if ( + appliedOverride !== null && + appliedOverride.top === shellTop && + appliedOverride.height === visualViewportHeight + ) { + return; + } + appliedOverride = { top: shellTop, height: visualViewportHeight }; + shell.style.top = `${shellTop}px`; shell.style.height = `${visualViewportHeight}px`; // Fixed-position descendants cannot inherit the shell element's pixel // height. Publish the same correction through the existing shell-height @@ -108,6 +135,28 @@ export function useMobileVisualViewportHeight( } animationFrame = window.requestAnimationFrame(updateHeight); }; + // For triggers where the layout viewport may have changed: window resize, + // rotation, and an editor gaining focus — the pass that sizes the shell + // for the arriving keyboard must start from the real containing block, + // and these triggers are rare enough that the forced layout is fine. + const scheduleContainingBlockUpdate = () => { + shellContainingBlockHeightStale = true; + scheduleUpdate(); + }; + const handleVisualViewportScroll = () => { + // Keyboard-less visual-viewport pans (URL-bar collapse, momentum + // settling) don't change the containing block and need no override — + // the pan compensation exists for the keyboard focus-reveal pan. Only + // an already-applied override still has to track pans, because embedded + // browsers apply one without any keyboard. + if ( + appliedOverride === null && + !isKeyboardFocusTarget(document.activeElement) + ) { + return; + } + scheduleUpdate(); + }; // Safari with its bottom toolbar visible does not update the visual // viewport until the keyboard animation ends. Restore the normal shell @@ -124,13 +173,16 @@ export function useMobileVisualViewportHeight( }; const handleFocusIn = (event: FocusEvent) => { if (!isKeyboardFocusTarget(event.target)) return; - scheduleUpdate(); + // Programmatic focus (composer autofocus) can be the only trigger for a + // keyboard, so this must always schedule a full, freshly measured pass. + scheduleContainingBlockUpdate(); }; updateHeight(); visualViewport.addEventListener("resize", scheduleUpdate); - visualViewport.addEventListener("scroll", scheduleUpdate); - window.addEventListener("resize", scheduleUpdate); + visualViewport.addEventListener("scroll", handleVisualViewportScroll); + window.addEventListener("resize", scheduleContainingBlockUpdate); + window.addEventListener("orientationchange", scheduleContainingBlockUpdate); if (restoreImmediatelyOnKeyboardDismissal) { document.addEventListener("focusout", handleFocusOut); document.addEventListener("focusin", handleFocusIn); @@ -138,8 +190,12 @@ export function useMobileVisualViewportHeight( return () => { visualViewport.removeEventListener("resize", scheduleUpdate); - visualViewport.removeEventListener("scroll", scheduleUpdate); - window.removeEventListener("resize", scheduleUpdate); + visualViewport.removeEventListener("scroll", handleVisualViewportScroll); + window.removeEventListener("resize", scheduleContainingBlockUpdate); + window.removeEventListener( + "orientationchange", + scheduleContainingBlockUpdate, + ); if (restoreImmediatelyOnKeyboardDismissal) { document.removeEventListener("focusout", handleFocusOut); document.removeEventListener("focusin", handleFocusIn); diff --git a/apps/app/src/components/thread/timeline/ConversationMessageContent.tsx b/apps/app/src/components/thread/timeline/ConversationMessageContent.tsx index 4b372e139e..415d83be9d 100644 --- a/apps/app/src/components/thread/timeline/ConversationMessageContent.tsx +++ b/apps/app/src/components/thread/timeline/ConversationMessageContent.tsx @@ -54,7 +54,10 @@ import { import { turnRequestLabel } from "@bb/client-core"; import { splitStreamingMarkdown } from "./streaming-markdown-split.js"; import { TurnRequestLabel } from "./TurnRequestLabel.js"; -import { MessageActionBar } from "./MessageActionBar.js"; +import { + MessageActionBar, + PROSE_COLUMN_INSET_CLASS, +} from "./MessageActionBar.js"; import { ConversationMessageOverflowToggle, useIsOverflowing, @@ -678,7 +681,10 @@ function AssistantConversationMessage({ return (
{/* diff --git a/apps/app/src/components/thread/timeline/MessageActionBar.test.tsx b/apps/app/src/components/thread/timeline/MessageActionBar.test.tsx index 6eb9086c74..b4cda7ce11 100644 --- a/apps/app/src/components/thread/timeline/MessageActionBar.test.tsx +++ b/apps/app/src/components/thread/timeline/MessageActionBar.test.tsx @@ -16,6 +16,7 @@ import { computeMessageActionRowLayout, findMessageActionTooltipCollisionBoundary, MessageActionBar, + MessageColumnWidthContext, } from "./MessageActionBar"; afterEach(() => { @@ -638,6 +639,145 @@ describe("MessageActionBar", () => { }); }); +describe("MessageActionBar observer budget", () => { + /** Counts `ResizeObserver` constructions without ever delivering entries. */ + function spyResizeObserverConstructions(): () => number { + let constructions = 0; + class CountingResizeObserver { + constructor(_callback: ResizeObserverCallback) { + constructions += 1; + } + observe() {} + unobserve() {} + disconnect() {} + } + vi.stubGlobal("ResizeObserver", CountingResizeObserver); + return () => constructions; + } + + it("constructs only the column fallback observer for a mobile overflow bar without a provider", () => { + mockMobileCoarsePointer(); + const constructionCount = spyResizeObserverConstructions(); + render( +
+ +
, + ); + + // The overflow branch renders a constant layout, so the slot-width + // observer is skipped; only the column fallback remains. + expect(constructionCount()).toBe(1); + }); + + it("creates no per-bar observer on the mobile overflow branch under the shared column width", () => { + mockMobileCoarsePointer(); + const constructionCount = spyResizeObserverConstructions(); + render( + + + , + ); + expect(constructionCount()).toBe(0); + + // The shared width is what admits the in-place expansion: three 28px + // touch actions (100px with gaps) fit the 358px column comfortably. + fireEvent.click(screen.getByRole("button", { name: "Message actions" })); + expect( + screen + .getAllByRole("button") + .map((button) => button.getAttribute("aria-label")), + ).toEqual(["Copy message", "Add to chat", "Fork into new thread"]); + }); + + it("constructs only the slot observer for a desktop bar under the shared column width", () => { + const constructionCount = spyResizeObserverConstructions(); + render( + + + , + ); + expect(constructionCount()).toBe(1); + }); +}); + +describe("MessageActionBar shared column width", () => { + /** + * Mounts a mobile bar with three 28px touch actions (a 100px row, so the + * in-place expansion needs a 116px column with its 16px comfort margin) + * under the shared list width, taps "⋯", and reports whether the actions + * expanded in place rather than opening the popover. + */ + function expandsInPlaceAt({ + alignment, + listWidth, + }: { + alignment: "start" | "end"; + listWidth: number; + }): boolean { + const { container, unmount } = render( + + + , + ); + fireEvent.click(screen.getByRole("button", { name: "Message actions" })); + // The popover portals its items outside the container, so only a button + // inside the bar's own tree is the in-place row. + const popover = document.body.querySelector('[data-side="top"]'); + const inPlaceRow = within(container).queryByRole("button", { + name: "Fork into new thread", + }); + unmount(); + if (popover === null) { + expect(inPlaceRow).not.toBeNull(); + return true; + } + expect(inPlaceRow).toBeNull(); + return false; + } + + it("reads the assistant column as the shared list width minus its px-2 padding", () => { + mockMobileCoarsePointer(); + // The bar's own column observer reports the padded assistant column's + // content box: 16px narrower than the list root the shared measurement + // observes. The shared path has to land on the same threshold. + expect(expandsInPlaceAt({ alignment: "start", listWidth: 131 })).toBe( + false, + ); + expect(expandsInPlaceAt({ alignment: "start", listWidth: 132 })).toBe( + true, + ); + }); + + it("reads the unpadded user column at the full shared list width", () => { + mockMobileCoarsePointer(); + expect(expandsInPlaceAt({ alignment: "end", listWidth: 115 })).toBe(false); + expect(expandsInPlaceAt({ alignment: "end", listWidth: 116 })).toBe(true); + }); +}); + describe("computeMessageActionRowLayout", () => { const metrics = { actionWidth: 20, overflowTriggerWidth: 20 }; diff --git a/apps/app/src/components/thread/timeline/MessageActionBar.tsx b/apps/app/src/components/thread/timeline/MessageActionBar.tsx index eb189eee5b..ada6d2e683 100644 --- a/apps/app/src/components/thread/timeline/MessageActionBar.tsx +++ b/apps/app/src/components/thread/timeline/MessageActionBar.tsx @@ -1,5 +1,7 @@ import { + createContext, useCallback, + useContext, useEffect, useRef, useState, @@ -174,11 +176,18 @@ export function computeMessageActionRowLayout({ * Width of the action row's slot. A callback ref (rather than an object ref * plus a mount effect) so the observer re-attaches when the bar swaps between * its desktop and touch trees — an effect keyed on mount would keep observing - * the unmounted tree's detached node. + * the unmounted tree's detached node. `enabled: false` keeps the hook (and a + * branch-stable hook order) without constructing an observer, for branches + * whose layout never reads the width. */ -function useMeasuredWidth( - resolveTarget?: (node: HTMLElement) => Element | null, -): { +export function useMeasuredWidth({ + enabled, + resolveTarget, +}: { + enabled: boolean; + /** Measure a related element (e.g. the message column) instead of the attached node. */ + resolveTarget?: (node: HTMLElement) => Element | null; +}): { measureRef: (node: HTMLElement | null) => void; width: number | undefined; } { @@ -188,7 +197,7 @@ function useMeasuredWidth( (node: HTMLElement | null) => { observerRef.current?.disconnect(); observerRef.current = null; - if (node === null || typeof ResizeObserver === "undefined") { + if (!enabled || node === null || typeof ResizeObserver === "undefined") { return; } const target = resolveTarget ? resolveTarget(node) : node; @@ -204,11 +213,32 @@ function useMeasuredWidth( observer.observe(target); observerRef.current = observer; }, - [resolveTarget], + [enabled, resolveTarget], ); return { measureRef, width }; } +/** + * Timeline-list-level share of the message column width. + * + * Every top-level row's `[data-message-column]` is as wide as the list root, + * so one list-level measurement stands in for the per-bar column observers. + * The top-level `TimelineRowsList` measures its root once and provides that + * width here; each bar recovers its column's content box from it by + * subtracting its column's own padding (`PROSE_COLUMN_INSET_PX` for the + * assistant column), which is what its own observer would have reported. + * `null` — no provider (stories, isolated renders) or a nested, narrower list + * shadowing the top-level value — means no shared measurement applies and the + * bar observes its own column. + */ +export interface SharedMessageColumnWidth { + /** Measured width; undefined until the observer first reports. */ + width: number | undefined; +} + +export const MessageColumnWidthContext = + createContext(null); + /** * The message column this row belongs to — the full timeline width, which for * a right-aligned user message is much wider than its bubble. Module-level so @@ -366,6 +396,18 @@ const BUBBLE_ALIGN_OFFSET_CLASS = "right-[13px] max-md:pointer-coarse:right-[11px]"; // Prose rows have no bubble padding, so only the hit-box slack is corrected. const PROSE_ALIGN_INSET_CLASS = "-ml-1 max-md:pointer-coarse:-ml-1.5"; +/** + * Horizontal padding of the assistant (prose) `[data-message-column]` in + * ConversationMessageContent, as the class it applies and the pixels it + * removes from the column's content box (8px a side). A bar observing that + * column reads its content box, so the shared list-level width — the + * unpadded list root — is this much wider than what a `start` bar's own + * observer would report; the bar subtracts it to land on the same number. + * The user column is unpadded (its bubble insets itself), so `end` bars take + * the shared width as is. Keep the pair in sync. + */ +export const PROSE_COLUMN_INSET_CLASS = "px-2"; +const PROSE_COLUMN_INSET_PX = 16; export function findMessageActionTooltipCollisionBoundary( node: HTMLElement | null, @@ -478,9 +520,29 @@ export function MessageActionBar({ const [collisionBoundary, setCollisionBoundary] = useState< HTMLElement | undefined >(); - const { measureRef, width: availableWidth } = useMeasuredWidth(); - const { measureRef: measureColumnRef, width: columnWidth } = - useMeasuredWidth(resolveMessageColumn); + const useMobileOverflowPopover = isCompactViewport && isPointerCoarse; + // The mobile overflow branch lays out a constant row (every action behind + // the "⋯" trigger), so the measured slot width feeds nothing there — skip + // that observer entirely. + const { measureRef, width: availableWidth } = useMeasuredWidth({ + enabled: !(useMobileOverflowPopover && mobileActionDisplay === "overflow"), + }); + const sharedColumnWidth = useContext(MessageColumnWidthContext); + const { measureRef: measureColumnRef, width: ownColumnWidth } = + useMeasuredWidth({ + enabled: sharedColumnWidth === null, + resolveTarget: resolveMessageColumn, + }); + // The shared value is the unpadded list root's width; the own observer + // reports the column's content box, which the assistant column's padding + // narrows — subtract it so both paths gate the expansion identically. + const columnWidth = + sharedColumnWidth === null + ? ownColumnWidth + : sharedColumnWidth.width === undefined + ? undefined + : sharedColumnWidth.width - + (alignment === "start" ? PROSE_COLUMN_INSET_PX : 0); // Touch-only: the hidden actions revealed in place by the "⋯" trigger. const [expanded, setExpanded] = useState(false); const expandedRowRef = useRef(null); @@ -601,7 +663,6 @@ export function MessageActionBar({ onSelect: action.onSelect, })), ]; - const useMobileOverflowPopover = isCompactViewport && isPointerCoarse; if (actions.length === 0) { return null; diff --git a/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx b/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx index c832b02ff7..69f3753c86 100644 --- a/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx +++ b/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx @@ -410,4 +410,24 @@ describe("SelectableMessageProse", () => { ), ); }); + + it("registers the shared pointer listeners as passive", () => { + const addSpy = vi.spyOn(document, "addEventListener"); + const view = render( + Answer prose, + ); + + // None of the pointer handlers call preventDefault; the passive flag is a + // perf contract (it keeps taps off the blocking-handler list), so pin it. + const optionsByType = new Map( + addSpy.mock.calls.map(([type, , options]) => [type, options]), + ); + for (const type of ["pointerdown", "pointerup", "pointercancel"]) { + expect(optionsByType.get(type), type).toEqual({ passive: true }); + } + + // Detach still matches (removeEventListener ignores `passive`): the + // shared-listener teardown test above covers the counts. + view.unmount(); + }); }); diff --git a/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx b/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx index 95e197a5c1..b987daf572 100644 --- a/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx +++ b/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx @@ -406,9 +406,17 @@ function handleSharedKeyUp(): void { } function attachSharedDocumentListeners(): void { - document.addEventListener("pointerdown", handleSharedPointerDown); - document.addEventListener("pointerup", handleSharedPointerRelease); - document.addEventListener("pointercancel", handleSharedPointerCancel); + // Passive: none of the pointer handlers call preventDefault, so declare it + // and keep every tap off the compositor's blocking-handler list. + document.addEventListener("pointerdown", handleSharedPointerDown, { + passive: true, + }); + document.addEventListener("pointerup", handleSharedPointerRelease, { + passive: true, + }); + document.addEventListener("pointercancel", handleSharedPointerCancel, { + passive: true, + }); document.addEventListener("mouseup", handleSharedPointerRelease); document.addEventListener("selectionchange", handleSharedSelectionChange); document.addEventListener("keyup", handleSharedKeyUp); diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx index 092b1798f3..ac9cc17fdf 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx @@ -1522,3 +1522,175 @@ describe("ThreadTimelineRows actions", () => { ); }); }); + +describe("ThreadTimelineRows shared message column width", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("expands overflow actions in place from the row list's one column measurement", () => { + mockSelectionMenuMedia({ isCompactViewport: true, isPointerCoarse: true }); + const observations: { callback: ResizeObserverCallback; node: Element }[] = + []; + class ControlledResizeObserver { + readonly #callback: ResizeObserverCallback; + constructor(callback: ResizeObserverCallback) { + this.#callback = callback; + } + observe(node: Element) { + observations.push({ callback: this.#callback, node }); + } + unobserve() {} + disconnect() {} + } + vi.stubGlobal("ResizeObserver", ControlledResizeObserver); + + const { container } = renderWithRouter( + , + ); + + // Report a width only for the top-level row list: the bars' own columns + // are never observed here, so an in-place expansion can only come from + // the shared list-level measurement flowing down through context. + act(() => { + for (const { callback, node } of observations) { + if (!node.hasAttribute("data-timeline-row-list")) continue; + callback( + [ + { + target: node, + contentRect: { width: 358, height: 600 }, + } as unknown as ResizeObserverEntry, + ], + undefined as unknown as ResizeObserver, + ); + } + }); + + const earlierMessage = container.querySelector( + '[data-timeline-row-id="earlier_agent_message"]', + ); + const trigger = earlierMessage?.querySelector( + '[aria-label="Message actions"]', + ); + if (!trigger) throw new Error("Missing overflow trigger"); + fireEvent.click(trigger); + + // In-place expansion, not the popover: the 358px column fits all three + // 28px touch actions with the comfort margin to spare. + expect(document.body.querySelector('[data-side="top"]')).toBeNull(); + expect( + earlierMessage?.querySelector('[aria-label="Copy message"]'), + ).not.toBeNull(); + expect( + earlierMessage?.querySelector('[aria-label="Fork into new thread"]'), + ).not.toBeNull(); + }); + + it("subtracts the assistant column's padding from the shared list width", () => { + mockSelectionMenuMedia({ isCompactViewport: true, isPointerCoarse: true }); + const observations: { callback: ResizeObserverCallback; node: Element }[] = + []; + class ControlledResizeObserver { + readonly #callback: ResizeObserverCallback; + constructor(callback: ResizeObserverCallback) { + this.#callback = callback; + } + observe(node: Element) { + observations.push({ callback: this.#callback, node }); + } + unobserve() {} + disconnect() {} + } + vi.stubGlobal("ResizeObserver", ControlledResizeObserver); + + const { container } = renderWithRouter( + , + ); + const reportListWidth = (width: number) => { + act(() => { + for (const { callback, node } of observations) { + if (!node.hasAttribute("data-timeline-row-list")) continue; + callback( + [ + { + target: node, + contentRect: { width, height: 600 }, + } as unknown as ResizeObserverEntry, + ], + undefined as unknown as ResizeObserver, + ); + } + }); + }; + const earlierMessage = container.querySelector( + '[data-timeline-row-id="earlier_agent_message"]', + ); + if (!earlierMessage) throw new Error("Missing earlier assistant row"); + const clickTrigger = () => { + const trigger = earlierMessage.querySelector( + '[aria-label="Message actions"]', + ); + if (!trigger) throw new Error("Missing overflow trigger"); + fireEvent.click(trigger); + }; + + // The assistant `[data-message-column]` is `px-2`, so a 131px list leaves + // it a 115px content box: one short of the 116px the three 28px touch + // actions need with their comfort margin. The popover must win, exactly + // as it did when each bar observed its own column. + reportListWidth(131); + clickTrigger(); + expect(document.body.querySelector('[data-side="top"]')).not.toBeNull(); + expect( + earlierMessage.querySelector('[aria-label="Copy message"]'), + ).toBeNull(); + + // One more pixel of list width clears the threshold: in place, no popover. + reportListWidth(132); + clickTrigger(); + expect(document.body.querySelector('[data-side="top"]')).toBeNull(); + expect( + earlierMessage.querySelector('[aria-label="Copy message"]'), + ).not.toBeNull(); + expect( + earlierMessage.querySelector('[aria-label="Fork into new thread"]'), + ).not.toBeNull(); + }); +}); diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx index c0d1ae1fc4..f6fbb5b7a2 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx @@ -66,6 +66,10 @@ import type { UserAttachmentImageSrcResolver, } from "./types.js"; import { ConversationMessageContent } from "./ConversationMessageContent.js"; +import { + MessageColumnWidthContext, + useMeasuredWidth, +} from "./MessageActionBar.js"; import { TimelineSelectionMenu } from "./TimelineSelectionMenu.js"; import type { MessageProseSelection } from "./SelectableMessageProse.js"; import { ExpandableTimelineRow } from "./ExpandableTimelineRow.js"; @@ -2144,81 +2148,99 @@ function TimelineRowsList({ detailScrollRoot?.getScrollElement ?? bottomAnchor?.getScrollElement ?? null; + const isTopLevelList = spacing === "top-level"; + // One observer for every action bar below: each top-level row's message + // column is as wide as this list, so the bars derive their column's content + // width from this shared measurement (MessageColumnWidthContext; the bar + // subtracts its own column's padding) instead of observing their own + // columns. Nested lists are narrower, so they shadow the value with null + // and their bars fall back to per-bar measurement. + const { measureRef: messageColumnWidthSourceRef, width: messageColumnWidth } = + useMeasuredWidth({ enabled: isTopLevelList }); + const messageColumnWidthValue = useMemo( + () => ({ width: messageColumnWidth }), + [messageColumnWidth], + ); return ( -
- { - const item = items[index]; - return item?.kind === "row" - ? estimateTimelineWindowedRowHeight(item.row, spacing) - : 28; - }} - gap={spacing === "bundle" ? 0 : 8} - getScrollElement={getWindowingScrollElement} - itemKeys={itemKeys} - measurements={measurements} - minItemCount={ - spacing === "top-level" ? (isCompactViewport ? 40 : 60) : 20 - } - renderItem={(index, windowedState) => { - const item = items[index]; - if (item === undefined) { - return null; +
+ { + const item = items[index]; + return item?.kind === "row" + ? estimateTimelineWindowedRowHeight(item.row, spacing) + : 28; + }} + gap={spacing === "bundle" ? 0 : 8} + getScrollElement={getWindowingScrollElement} + itemKeys={itemKeys} + measurements={measurements} + minItemCount={ + spacing === "top-level" ? (isCompactViewport ? 40 : 60) : 20 } - if (item.kind === "unread-divider") { + renderItem={(index, windowedState) => { + const item = items[index]; + if (item === undefined) { + return null; + } + if (item.kind === "unread-divider") { + return ( +
+ {windowedState.isRealized ? ( + + ) : null} +
+ ); + } return ( -
{windowedState.isRealized ? ( - ) : null} -
+ ); - } - return ( - - {windowedState.isRealized ? ( - - ) : null} - - ); - }} - /> -
+ }} + /> +
+ ); } diff --git a/apps/app/src/components/thread/timeline/TimelineTitleView.tsx b/apps/app/src/components/thread/timeline/TimelineTitleView.tsx index d6eaf56605..fbf93ab58f 100644 --- a/apps/app/src/components/thread/timeline/TimelineTitleView.tsx +++ b/apps/app/src/components/thread/timeline/TimelineTitleView.tsx @@ -1,4 +1,4 @@ -import { Fragment, useEffect, useState } from "react"; +import { Fragment } from "react"; import type { KeyboardEvent, MouseEvent, ReactNode } from "react"; import { assertNever, @@ -15,6 +15,7 @@ import { import { cn } from "@bb/shared-ui/lib/utils"; import { DiffStatsTally } from "@/components/ui/diff-stats-tally.js"; import { RouteAnchor } from "@/components/ui/app-route-anchor.js"; +import { useSecondTick } from "@/hooks/useSecondTick"; /** * Resolves a title's declared action to a click callback. Return `null` to @@ -209,26 +210,20 @@ function renderSegment( } /** - * Ticks the displayed elapsed time locally while the row is still active. - * The truth is `startedAt` (the wall-clock when the work began); the App - * derives `now - startedAt` and ticks once per second until the row reaches - * a terminal status (at which point a static `completedAt - startedAt` is - * shown by the caller instead). Stays empty until the elapsed time crosses - * the visible threshold (>1s) to avoid sub-second flicker on row entry. + * Ticks the displayed elapsed time while the row is still active. The truth + * is `startedAt` (the wall-clock when the work began); the App derives + * `now - startedAt` from the shared 1 Hz ticker — one interval for every + * in-flight row on screen, paused while the document is hidden — until the + * row reaches a terminal status (at which point a static + * `completedAt - startedAt` is shown by the caller instead). Stays empty + * until the elapsed time crosses the visible threshold (>1s) to avoid + * sub-second flicker on row entry. */ function LiveDurationText({ startedAt }: { startedAt: number }) { - const [tick, setTick] = useState(() => Date.now() - startedAt); + const elapsedMs = useSecondTick() - startedAt; - useEffect(() => { - setTick(Date.now() - startedAt); - const interval = window.setInterval(() => { - setTick(Date.now() - startedAt); - }, 1_000); - return () => window.clearInterval(interval); - }, [startedAt]); - - if (tick <= 1_000) return null; - return <>{durationToCompactString(tick)}; + if (elapsedMs <= 1_000) return null; + return <>{durationToCompactString(elapsedMs)}; } function renderDecoration( diff --git a/apps/app/src/components/ui/app-route-anchor.test.tsx b/apps/app/src/components/ui/app-route-anchor.test.tsx new file mode 100644 index 0000000000..f346393344 --- /dev/null +++ b/apps/app/src/components/ui/app-route-anchor.test.tsx @@ -0,0 +1,65 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { MemoryRouter, useLocation } from "react-router-dom"; +import { afterEach, describe, expect, it } from "vitest"; +import { + RouteAnchor, + RouteNavigationProvider, + useIsRouteNavigationPending, +} from "./app-route-anchor"; + +afterEach(() => { + cleanup(); +}); + +interface NavigationSample { + isPending: boolean; + pathname: string; +} + +const samples: NavigationSample[] = []; + +/** + * Records every committed (isPending, pathname) pair. The pairs prove commit + * ordering: a `{ isPending: true, pathname: }` sample means the pending + * flag painted while the previous route was still on screen, i.e. the tap's + * event did not flush the destination route synchronously. + */ +function NavigationSampler() { + const isPending = useIsRouteNavigationPending(); + const { pathname } = useLocation(); + samples.push({ isPending, pathname }); + return null; +} + +describe("RouteAnchor transition navigation", () => { + it("swaps the route in a later commit than the tap and signals pending in between", () => { + samples.length = 0; + render( + + + + open thr-new + + , + ); + expect(samples).toEqual([ + { isPending: false, pathname: "/threads/thr-old" }, + ]); + + fireEvent.click(screen.getByRole("link", { name: "open thr-new" })); + + // The tap's urgent commit shows the pending affordance with the old route + // still mounted; the destination route lands in a follow-up transition + // commit, which also clears the pending flag. + expect(samples).toContainEqual({ + isPending: true, + pathname: "/threads/thr-old", + }); + expect(samples.at(-1)).toEqual({ + isPending: false, + pathname: "/threads/thr-new", + }); + }); +}); diff --git a/apps/app/src/components/ui/app-route-anchor.tsx b/apps/app/src/components/ui/app-route-anchor.tsx index 341a02db2f..72658a943d 100644 --- a/apps/app/src/components/ui/app-route-anchor.tsx +++ b/apps/app/src/components/ui/app-route-anchor.tsx @@ -6,6 +6,7 @@ import { useLayoutEffect, useMemo, useRef, + useTransition, type ComponentPropsWithoutRef, type MouseEvent as ReactMouseEvent, type ReactNode, @@ -36,6 +37,23 @@ type RouteNavigate = (path: string, options?: RouteNavigateOptions) => void; const RouteNavigationContext = createContext(null); +// Separate from RouteNavigationContext on purpose: the pending bit flips on +// every navigation, and folding it into the navigate context would re-render +// every navigate consumer (sidebar rows, thread actions) per navigation — +// the exact churn RouteNavigationContext exists to avoid. +const RouteNavigationPendingContext = createContext(false); + +/** + * True while a navigation started through {@link useRouteNavigate} or + * {@link RouteAnchor} is still rendering the destination route. Navigation + * runs at transition priority, so the previous route stays on screen for a + * beat; surfaces read this to show a lightweight pending affordance (e.g. + * keeping the tapped row's active state) instead of appearing unresponsive. + */ +export function useIsRouteNavigationPending(): boolean { + return useContext(RouteNavigationPendingContext); +} + /** * A `navigate` whose identity never changes and whose caller does not * subscribe to the router's location. @@ -94,13 +112,24 @@ export function RouteNavigationProvider({ useLayoutEffect(() => { navigateRef.current = navigate; }, [navigate]); - const navigateRoute = useCallback((path, options) => { - if (options === undefined) { - navigateRef.current(path); - return; - } - navigateRef.current(path, options); - }, []); + // Navigate at transition priority: a tap's urgent commit (active states, + // isNavigationPending) paints first, and the destination route renders in an + // interruptible follow-up commit instead of blocking the tap's frame. + // `startNavigationTransition` has a stable identity, so `navigateRoute` + // keeps the never-changing identity its consumers depend on. + const [isNavigationPending, startNavigationTransition] = useTransition(); + const navigateRoute = useCallback( + (path, options) => { + startNavigationTransition(() => { + if (options === undefined) { + navigateRef.current(path); + return; + } + navigateRef.current(path, options); + }); + }, + [startNavigationTransition], + ); useEffect(() => { const browserApi = getDesktopBrowserApi(); if (browserApi === null) { @@ -116,7 +145,9 @@ export function RouteNavigationProvider({ return ( - {children} + + {children} + ); } diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx new file mode 100644 index 0000000000..acc7d6fe23 --- /dev/null +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx @@ -0,0 +1,276 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render } from "@testing-library/react"; +import { getDefaultStore } from "jotai"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { BottomAnchoredScrollBody } from "@/components/ui/bottom-anchored-scroll-body"; +import { threadTimelineScrollAnchorAtomFamily } from "@/lib/thread-timeline-scroll-anchor"; + +// Per-scroll-event costs on coarse pointers: the transient-scrollbar attribute +// is written once per scroll burst (the at-rest `.thread-scrollbar` rules hide +// the thumb on every platform, so a touch flick still has to flip it), the +// scroll-anchor capture throttle relaxes to the coarse cadence, and captures +// reuse a cached row NodeList that the ResizeObserver invalidates. + +interface ScrollMetrics { + scrollHeight: number; + clientHeight: number; + scrollTop: number; +} + +interface RowRect { + top: number; + bottom: number; +} + +const SCROLL_AREA_CLASS = "scroll-area"; + +class ResizeObserverMock implements ResizeObserver { + static instances: ResizeObserverMock[] = []; + readonly callback: ResizeObserverCallback; + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + ResizeObserverMock.instances.push(this); + } + observe() {} + unobserve() {} + disconnect() {} + trigger() { + this.callback([], this); + } +} + +function getLatestResizeObserver(): ResizeObserverMock { + const instance = ResizeObserverMock.instances.at(-1); + if (!instance) throw new Error("Expected a ResizeObserver instance."); + return instance; +} + +function stubMediaQueries(matching: ReadonlySet): void { + vi.spyOn(window, "matchMedia").mockImplementation((query: string) => ({ + matches: matching.has(query), + media: query, + onchange: null, + addListener: () => {}, + removeListener: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, + })); +} + +function setScrollMetrics(element: HTMLElement, metrics: ScrollMetrics) { + Object.defineProperty(element, "scrollHeight", { + configurable: true, + value: metrics.scrollHeight, + }); + Object.defineProperty(element, "clientHeight", { + configurable: true, + value: metrics.clientHeight, + }); + element.scrollTop = metrics.scrollTop; +} + +function mockScrollAreaRect(scrollArea: HTMLElement) { + vi.spyOn(scrollArea, "getBoundingClientRect").mockReturnValue( + new DOMRect(0, 0, 100, 100), + ); +} + +function mockRowRect(row: HTMLElement, rect: RowRect) { + vi.spyOn(row, "getBoundingClientRect").mockReturnValue( + new DOMRect(0, rect.top, 100, rect.bottom - rect.top), + ); +} + +function requireHTMLElement(element: Element | null) { + if (!(element instanceof HTMLElement)) { + throw new Error("Expected HTMLElement."); + } + return element; +} + +function renderTimeline(threadId: string, rowIds: string[]) { + const view = render( + Footer
} + maxWidthClassName="max-w-none" + scrollAreaClassName={SCROLL_AREA_CLASS} + scrollAnchorThreadId={threadId} + > + {rowIds.map((rowId) => ( +
+ {rowId} +
+ ))} + , + ); + const scrollArea = requireHTMLElement( + view.container.querySelector(`.${SCROLL_AREA_CLASS}`), + ); + const rowElements = new Map(); + for (const rowId of rowIds) { + rowElements.set( + rowId, + requireHTMLElement( + view.container.querySelector(`[data-timeline-row-id="${rowId}"]`), + ), + ); + } + return { scrollArea, rowElements }; +} + +function readAnchor(threadId: string) { + return getDefaultStore().get(threadTimelineScrollAnchorAtomFamily(threadId)); +} + +beforeEach(() => { + ResizeObserverMock.instances = []; + vi.stubGlobal("ResizeObserver", ResizeObserverMock); + vi.stubGlobal( + "requestAnimationFrame", + vi.fn(() => 1), + ); + vi.stubGlobal("cancelAnimationFrame", vi.fn()); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); + const store = getDefaultStore(); + for (const threadId of ["coarse-thread", "cache-thread"]) { + store.set(threadTimelineScrollAnchorAtomFamily(threadId), null); + } + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("BottomAnchoredScrollBody on coarse pointers", () => { + it("shows the transient scrollbar with one attribute write per scroll burst", () => { + stubMediaQueries(new Set(["(pointer: coarse)"])); + vi.useFakeTimers(); + const { scrollArea } = renderTimeline("coarse-thread", ["row-a"]); + const attributeWrites = new MutationObserver(() => {}); + attributeWrites.observe(scrollArea, { + attributes: true, + attributeFilter: ["data-scrollbar-scrolling"], + }); + + // `.thread-scrollbar` paints the thumb transparent at rest on every + // platform, and Android Chrome / iOS apply `scrollbar-color` to their + // overlay indicators, so a touch flick still has to flip the attribute — + // once per burst: the steady-state events must not re-set it, since each + // attribute write is a style invalidation while the browser is scrolling. + for (let scrollTop = 10; scrollTop <= 50; scrollTop += 10) { + scrollArea.scrollTop = scrollTop; + fireEvent.scroll(scrollArea); + } + expect(scrollArea.getAttribute("data-scrollbar-scrolling")).toBe("true"); + expect(attributeWrites.takeRecords()).toHaveLength(1); + + // The thumb hides again once scrolling goes idle. + vi.runAllTimers(); + expect(scrollArea.hasAttribute("data-scrollbar-scrolling")).toBe(false); + attributeWrites.disconnect(); + }); + + it("captures scroll anchors at the relaxed coarse cadence", () => { + stubMediaQueries(new Set(["(pointer: coarse)"])); + // Fake performance.now so the throttle windows below are deterministic. + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance"] }); + const { scrollArea } = renderTimeline("coarse-thread", ["row-a"]); + + // Move the clock past the throttle window so the first capture writes + // immediately, leaving the throttle's lastWriteAt equal to now. (Running + // pending timers instead would also fire the scrollbar idle timeout and + // advance the clock 600ms past that write.) + vi.advanceTimersByTime(1_000); + fireEvent.scroll(scrollArea); + expect(readAnchor("coarse-thread")).not.toBeNull(); + getDefaultStore().set( + threadTimelineScrollAnchorAtomFamily("coarse-thread"), + null, + ); + + // A capture 50ms into the window arms a trailing write for the remainder + // of the coarse throttle: 250 - 50 = 200ms out. + vi.advanceTimersByTime(50); + fireEvent.scroll(scrollArea); + expect(readAnchor("coarse-thread")).toBeNull(); + + // The fine-pointer cadence (100ms window → 50ms remainder) must not fire + // on a coarse pointer... + vi.advanceTimersByTime(199); + expect(readAnchor("coarse-thread")).toBeNull(); + + // ...but the trailing write still records the resting position at 250ms. + vi.advanceTimersByTime(1); + expect(readAnchor("coarse-thread")).toEqual({ + rowId: "", + offsetWithinRow: 0, + atBottom: true, + }); + }); +}); + +describe("BottomAnchoredScrollBody row NodeList cache", () => { + it("reuses the cached rows across captures until a resize invalidates them", () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance"] }); + const { scrollArea, rowElements } = renderTimeline("cache-thread", [ + "row-a", + "row-b", + "row-c", + ]); + mockScrollAreaRect(scrollArea); + mockRowRect(requireHTMLElement(rowElements.get("row-a")!), { + top: -120, + bottom: -20, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-b")!), { + top: -20, + bottom: 80, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-c")!), { + top: 80, + bottom: 180, + }); + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + const queryRows = vi.spyOn(scrollArea, "querySelectorAll"); + + // Move the clock past the throttle window so every capture below writes + // immediately instead of arming a trailing timeout. + vi.advanceTimersByTime(1_000); + scrollArea.scrollTop = 150; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readAnchor("cache-thread")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + expect(queryRows).toHaveBeenCalledTimes(1); + + // A second capture in the same layout reuses the cached NodeList. + vi.advanceTimersByTime(200); + scrollArea.scrollTop = 140; + fireEvent.scroll(scrollArea); + expect(queryRows).toHaveBeenCalledTimes(1); + + // A ResizeObserver delivery means rows may have mounted or unmounted; + // the next capture queries fresh. + getLatestResizeObserver().trigger(); + vi.advanceTimersByTime(200); + scrollArea.scrollTop = 130; + fireEvent.scroll(scrollArea); + expect(queryRows).toHaveBeenCalledTimes(2); + expect(readAnchor("cache-thread")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + }); +}); diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx index d7962a1165..2c719442c5 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx @@ -11,8 +11,10 @@ import { threadTimelineScrollAnchorAtomFamily } from "@/lib/thread-timeline-scro // Real externals only: the ResizeObserver/rAF used by the scroll body are // browser primitives jsdom omits, so they are stubbed; nothing in our own code -// is mocked. The atom is read back from the real default jotai store the -// component writes to. +// is mocked. The ResizeObserver stub delivers what a browser delivers — an +// entry per observed target carrying its box sizes — so the resize path under +// test is the one production runs. The atom is read back from the real default +// jotai store the component writes to. interface ScrollMetrics { scrollHeight: number; @@ -32,18 +34,48 @@ const SCROLL_AREA_HEIGHT = 100; class ResizeObserverMock implements ResizeObserver { static instances: ResizeObserverMock[] = []; readonly callback: ResizeObserverCallback; + readonly targets: Element[] = []; constructor(callback: ResizeObserverCallback) { this.callback = callback; ResizeObserverMock.instances.push(this); } - observe() {} + observe(target: Element) { + this.targets.push(target); + } unobserve() {} disconnect() {} trigger() { - this.callback([], this); + this.callback(this.targets.map(makeResizeEntry), this); } } +// The scroll port's content box is its client height and the content wrapper's +// border box is the port's scroll height: the pair the component derives its +// cached max offset from. The other box of each entry differs by 8px so a +// refresh that reads the wrong one shows up as a skewed max offset. +function makeResizeEntry(target: Element): ResizeObserverEntry { + const isScrollPort = target.classList.contains(SCROLL_AREA_CLASS); + const scrollPort = isScrollPort ? target : target.parentElement; + if (!scrollPort) { + throw new Error("Expected the content wrapper inside the scroll port."); + } + const contentBlockSize = isScrollPort + ? scrollPort.clientHeight + : scrollPort.scrollHeight - 8; + const borderBlockSize = isScrollPort + ? scrollPort.clientHeight + 8 + : scrollPort.scrollHeight; + return { + target, + contentRect: new DOMRect(0, 0, 100, contentBlockSize), + borderBoxSize: [{ blockSize: borderBlockSize, inlineSize: 100 }], + contentBoxSize: [{ blockSize: contentBlockSize, inlineSize: 100 }], + devicePixelContentBoxSize: [ + { blockSize: contentBlockSize, inlineSize: 100 }, + ], + }; +} + function getLatestResizeObserver(): ResizeObserverMock { const instance = ResizeObserverMock.instances.at(-1); if (!instance) throw new Error("Expected a ResizeObserver instance."); @@ -80,8 +112,12 @@ function mockScrollAreaRect(scrollArea: HTMLElement) { } function mockRowRect(row: HTMLElement, rect: RowRect) { - vi.spyOn(row, "getBoundingClientRect").mockReturnValue( - new DOMRect(0, rect.top, 100, rect.bottom - rect.top), + // A row the timeline has since unmounted reports an empty rect, as a + // browser's disconnected element does. + vi.spyOn(row, "getBoundingClientRect").mockImplementation(() => + row.isConnected + ? new DOMRect(0, rect.top, 100, rect.bottom - rect.top) + : new DOMRect(0, 0, 0, 0), ); } @@ -171,6 +207,11 @@ function renderTimeline({ getByRole: view.getByRole, scrollArea, rowElements, + // Rows mounted by a later rerender are not in `rowElements`. + getRow: (rowId: string) => + requireHTMLElement( + view.container.querySelector(`[data-timeline-row-id="${rowId}"]`), + ), rerenderRows: (nextRowIds: string[]) => view.rerender(timeline(nextRowIds)), unmount: view.unmount, }; @@ -297,6 +338,65 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { }); }); + it("follows the row window when a windowed timeline slides it without a resize", () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance"] }); + const { scrollArea, getRow, rerenderRows, unmount } = renderTimeline({ + threadId: "thread-a", + // row-z is the timeline's last row, which the windowed list keeps + // mounted whatever the window holds. + rowIds: ["row-a", "row-b", "row-c", "row-z"], + virtualized: true, + }); + mockScrollAreaRect(scrollArea); + mockRowRect(getRow("row-a"), { top: -120, bottom: -20 }); + mockRowRect(getRow("row-b"), { top: -20, bottom: 80 }); + mockRowRect(getRow("row-c"), { top: 80, bottom: 180 }); + mockRowRect(getRow("row-z"), { top: 5_000, bottom: 5_100 }); + setScrollMetrics(scrollArea, { + scrollHeight: 6_000, + clientHeight: 100, + scrollTop: 5_900, + }); + getLatestResizeObserver().trigger(); + + // Past the throttle window, so each capture below writes immediately. + vi.advanceTimersByTime(1_000); + scrollArea.scrollTop = 1_000; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + + // A flick shorter than the overscan slides the window inside the + // fixed-height spacer: row-b and row-c unmount, row-x and row-y mount + // above row-a, and row-a and row-z stay connected. No observed box + // changed size, so the ResizeObserver never fires. + rerenderRows(["row-x", "row-y", "row-a", "row-z"]); + mockRowRect(getRow("row-x"), { top: -130, bottom: -30 }); + mockRowRect(getRow("row-y"), { top: -30, bottom: 70 }); + mockRowRect(getRow("row-a"), { top: 70, bottom: 170 }); + vi.advanceTimersByTime(1_000); + scrollArea.scrollTop = 900; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-y", + offsetWithinRow: 30, + atBottom: false, + }); + + // Leaving the thread flushes a final capture from the same row set. + unmount(); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-y", + offsetWithinRow: 30, + atBottom: false, + }); + }); + it("finds the visible anchor with logarithmic row measurements", () => { const rowIds = Array.from({ length: 128 }, (_, index) => `row-${index}`); const { scrollArea, rowElements } = renderTimeline({ diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.settle-tail.test.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.settle-tail.test.tsx new file mode 100644 index 0000000000..bbd03f0c8b --- /dev/null +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.settle-tail.test.tsx @@ -0,0 +1,340 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render } from "@testing-library/react"; +import { getDefaultStore } from "jotai"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { BottomAnchoredScrollBody } from "@/components/ui/bottom-anchored-scroll-body"; +import { threadTimelineScrollAnchorAtomFamily } from "@/lib/thread-timeline-scroll-anchor"; + +// Companion to the scroll-preservation suite, focused on the geometry-read +// budget of the resize path: the observed frame may read live +// scrollHeight/clientHeight, but the rAF settle tail must run on the cached +// max offset (at most one live verification read when a cached restore found +// drift), and deliveries that carry ResizeObserver box sizes must refresh the +// cache from them without forcing layout at all. + +interface ScrollMetrics { + scrollHeight: number; + clientHeight: number; + scrollTop: number; +} + +const SCROLL_AREA_CLASS = "scroll-area"; +const THREAD_ID = "settle-thread"; + +class ResizeObserverMock implements ResizeObserver { + static instances: ResizeObserverMock[] = []; + readonly callback: ResizeObserverCallback; + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + ResizeObserverMock.instances.push(this); + } + observe() {} + unobserve() {} + disconnect() {} + trigger(entries: ResizeObserverEntry[] = []) { + this.callback(entries, this); + } +} + +function getLatestResizeObserver(): ResizeObserverMock { + const instance = ResizeObserverMock.instances.at(-1); + if (!instance) throw new Error("Expected a ResizeObserver instance."); + return instance; +} + +interface ManualAnimationFrames { + runFrame: () => void; + hasPending: () => boolean; +} + +// Unlike the scroll-preservation suite (which discards rAF callbacks because +// the settle tail is irrelevant there), these tests drive the tail frame by +// frame to observe what each one reads. +function installManualAnimationFrames(): ManualAnimationFrames { + let nextHandle = 1; + const pending = new Map(); + vi.stubGlobal( + "requestAnimationFrame", + vi.fn((callback: FrameRequestCallback) => { + const handle = nextHandle; + nextHandle += 1; + pending.set(handle, callback); + return handle; + }), + ); + vi.stubGlobal( + "cancelAnimationFrame", + vi.fn((handle: number) => { + pending.delete(handle); + }), + ); + return { + runFrame() { + const callbacks = [...pending.values()]; + pending.clear(); + for (const callback of callbacks) { + callback(window.performance.now()); + } + }, + hasPending() { + return pending.size > 0; + }, + }; +} + +function setScrollMetrics(element: HTMLElement, metrics: ScrollMetrics) { + Object.defineProperty(element, "scrollHeight", { + configurable: true, + value: metrics.scrollHeight, + }); + Object.defineProperty(element, "clientHeight", { + configurable: true, + value: metrics.clientHeight, + }); + element.scrollTop = metrics.scrollTop; +} + +interface GeometryReadCounters { + readScrollHeight: ReturnType; + readClientHeight: ReturnType; +} + +function installGeometryReadCounters( + element: HTMLElement, + metrics: Pick, +): GeometryReadCounters { + const readScrollHeight = vi.fn(() => metrics.scrollHeight); + const readClientHeight = vi.fn(() => metrics.clientHeight); + Object.defineProperty(element, "scrollHeight", { + configurable: true, + get: readScrollHeight, + }); + Object.defineProperty(element, "clientHeight", { + configurable: true, + get: readClientHeight, + }); + return { readScrollHeight, readClientHeight }; +} + +interface ResizeEntryBoxes { + contentBlockSize: number; + borderBlockSize: number; +} + +// The two boxes differ so a test can tell which one the component reads: the +// scroll port's content box is its client height (padding excluded) and the +// content wrapper's border box is the scroll height. +function makeResizeEntry( + target: Element, + boxes: ResizeEntryBoxes, +): ResizeObserverEntry { + const contentBoxSize: ResizeObserverSize = { + blockSize: boxes.contentBlockSize, + inlineSize: 100, + }; + const borderBoxSize: ResizeObserverSize = { + blockSize: boxes.borderBlockSize, + inlineSize: 100, + }; + return { + target, + contentRect: new DOMRect(0, 0, 100, boxes.contentBlockSize), + borderBoxSize: [borderBoxSize], + contentBoxSize: [contentBoxSize], + devicePixelContentBoxSize: [contentBoxSize], + }; +} + +function requireHTMLElement(element: Element | null) { + if (!(element instanceof HTMLElement)) { + throw new Error("Expected HTMLElement."); + } + return element; +} + +function renderScrollBody() { + const view = render( + Footer} + maxWidthClassName="max-w-none" + scrollAreaClassName={SCROLL_AREA_CLASS} + scrollAnchorThreadId={THREAD_ID} + > +
row-a
+ , + ); + const scrollArea = requireHTMLElement( + view.container.querySelector(`.${SCROLL_AREA_CLASS}`), + ); + const scrollContent = requireHTMLElement(scrollArea.firstElementChild); + return { scrollArea, scrollContent }; +} + +let frames: ManualAnimationFrames; + +beforeEach(() => { + ResizeObserverMock.instances = []; + vi.stubGlobal("ResizeObserver", ResizeObserverMock); + frames = installManualAnimationFrames(); +}); + +afterEach(() => { + cleanup(); + getDefaultStore().set(threadTimelineScrollAnchorAtomFamily(THREAD_ID), null); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +// Pin the viewport to the bottom of settled content and drain the mount tail, +// then grow the content so the observed frame restores to the new bottom and +// arms a fresh settle tail. +function growContentWhilePinned() { + const rendered = renderScrollBody(); + const { scrollArea } = rendered; + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + frames.runFrame(); + + setScrollMetrics(scrollArea, { + scrollHeight: 500, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(400); + return rendered; +} + +describe("BottomAnchoredScrollBody settle tail", () => { + it("settles without re-reading geometry when the cached restore finds no drift", () => { + const { scrollArea } = growContentWhilePinned(); + const { readScrollHeight, readClientHeight } = installGeometryReadCounters( + scrollArea, + { scrollHeight: 500, clientHeight: 100 }, + ); + + // No drift after the observed frame: the tail's first cached comparison + // sees the pinned position and stops without a single forced layout. + frames.runFrame(); + expect(scrollArea.scrollTop).toBe(400); + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + expect(frames.hasPending()).toBe(false); + }); + + it("spends at most one live read when the settle tail corrects drift", () => { + const { scrollArea } = growContentWhilePinned(); + // Cascading layout (footer/prompt height settling) moved scrollTop after + // the observed frame without resizing the observed boxes. + scrollArea.scrollTop = 390; + const { readScrollHeight, readClientHeight } = installGeometryReadCounters( + scrollArea, + { scrollHeight: 500, clientHeight: 100 }, + ); + + // First tail frame corrects against the cache alone. + frames.runFrame(); + expect(scrollArea.scrollTop).toBe(400); + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + + // The cached correction arms exactly one live verification read. + frames.runFrame(); + expect(readScrollHeight).toHaveBeenCalledTimes(1); + expect(readClientHeight).toHaveBeenCalledTimes(1); + expect(scrollArea.scrollTop).toBe(400); + + // Verification found the bottom stable, so the tail is done. + frames.runFrame(); + expect(readScrollHeight).toHaveBeenCalledTimes(1); + expect(readClientHeight).toHaveBeenCalledTimes(1); + expect(frames.hasPending()).toBe(false); + }); + + it("derives the cached max offset from observed box sizes without forcing layout", () => { + const { scrollArea, scrollContent } = renderScrollBody(); + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + frames.runFrame(); + + // Detach mid-timeline (the detach edge spends its allowed verification + // read here, before the counters are installed). + scrollArea.scrollTop = 150; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + + const liveMetrics = { scrollHeight: 900, clientHeight: 100 }; + const { readScrollHeight, readClientHeight } = installGeometryReadCounters( + scrollArea, + liveMetrics, + ); + + // Content grows to 900 while detached. The delivery carries the observer's + // own box sizes, so the cache refresh needs no scrollHeight/clientHeight. + // The refresh must read the scroll port's content box (100) and the + // content wrapper's border box (900); the other box of each pair is off. + getLatestResizeObserver().trigger([ + makeResizeEntry(scrollArea, { + contentBlockSize: 100, + borderBlockSize: 108, + }), + makeResizeEntry(scrollContent, { + contentBlockSize: 880, + borderBlockSize: 900, + }), + ]); + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + + // The derived max offset (800) is what scroll classification runs on: + // 790 is outside the 4px threshold, so the viewport stays detached and + // the next growth (max offset 850) leaves it where it is... + scrollArea.scrollTop = 790; + fireEvent.scroll(scrollArea); + liveMetrics.scrollHeight = 950; + getLatestResizeObserver().trigger([ + makeResizeEntry(scrollArea, { + contentBlockSize: 100, + borderBlockSize: 108, + }), + makeResizeEntry(scrollContent, { + contentBlockSize: 930, + borderBlockSize: 950, + }), + ]); + expect(scrollArea.scrollTop).toBe(790); + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + + // ...while 847 is within it, so this scroll re-attaches — still without + // a live read. + scrollArea.scrollTop = 847; + fireEvent.scroll(scrollArea); + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + + // Re-attached: the next growth's observed frame follows the bottom (its + // restore legitimately reads fresh geometry). + liveMetrics.scrollHeight = 1_000; + getLatestResizeObserver().trigger([ + makeResizeEntry(scrollArea, { + contentBlockSize: 100, + borderBlockSize: 108, + }), + makeResizeEntry(scrollContent, { + contentBlockSize: 980, + borderBlockSize: 1_000, + }), + ]); + expect(scrollArea.scrollTop).toBe(900); + }); +}); diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx index 415dd47a04..65d562a3fd 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx @@ -11,6 +11,7 @@ import { import type { ReactNode } from "react"; import { useStore } from "jotai"; import { cn } from "@bb/shared-ui/lib/utils"; +import { usePointerCoarse } from "@bb/shared-ui/hooks/use-pointer-coarse"; import { PAGE_SHELL_CONTENT_STYLE } from "./page-shell-content-style.js"; import { supportsScrollAnchoring } from "@/lib/scroll-anchoring-support"; import { @@ -91,6 +92,10 @@ const BOTTOM_RESTORE_SETTLE_FRAME_COUNT = 3; // Throttle continuous scroll-anchor capture so a fast scroll writes the atom at // most this often, plus a trailing write for the final resting position. const SCROLL_ANCHOR_CAPTURE_THROTTLE_MS = 100; +// Coarse pointers only need restore-on-return fidelity — the resting position, +// always carried by the trailing write — not 10 Hz mid-flick samples, each of +// which costs rect reads while the browser is already busy scrolling. +const COARSE_SCROLL_ANCHOR_CAPTURE_THROTTLE_MS = 250; // While a saved anchor's row hasn't hydrated yet, the ResizeObserver re-applies // the restore as content settles. Give up (fall back to bottom) after this many // observed re-applies so a deleted/never-arriving row can't hang at the top. @@ -98,9 +103,11 @@ const SCROLL_ANCHOR_RESTORE_MAX_ATTEMPTS = 8; const TIMELINE_ROW_ID_SELECTOR = "[data-timeline-row-id]"; const TOP_LEVEL_TIMELINE_ROW_LIST_SELECTOR = '[data-timeline-row-list="top-level"]'; +const TIMELINE_VIRTUAL_SPACER_SELECTOR = + ":scope > [data-timeline-virtual-spacer]"; const DIRECT_TIMELINE_ROW_SELECTOR = [ `:scope > ${TIMELINE_ROW_ID_SELECTOR}`, - `:scope > [data-timeline-virtual-spacer] > ${TIMELINE_ROW_ID_SELECTOR}`, + `${TIMELINE_VIRTUAL_SPACER_SELECTOR} > ${TIMELINE_ROW_ID_SELECTOR}`, ].join(", "); const SCROLL_INTENT_KEYS = new Set([ "ArrowDown", @@ -169,18 +176,32 @@ interface TopMostVisibleRow { offsetWithinRow: number; } -function getScrollAnchorRows(scrollArea: HTMLElement): NodeListOf { +interface ScrollAnchorRowQuery { + rows: NodeListOf; + // A windowed timeline mounts only the rows near the viewport (plus a few + // pinned ones) inside a fixed-height spacer. + windowed: boolean; +} + +function getScrollAnchorRows(scrollArea: HTMLElement): ScrollAnchorRowQuery { const topLevelList = scrollArea.querySelector( TOP_LEVEL_TIMELINE_ROW_LIST_SELECTOR, ); if (topLevelList) { - return topLevelList.querySelectorAll( - DIRECT_TIMELINE_ROW_SELECTOR, - ); + return { + rows: topLevelList.querySelectorAll( + DIRECT_TIMELINE_ROW_SELECTOR, + ), + windowed: + topLevelList.querySelector(TIMELINE_VIRTUAL_SPACER_SELECTOR) !== null, + }; } // Embedded/test surfaces may render timeline rows without the app's // top-level list wrapper. - return scrollArea.querySelectorAll(TIMELINE_ROW_ID_SELECTOR); + return { + rows: scrollArea.querySelectorAll(TIMELINE_ROW_ID_SELECTOR), + windowed: false, + }; } // The top-most timeline row whose bottom edge is below the scroll area's top @@ -189,9 +210,9 @@ function getScrollAnchorRows(scrollArea: HTMLElement): NodeListOf { // reproduce a mid-row reading position. function getTopMostVisibleRow( scrollArea: HTMLElement, + rows: NodeListOf, ): TopMostVisibleRow | null { const scrollAreaTop = scrollArea.getBoundingClientRect().top; - const rows = getScrollAnchorRows(scrollArea); let low = 0; let high = rows.length - 1; let visibleRow: HTMLElement | null = null; @@ -277,6 +298,7 @@ export function BottomAnchoredScrollBody({ scrollAnchorThreadId, }: BottomAnchoredScrollBodyProps) { const store = useStore(); + const isPointerCoarse = usePointerCoarse(); const scrollAreaRef = useRef(null); const scrollContentRef = useRef(null); const shouldStickToBottomRef = useRef(true); @@ -285,6 +307,10 @@ export function BottomAnchoredScrollBody({ const pointerScrollIntentRef = useRef(false); const restoreFrameRef = useRef(null); const restoreFramesRemainingRef = useRef(0); + // Set when a settle-tail frame corrected drift against the cached max + // offset: the next tail frame then spends the tail's single live + // verification read (see queueBottomRestore). + const restoreTailLiveReadRef = useRef(false); const pendingPrependAnchorRef = useRef<{ scrollHeight: number; scrollTop: number; @@ -322,6 +348,20 @@ export function BottomAnchoredScrollBody({ // live geometry — the pre-cache behavior — instead of trusting a frozen // value that would classify every position as at-bottom. const resizeObserverHasDeliveredRef = useRef(false); + // The last box sizes the ResizeObserver reported for the scroll port and + // the content wrapper. Together they are the observer's own measurement of + // `scrollHeight - clientHeight`, letting the resize path refresh the cache + // without a forced layout (see handleScrollAreaResize). + const observedScrollGeometryRef = useRef<{ + scrollAreaClientHeight: number | null; + scrollContentHeight: number | null; + }>({ scrollAreaClientHeight: null, scrollContentHeight: null }); + // The row NodeList behind scroll-anchor capture on an unwindowed timeline, + // so throttled samples don't repeat a querySelectorAll over the whole + // thread. Row-set changes surface as content size changes there, so the + // ResizeObserver invalidates it. getScrollAnchorRowsCached never fills it + // for a windowed timeline, whose row swaps change no observed size. + const scrollAnchorRowsRef = useRef | null>(null); const [isAtBottom, setIsAtBottom] = useState(true); const initialScrollRestoreRowId = useMemo(() => { if (scrollAnchorThreadId === undefined) return null; @@ -360,6 +400,7 @@ export function BottomAnchoredScrollBody({ window.cancelAnimationFrame(restoreFrameRef.current); restoreFrameRef.current = null; restoreFramesRemainingRef.current = 0; + restoreTailLiveReadRef.current = false; }, []); // Snap scrollTop back to the bottom if anchoring has let us drift away. @@ -387,6 +428,22 @@ export function BottomAnchoredScrollBody({ return true; }, [refreshMaxScrollOffset]); + // The settle-tail variant: runs on the cached max offset so tail frames + // don't force layout. Any real size change re-enters through the + // ResizeObserver with fresh geometry; what the tail catches is scrollTop + // drifting (clamps, anchoring adjustments) while the observed sizes — and + // therefore the cache — still hold. + const restoreBottomFromCacheOnce = useCallback(() => { + const scrollArea = scrollAreaRef.current; + if (!scrollArea || !shouldStickToBottomRef.current) return false; + const maxScrollOffset = readMaxScrollOffset(scrollArea); + if (isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop)) { + return false; + } + scrollArea.scrollTop = maxScrollOffset; + return true; + }, [readMaxScrollOffset]); + const queueBottomRestore = useCallback(() => { if (!shouldStickToBottomRef.current) return; // Restore synchronously in the frame the size change was observed. @@ -402,13 +459,27 @@ export function BottomAnchoredScrollBody({ // isn't final in the observed frame. restoreBottomOnce(); restoreFramesRemainingRef.current = BOTTOM_RESTORE_SETTLE_FRAME_COUNT; + restoreTailLiveReadRef.current = false; if (restoreFrameRef.current !== null) return; const runQueuedRestore = () => { restoreFrameRef.current = null; - if (!restoreBottomOnce()) { + // Tail frames reuse the cache: the observed frame just read fresh + // geometry, so re-reading it every settle frame only re-forces layout. + // A cached correction can itself mean layout moved under the cache, so + // it arms exactly one live verification read on the following frame — + // bounding the whole tail to a single forced layout. + const useLiveRead = restoreTailLiveReadRef.current; + restoreTailLiveReadRef.current = false; + const restored = useLiveRead + ? restoreBottomOnce() + : restoreBottomFromCacheOnce(); + if (!restored) { restoreFramesRemainingRef.current = 0; return; } + if (!useLiveRead) { + restoreTailLiveReadRef.current = true; + } restoreFramesRemainingRef.current -= 1; if (restoreFramesRemainingRef.current > 0) { restoreFrameRef.current = @@ -416,7 +487,7 @@ export function BottomAnchoredScrollBody({ } }; restoreFrameRef.current = window.requestAnimationFrame(runQueuedRestore); - }, [restoreBottomOnce]); + }, [restoreBottomOnce, restoreBottomFromCacheOnce]); const scrollToBottom = useCallback(() => { const scrollArea = scrollAreaRef.current; @@ -512,6 +583,27 @@ export function BottomAnchoredScrollBody({ ); }, []); + const getScrollAnchorRowsCached = useCallback((scrollArea: HTMLElement) => { + const cached = scrollAnchorRowsRef.current; + if ( + cached && + (cached.length === 0 || + (cached[0]?.isConnected === true && + cached[cached.length - 1]?.isConnected === true)) + ) { + return cached; + } + const { rows, windowed } = getScrollAnchorRows(scrollArea); + // A windowed timeline slides its row window inside a fixed-height spacer: + // rows mount and unmount with no observed size change, and its last row + // is force-mounted, so neither the ResizeObserver nor the edge-row + // connectivity check above sees the swap. Only the rows near the viewport + // are mounted there, so querying on every capture is cheap; the cache is + // for the unwindowed timeline, where the query spans the whole thread. + scrollAnchorRowsRef.current = windowed ? null : rows; + return rows; + }, []); + // Persist the current scroll position (top-most visible row + within-row // offset + atBottom) into the per-thread atom so returning to this thread // restores it. Continuous capture keeps the atom current while mounted; cleanup @@ -561,7 +653,10 @@ export function BottomAnchoredScrollBody({ }); return; } - const topMostRow = getTopMostVisibleRow(scrollArea); + const topMostRow = getTopMostVisibleRow( + scrollArea, + getScrollAnchorRowsCached(scrollArea), + ); // No rows yet: don't clobber a good anchor with an empty one. if (!topMostRow) return; store.set(anchorAtom, { @@ -571,6 +666,7 @@ export function BottomAnchoredScrollBody({ }); }, [ + getScrollAnchorRowsCached, hasRecentUserScrollIntent, readMaxScrollOffset, refreshMaxScrollOffset, @@ -579,12 +675,16 @@ export function BottomAnchoredScrollBody({ ], ); + const scrollAnchorCaptureThrottleMs = isPointerCoarse + ? COARSE_SCROLL_ANCHOR_CAPTURE_THROTTLE_MS + : SCROLL_ANCHOR_CAPTURE_THROTTLE_MS; + const captureScrollAnchorThrottled = useCallback(() => { if (scrollAnchorThreadId === undefined) return; const throttle = scrollAnchorCaptureThrottleRef.current; const now = window.performance.now(); const elapsed = now - throttle.lastWriteAt; - if (elapsed >= SCROLL_ANCHOR_CAPTURE_THROTTLE_MS) { + if (elapsed >= scrollAnchorCaptureThrottleMs) { throttle.lastWriteAt = now; writeScrollAnchor(); return; @@ -596,8 +696,8 @@ export function BottomAnchoredScrollBody({ throttle.trailingTimeout = null; throttle.lastWriteAt = window.performance.now(); writeScrollAnchor(); - }, SCROLL_ANCHOR_CAPTURE_THROTTLE_MS - elapsed); - }, [scrollAnchorThreadId, writeScrollAnchor]); + }, scrollAnchorCaptureThrottleMs - elapsed); + }, [scrollAnchorCaptureThrottleMs, scrollAnchorThreadId, writeScrollAnchor]); // Bring the saved anchor row into view (plus its within-row offset). Returns // the resulting scrollTop when the row was found, or null when it isn't yet @@ -821,48 +921,83 @@ export function BottomAnchoredScrollBody({ return true; }, [applyScrollRestore, queueBottomRestore]); - const handleScrollAreaResize = useCallback(() => { - const scrollArea = scrollAreaRef.current; - let shrankOntoBottomWhileDetached = false; - if (scrollArea) { - // The steady-state cache refresh: the observer watches both the scroll - // port and the content wrapper, so every legitimate - // scrollHeight/clientHeight change passes through here. The first - // delivery is also what makes the cache authoritative for hot-path - // reads (see resizeObserverHasDeliveredRef). - const previousMaxScrollOffset = maxScrollOffsetRef.current; - const cacheWasAuthoritative = resizeObserverHasDeliveredRef.current; - const maxScrollOffset = refreshMaxScrollOffset(scrollArea); - resizeObserverHasDeliveredRef.current = true; - shrankOntoBottomWhileDetached = - cacheWasAuthoritative && - !shouldStickToBottomRef.current && - maxScrollOffset < previousMaxScrollOffset && - isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop); - } - // While a restore is pending, the ResizeObserver is the settle signal; the - // bottom-restore is suppressed (stick-to-bottom is false) anyway. - if (advancePendingScrollRestore()) return; - if (shrankOntoBottomWhileDetached && scrollArea) { - // The detached mirror of the attach->detach edge in - // syncBottomStateFromScroll: a content shrink (collapsing a long tool - // output near the end) clamped a detached viewport onto the new, - // smaller maximum. The browser delivered that clamp's scroll event - // before this refresh, so the scroll handler classified it against the - // stale, larger cache and left the viewport detached. A live read used - // to re-attach on that very scroll event; do the same here, against - // fresh geometry, so streaming content keeps following the bottom. - attachToBottom(); - writeScrollAnchor(scrollArea); - } - queueBottomRestore(); - }, [ - advancePendingScrollRestore, - attachToBottom, - queueBottomRestore, - refreshMaxScrollOffset, - writeScrollAnchor, - ]); + const handleScrollAreaResize = useCallback( + (entries: ResizeObserverEntry[]) => { + const scrollArea = scrollAreaRef.current; + // Any observed size change may have mounted or unmounted timeline rows. + scrollAnchorRowsRef.current = null; + let shrankOntoBottomWhileDetached = false; + if (scrollArea) { + // The steady-state cache refresh: the observer watches both the scroll + // port and the content wrapper, so every legitimate + // scrollHeight/clientHeight change passes through here. The first + // delivery is also what makes the cache authoritative for hot-path + // reads (see resizeObserverHasDeliveredRef). + const previousMaxScrollOffset = maxScrollOffsetRef.current; + const cacheWasAuthoritative = resizeObserverHasDeliveredRef.current; + // Prefer the observer's own box sizes over a live + // scrollHeight/clientHeight read, which forces layout: the scroll + // port's content box is its client height (no padding, no horizontal + // scrollbar) and the content wrapper's border box is the scroll + // height, so the pair the observer just measured is the fresh + // geometry. Environments whose observer delivers no entries (test + // stubs) keep the live read. + const observedGeometry = observedScrollGeometryRef.current; + for (const entry of entries) { + if (entry.target === scrollArea) { + observedGeometry.scrollAreaClientHeight = + entry.contentBoxSize[0]?.blockSize ?? entry.contentRect.height; + } else if (entry.target === scrollContentRef.current) { + observedGeometry.scrollContentHeight = + entry.borderBoxSize[0]?.blockSize ?? entry.contentRect.height; + } + } + let maxScrollOffset: number; + if ( + observedGeometry.scrollAreaClientHeight !== null && + observedGeometry.scrollContentHeight !== null + ) { + maxScrollOffset = Math.max( + 0, + Math.round(observedGeometry.scrollContentHeight) - + Math.round(observedGeometry.scrollAreaClientHeight), + ); + maxScrollOffsetRef.current = maxScrollOffset; + } else { + maxScrollOffset = refreshMaxScrollOffset(scrollArea); + } + resizeObserverHasDeliveredRef.current = true; + shrankOntoBottomWhileDetached = + cacheWasAuthoritative && + !shouldStickToBottomRef.current && + maxScrollOffset < previousMaxScrollOffset && + isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop); + } + // While a restore is pending, the ResizeObserver is the settle signal; the + // bottom-restore is suppressed (stick-to-bottom is false) anyway. + if (advancePendingScrollRestore()) return; + if (shrankOntoBottomWhileDetached && scrollArea) { + // The detached mirror of the attach->detach edge in + // syncBottomStateFromScroll: a content shrink (collapsing a long tool + // output near the end) clamped a detached viewport onto the new, + // smaller maximum. The browser delivered that clamp's scroll event + // before this refresh, so the scroll handler classified it against the + // stale, larger cache and left the viewport detached. A live read used + // to re-attach on that very scroll event; do the same here, against + // fresh geometry, so streaming content keeps following the bottom. + attachToBottom(); + writeScrollAnchor(scrollArea); + } + queueBottomRestore(); + }, + [ + advancePendingScrollRestore, + attachToBottom, + queueBottomRestore, + refreshMaxScrollOffset, + writeScrollAnchor, + ], + ); // Begin restoring the saved scroll position on mount, before the listener // effect's `queueBottomRestore()` runs (a useEffect, which runs after layout @@ -935,8 +1070,17 @@ export function BottomAnchoredScrollBody({ if (!scrollArea || !scrollContent) return; let scrollbarIdleTimeout: number | null = null; - const handleScrollWithTransientScrollbar = () => { - scrollArea.dataset.scrollbarScrolling = "true"; + const handleScrollEvent = () => { + // `.thread-scrollbar` hides the thumb at rest on every platform + // (app.css sets `scrollbar-color: transparent transparent` with no + // pointer gate, and Android Chrome / iOS paint their overlay indicators + // with it), so this attribute is what shows the thumb during a touch + // flick too. Write it once per burst: a same-value write is still a + // style invalidation, and that per-event cost is what a scrolling phone + // cannot afford. + if (scrollArea.dataset.scrollbarScrolling !== "true") { + scrollArea.dataset.scrollbarScrolling = "true"; + } if (scrollbarIdleTimeout !== null) { window.clearTimeout(scrollbarIdleTimeout); } @@ -954,7 +1098,7 @@ export function BottomAnchoredScrollBody({ resizeObserver.observe(scrollContent); } - scrollArea.addEventListener("scroll", handleScrollWithTransientScrollbar, { + scrollArea.addEventListener("scroll", handleScrollEvent, { passive: true, }); scrollArea.addEventListener("wheel", markWheelScrollIntent, { @@ -980,10 +1124,7 @@ export function BottomAnchoredScrollBody({ return () => { resizeObserver?.disconnect(); - scrollArea.removeEventListener( - "scroll", - handleScrollWithTransientScrollbar, - ); + scrollArea.removeEventListener("scroll", handleScrollEvent); scrollArea.removeEventListener("wheel", markWheelScrollIntent); scrollArea.removeEventListener("touchstart", markTouchStartScrollIntent); scrollArea.removeEventListener("touchmove", markTouchMoveScrollIntent); diff --git a/apps/app/src/components/ui/disclosure.test.tsx b/apps/app/src/components/ui/disclosure.test.tsx index 64df40de0d..add7a27587 100644 --- a/apps/app/src/components/ui/disclosure.test.tsx +++ b/apps/app/src/components/ui/disclosure.test.tsx @@ -1,8 +1,12 @@ // @vitest-environment jsdom -import { act, cleanup, render } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { Provider, createStore } from "jotai"; +import { useState, type ReactNode } from "react"; +import { flushSync } from "react-dom"; import { afterEach, describe, expect, it, vi } from "vitest"; import { ExpandablePanel } from "./disclosure"; +import { layoutAnimationInFlightCountAtom } from "./layoutAnimationAtoms.js"; class ResizeObserverStub implements ResizeObserver { static instances: ResizeObserverStub[] = []; @@ -21,6 +25,7 @@ afterEach(() => { cleanup(); vi.unstubAllGlobals(); vi.restoreAllMocks(); + vi.useRealTimers(); }); function renderPanel(isExpanded: boolean) { @@ -85,3 +90,184 @@ describe("ExpandablePanel body height", () => { expect(region.style.transitionDuration).toBe("0s"); }); }); + +/** + * A toggleable panel driven by its own header, like a timeline tool row. + * With `collapsedContent` it is a row that shows a preview while collapsed. + */ +function TogglablePanel({ + collapsedContent, +}: { + collapsedContent?: ReactNode; +}) { + const [isExpanded, setIsExpanded] = useState(false); + return ( + setIsExpanded((expanded) => !expanded)} + collapsedContent={collapsedContent} + > + Expanded body + + ); +} + +describe("ExpandablePanel deferred body realization", () => { + it("flips the caret in the tap's commit and mounts the body in a deferred one", () => { + render(); + const header = screen.getByRole("button", { name: "Tool call" }); + + let bodyMountedInToggleCommit: boolean | null = null; + let headerExpandedInToggleCommit: string | null = null; + act(() => { + // flushSync stands in for the tap's discrete event: it flushes only the + // urgent lane, so the deferred body re-render is still pending when the + // samples are taken and lands when act exits. + flushSync(() => { + header.click(); + }); + bodyMountedInToggleCommit = screen.queryByText("Expanded body") !== null; + headerExpandedInToggleCommit = header.getAttribute("aria-expanded"); + }); + + // The tap's synchronous commit flips the caret without paying for the + // body subtree; the body lands in the follow-up interruptible commit. + expect(headerExpandedInToggleCommit).toBe("true"); + expect(bodyMountedInToggleCommit).toBe(false); + expect(screen.getByText("Expanded body")).toBeTruthy(); + }); + + it("keeps the closing body mounted through the collapse animation", () => { + vi.useFakeTimers(); + render(); + const header = screen.getByRole("button", { name: "Tool call" }); + fireEvent.click(header); + const body = screen.getByText("Expanded body"); + + fireEvent.click(header); + + // The collapse animates from the still-rendered subtree: the same body + // element stays mounted for the 200ms transition, then unmounts. + expect(header.getAttribute("aria-expanded")).toBe("false"); + expect(screen.getByText("Expanded body")).toBe(body); + + act(() => { + vi.advanceTimersByTime(200); + }); + expect(screen.queryByText("Expanded body")).toBeNull(); + }); + + it("keeps the preview, its height and the in-flight window until the body's commit", () => { + // jsdom lays nothing out: stand in a text-length metric so the region's + // height write tells the preview, an empty body wrapper and the body apart. + vi.spyOn(HTMLElement.prototype, "offsetHeight", "get").mockImplementation( + function (this: HTMLElement) { + return this.textContent?.length ?? 0; + }, + ); + vi.useFakeTimers(); + const store = createStore(); + render( + + Collapsed summary} /> + , + ); + const header = screen.getByRole("button", { name: "Tool call" }); + const preview = screen.getByText("Collapsed summary"); + const region = preview.parentElement?.parentElement; + if (!region) { + throw new Error("Panel body region was not rendered"); + } + const previewHeight = `${"Collapsed summary".length}px`; + const bodyHeight = `${"Expanded body".length}px`; + expect(region.style.height).toBe(previewHeight); + + let toggleCommit: { + ariaExpanded: string | null; + previewMounted: boolean; + height: string; + transitionDuration: string; + inFlight: number; + } | null = null; + act(() => { + flushSync(() => { + header.click(); + }); + toggleCommit = { + ariaExpanded: header.getAttribute("aria-expanded"), + previewMounted: preview.isConnected, + height: region.style.height, + transitionDuration: region.style.transitionDuration, + inFlight: store.get(layoutAnimationInFlightCountAtom), + }; + }); + + // The tap's commit flips the caret only. The preview stays on screen at + // its own height; no tween or in-flight window opens against the empty + // wrapper the body has not filled yet. + expect(toggleCommit).toEqual({ + ariaExpanded: "true", + previewMounted: true, + height: previewHeight, + transitionDuration: "0s", + inFlight: 0, + }); + + // The deferred commit swaps the preview for the body, eases the region + // toward the body's real height and opens the 200ms in-flight window. + expect(screen.queryByText("Collapsed summary")).toBeNull(); + expect(screen.getByText("Expanded body")).toBeTruthy(); + expect(region.style.height).toBe(bodyHeight); + expect(region.style.transitionDuration).toBe(""); + expect(store.get(layoutAnimationInFlightCountAtom)).toBe(1); + act(() => { + vi.advanceTimersByTime(200); + }); + expect(store.get(layoutAnimationInFlightCountAtom)).toBe(0); + }); + + it("reuses the retained body when a reopen lands inside the close window", () => { + vi.useFakeTimers(); + render(); + const header = screen.getByRole("button", { name: "Tool call" }); + fireEvent.click(header); + const region = screen.getByText("Expanded body").closest("[aria-hidden]"); + if (!region) { + throw new Error("Panel body region was not rendered"); + } + + fireEvent.click(header); + expect(region.getAttribute("aria-hidden")).toBe("true"); + act(() => { + vi.advanceTimersByTime(100); + }); + // The element the close window is retaining while it animates out. + const body = screen.getByText("Expanded body"); + + let reopenCommitBody: Element | null = null; + let reopenCommitAriaHidden: string | null = null; + act(() => { + flushSync(() => { + header.click(); + }); + reopenCommitBody = screen.queryByText("Expanded body"); + reopenCommitAriaHidden = region.getAttribute("aria-hidden"); + }); + + // The reopen tap's commit reopens the region around the subtree the close + // window retained instead of blanking it until the deferred body lands, + // and that deferred commit reconciles into the same element. + expect(reopenCommitBody).toBe(body); + expect(reopenCommitAriaHidden).toBe("false"); + expect(screen.getByText("Expanded body")).toBe(body); + + // The cancelled close no longer unmounts the body when its timer would + // have fired. + act(() => { + vi.advanceTimersByTime(200); + }); + expect(screen.getByText("Expanded body")).toBe(body); + }); +}); diff --git a/apps/app/src/components/ui/disclosure.tsx b/apps/app/src/components/ui/disclosure.tsx index e052238e02..0d92b28735 100644 --- a/apps/app/src/components/ui/disclosure.tsx +++ b/apps/app/src/components/ui/disclosure.tsx @@ -1,5 +1,6 @@ import { useSetAtom } from "jotai"; import { + useDeferredValue, useEffect, useLayoutEffect, useMemo, @@ -8,10 +9,20 @@ import { type ReactNode, } from "react"; import { cn } from "@bb/shared-ui/lib/utils"; +import { + observedBorderBoxBlockSize, + observeSharedResize, +} from "@/lib/shared-resize-observer"; import { layoutAnimationInFlightCountAtom } from "./layoutAnimationAtoms.js"; import { CONTROL_HOVER_TRANSITION } from "@bb/shared-ui/motion"; const EXPANDABLE_PANEL_TRANSITION_MS = 200; + +/** Read half of the panel height sync, staged for the shared write phase. */ +interface PanelHeightSync { + isToggleAnimating: boolean; + heightPx: number; +} const useBrowserLayoutEffect = typeof window === "undefined" ? useEffect : useLayoutEffect; @@ -129,14 +140,15 @@ interface ExpandablePanelProps { interface AnimatedExpandablePanelContentProps { collapsedContent: ReactNode; contentClassName?: string; - isExpanded: boolean; + /** The body is mounted; see `ExpandablePanel`'s `isBodyExpanded`. */ + isBodyExpanded: boolean; renderedBody: ReactNode; } function AnimatedExpandablePanelContent({ collapsedContent, contentClassName, - isExpanded, + isBodyExpanded, renderedBody, }: AnimatedExpandablePanelContentProps) { const regionRef = useRef(null); @@ -155,7 +167,7 @@ function AnimatedExpandablePanelContent({ } toggleAnimationDeadlineRef.current = performance.now() + EXPANDABLE_PANEL_TRANSITION_MS; - }, [isExpanded]); + }, [isBodyExpanded]); useBrowserLayoutEffect(() => { const region = regionRef.current; @@ -164,23 +176,38 @@ function AnimatedExpandablePanelContent({ return; } - const syncHeight = () => { - const isToggleAnimating = - performance.now() < toggleAnimationDeadlineRef.current; + const readHeightSync = ( + entry: ResizeObserverEntry | undefined, + ): PanelHeightSync => { + // The entry's border box is offsetHeight's metric without the layout + // read; the mount sync below has no entry and pays that read once. + const observedHeight = + entry === undefined ? undefined : observedBorderBoxBlockSize(entry); + return { + isToggleAnimating: + performance.now() < toggleAnimationDeadlineRef.current, + heightPx: observedHeight ?? target.offsetHeight, + }; + }; + const writeHeightSync = ({ + heightPx, + isToggleAnimating, + }: PanelHeightSync) => { region.style.transitionDuration = isToggleAnimating ? "" : "0s"; - region.style.height = `${target.offsetHeight}px`; + region.style.height = `${heightPx}px`; }; - syncHeight(); + writeHeightSync(readHeightSync(undefined)); if (typeof ResizeObserver === "undefined") { return; } - const resizeObserver = new ResizeObserver(syncHeight); - resizeObserver.observe(target); - return () => resizeObserver.disconnect(); - }, [collapsedContent, isExpanded, renderedBody]); + return observeSharedResize(target, { + read: readHeightSync, + write: writeHeightSync, + }); + }, [collapsedContent, isBodyExpanded, renderedBody]); return (
- {isExpanded ? ( + {isBodyExpanded ? (
{renderedBody}
@@ -223,12 +250,30 @@ export function ExpandablePanel({ const headerRootClassName = cn("px-2 py-1", headerClassName); const [isClosing, setIsClosing] = useState(false); const renderedBodyRef = useRef(null); + // The header/chevron flip stays urgent (it reads `isExpanded` directly), + // but the body realizes off the deferred value: an expand tap's discrete + // commit paints the caret in the first frame, and the body subtree — the + // expensive part of a large tool section — mounts in a follow-up + // interruptible commit. On first render the deferred value equals + // `isExpanded`, so rows that mount already expanded render their body + // immediately. + const deferredIsExpanded = useDeferredValue(isExpanded); const expandedBody = useMemo(() => { - if (!isExpanded) { + if (!deferredIsExpanded) { return null; } return renderBody ? renderBody() : children; - }, [children, isExpanded, renderBody]); + }, [children, deferredIsExpanded, renderBody]); + // The body region follows the body, not the tap: its content branch, + // transition classes, height tween, toggle deadline and in-flight window + // all key on this flag. It opens in the commit that mounts the body — the + // deferred one, or the reopen tap's own commit while the close window + // still retains the subtree (`isClosing` holds only while + // `renderedBodyRef` does) — and closes in the collapse tap's commit. + // Opening it on the urgent `isExpanded` would swap the collapsed preview + // for an empty wrapper and tween toward that, with the body landing a + // commit later, possibly after the 200ms window has already closed. + const isBodyExpanded = isExpanded && (deferredIsExpanded || isClosing); // Signal to AutoHeightContainer / HeightTransition wrappers that a // CSS-driven layout animation is in flight, so they snap their wrapper to @@ -257,14 +302,18 @@ export function ExpandablePanel({ window.clearTimeout(timer); release(); }; - }, [isExpanded, setLayoutAnimationInFlightCount]); + }, [isBodyExpanded, setLayoutAnimationInFlightCount]); + // Retain the last realized body for the close window. Gated on the + // deferred value: between an expand tap and its body commit `expandedBody` + // is still null, and writing that would drop the subtree a reopen inside + // the close window reuses. useBrowserLayoutEffect(() => { - if (!isExpanded) { + if (!deferredIsExpanded) { return; } renderedBodyRef.current = expandedBody; - }, [expandedBody, isExpanded]); + }, [deferredIsExpanded, expandedBody]); useBrowserLayoutEffect(() => { if (isExpanded) { @@ -285,7 +334,10 @@ export function ExpandablePanel({ }, EXPANDABLE_PANEL_TRANSITION_MS); return () => clearTimeout(timeout); }, [hasCollapsedContent, isExpanded]); - const renderedBody = isExpanded + // While the deferred value still says expanded the realized body is the + // freshest subtree (a collapse tap animates out from it without a + // remount); otherwise the close window's retained one. + const renderedBody = deferredIsExpanded ? expandedBody : isClosing ? renderedBodyRef.current @@ -314,15 +366,17 @@ export function ExpandablePanel({ ) : (
{ }); }); +function makeResizeEntry( + target: Element, + borderBoxBlockSize: number, + contentRectHeight: number, +): ResizeObserverEntry { + return { + target, + contentRect: new DOMRect(0, 0, 200, contentRectHeight), + borderBoxSize: [{ blockSize: borderBoxBlockSize, inlineSize: 200 }], + contentBoxSize: [{ blockSize: contentRectHeight, inlineSize: 200 }], + devicePixelContentBoxSize: [ + { blockSize: borderBoxBlockSize, inlineSize: 200 }, + ], + }; +} + describe("AutoHeightContainer", () => { + it("sizes the wrapper from the observed border box", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + + const view = render( + + Streaming response + , + ); + const inner = view.getByText("Streaming response").parentElement; + const wrapper = inner?.parentElement; + const observer = ResizeObserverStub.instances[0]; + if (!inner || !wrapper || !observer) { + throw new Error("AutoHeightContainer did not render"); + } + + // A padded inner: the border box (offsetHeight's metric, used by the + // mount and snap paths) is taller than the content rect. Sizing the + // wrapper from the content rect would clip it. + act(() => { + observer.callback([makeResizeEntry(inner, 120, 112)], observer); + }); + + expect(wrapper.style.height).toBe("120px"); + }); + it("snap-syncs an authoritative layout revision", () => { vi.stubGlobal("ResizeObserver", ResizeObserverStub); diff --git a/apps/app/src/components/ui/height-transition.tsx b/apps/app/src/components/ui/height-transition.tsx index c01c03d720..ae3702c76d 100644 --- a/apps/app/src/components/ui/height-transition.tsx +++ b/apps/app/src/components/ui/height-transition.tsx @@ -8,6 +8,10 @@ import { subscribeToDocumentVisibility, } from "@/lib/document-visibility"; import { supportsScrollAnchoring } from "@/lib/scroll-anchoring-support"; +import { + observedBorderBoxBlockSize, + observeSharedResize, +} from "@/lib/shared-resize-observer"; import { layoutAnimationInFlightCountAtom } from "./layoutAnimationAtoms.js"; // Shared animation tokens for height transitions across the timeline. @@ -141,6 +145,21 @@ function cancelIntrinsicHeightRestore( resizeState.restoreTimerId = null; } +// The observer already measured the inner this frame, so reading the entry +// costs nothing, and the border box is the same metric as the offsetHeight +// used by the non-observer paths (initial mount, visibility snap) — the two +// must agree or a padded inner would get clipped by a content-box height. +// A dispatch without an entry (the shared observer's broadcast re-sync) or +// with a box-less synthetic one pays the offsetHeight read instead. +function getObservedInnerHeight( + entry: ResizeObserverEntry | undefined, + inner: HTMLElement, +): number { + const observed = + entry === undefined ? undefined : observedBorderBoxBlockSize(entry); + return observed ?? inner.offsetHeight; +} + interface HeightTransitionProps { visible: boolean; children: ReactNode; @@ -168,24 +187,34 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { let lastWidth: number | null = null; let pendingVisibilitySnap = false; const snapState: SnapState = { savedDuration: null, restoreFrame: null }; - const observer = new ResizeObserver((entries) => { - const entry = entries[0]; - if (!entry) return; - const { width, height } = entry.contentRect; - const widthChanged = lastWidth !== null && width !== lastWidth; - // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) - // is in flight, the inner is itself animating its size every frame. - // Running our own 180ms transition on top compounds the lag and drags - // scrollHeight, which the bottom-anchor sentinel then chases. - const layoutAnimationActive = - store.get(layoutAnimationInFlightCountAtom) > 0; - const snap = widthChanged || pendingVisibilitySnap || layoutAnimationActive; - pendingVisibilitySnap = false; - lastWidth = width; - const nextHeight = visible ? `${height}px` : "0px"; - applyHeight(wrapper, nextHeight, snap, snapState); + const unobserveInner = observeSharedResize(inner, { + read: (entry) => { + const width = entry?.contentRect?.width; + const widthChanged = + lastWidth !== null && width !== undefined && width !== lastWidth; + // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) + // is in flight, the inner is itself animating its size every frame. + // Running our own 180ms transition on top compounds the lag and drags + // scrollHeight, which the bottom-anchor sentinel then chases. + const layoutAnimationActive = + store.get(layoutAnimationInFlightCountAtom) > 0; + const snap = + widthChanged || pendingVisibilitySnap || layoutAnimationActive; + pendingVisibilitySnap = false; + if (width !== undefined) { + lastWidth = width; + } + return { + nextHeight: visible + ? `${getObservedInnerHeight(entry, inner)}px` + : "0px", + snap, + }; + }, + write: ({ nextHeight, snap }) => { + applyHeight(wrapper, nextHeight, snap, snapState); + }, }); - observer.observe(inner); // While a tab is hidden, ResizeObserver delivery is throttled and the CSS // height transition stays armed. If content grew during streaming, the // first observer fire after the user returns interpolates the full delta @@ -202,7 +231,7 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { const unsubscribeFromDocumentVisibility = subscribeToDocumentVisibility(onVisibility); return () => { - observer.disconnect(); + unobserveInner(); unsubscribeFromDocumentVisibility(); cleanupSnapState(wrapper, snapState); }; @@ -331,39 +360,51 @@ export function AutoHeightContainer({ initialSettleComplete = true; }, AUTO_HEIGHT_INITIAL_SETTLE_MS); }; - const observer = new ResizeObserver((entries) => { - const entry = entries[0]; - if (!entry) return; - const { width, height } = entry.contentRect; - const widthChanged = lastWidth !== null && width !== lastWidth; - // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) - // is in flight, the inner is itself animating its size every frame. - // Running our own 180ms transition on top compounds the lag and drags - // scrollHeight, which the bottom-anchor sentinel then chases. - const layoutAnimationActive = - store.get(layoutAnimationInFlightCountAtom) > 0; - const snap = - widthChanged || - pendingVisibilitySnap || - !initialSettleComplete || - layoutAnimationActive; - pendingVisibilitySnap = false; - lastWidth = width; - if (widthChanged || resizeState.usingIntrinsicHeight) { - enterIntrinsicHeightMode(wrapper, resizeState, snapState); - scheduleIntrinsicHeightRestore({ - inner, - resizeState, - snapState, - target: wrapper, - }); + const unobserveInner = observeSharedResize(inner, { + read: (entry) => { + const width = entry?.contentRect?.width; + const widthChanged = + lastWidth !== null && width !== undefined && width !== lastWidth; + // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) + // is in flight, the inner is itself animating its size every frame. + // Running our own 180ms transition on top compounds the lag and drags + // scrollHeight, which the bottom-anchor sentinel then chases. + const layoutAnimationActive = + store.get(layoutAnimationInFlightCountAtom) > 0; + const snap = + widthChanged || + pendingVisibilitySnap || + !initialSettleComplete || + layoutAnimationActive; + pendingVisibilitySnap = false; + if (width !== undefined) { + lastWidth = width; + } + if (widthChanged || resizeState.usingIntrinsicHeight) { + return { useIntrinsicHeight: true as const }; + } + return { + useIntrinsicHeight: false as const, + nextHeight: `${getObservedInnerHeight(entry, inner)}px`, + snap, + }; + }, + write: (sync) => { + if (sync.useIntrinsicHeight) { + enterIntrinsicHeightMode(wrapper, resizeState, snapState); + scheduleIntrinsicHeightRestore({ + inner, + resizeState, + snapState, + target: wrapper, + }); + deferInitialSettleComplete(); + return; + } + applyHeight(wrapper, sync.nextHeight, sync.snap, snapState); deferInitialSettleComplete(); - return; - } - applyHeight(wrapper, `${height}px`, snap, snapState); - deferInitialSettleComplete(); + }, }); - observer.observe(inner); // See HeightTransition's matching block: a hidden tab pauses observer // delivery and the height transition, so content streamed in while the // tab was backgrounded would otherwise animate in over 180ms on return @@ -378,7 +419,7 @@ export function AutoHeightContainer({ subscribeToDocumentVisibility(onVisibility); return () => { snapToCurrentHeightRef.current = null; - observer.disconnect(); + unobserveInner(); unsubscribeFromDocumentVisibility(); window.clearTimeout(initialSettleTimerId); cancelIntrinsicHeightRestore(resizeState); diff --git a/apps/app/src/components/ui/sidebar.test.tsx b/apps/app/src/components/ui/sidebar.test.tsx index 86f26aff67..0c2932f1d5 100644 --- a/apps/app/src/components/ui/sidebar.test.tsx +++ b/apps/app/src/components/ui/sidebar.test.tsx @@ -8,6 +8,7 @@ import { screen, } from "@testing-library/react"; import { memo } from "react"; +import { flushSync } from "react-dom"; import { renderToString } from "react-dom/server"; import { afterEach, describe, expect, it, vi } from "vitest"; import { CompactViewportOverrideProvider } from "@bb/shared-ui/hooks/use-compact-viewport"; @@ -331,6 +332,37 @@ describe("mobile sidebar deferred realization", () => { expect(getMobilePanel()?.textContent).toContain("Sidebar content"); }); + it("keeps the realize commit out of the open tap's synchronous flush", () => { + vi.useFakeTimers(); + renderCompactSidebarHarness(); + const trigger = screen.getByRole("button", { name: "Toggle Sidebar" }); + + let panelStyledForSlideInTap = false; + let realizedInTapFlush = true; + act(() => { + // flushSync stands in for the tap's discrete event: it flushes only the + // urgent lane, so the transition-priority realize commit is still + // pending when the samples are taken and lands when act exits. + flushSync(() => { + trigger.click(); + }); + const panel = getMobilePanel(); + panelStyledForSlideInTap = panel?.style.translate === "0%"; + realizedInTapFlush = + panel?.textContent?.includes("Sidebar content") ?? false; + }); + + // The tap's own flush only starts the slide (inline drag styles); the + // subtree mounts in the interruptible commit that follows, so the first + // frame of the slide never waits on the realize commit. + expect(panelStyledForSlideInTap).toBe(true); + expect(realizedInTapFlush).toBe(false); + expect(getMobilePanel()?.textContent).toContain("Sidebar content"); + + settleMobileToggle(); + expect(getMobilePanel()?.dataset.state).toBe("open"); + }); + // The width is an inherited custom property unless registered otherwise // (theme.css registers it non-inherited). Either way it must be written on // the elements that read it and never on the provider wrapper: the wrapper diff --git a/apps/app/src/components/ui/sidebar.tsx b/apps/app/src/components/ui/sidebar.tsx index fbfebd844b..f04b1c7292 100644 --- a/apps/app/src/components/ui/sidebar.tsx +++ b/apps/app/src/components/ui/sidebar.tsx @@ -636,8 +636,15 @@ const SidebarProvider = React.forwardRef< } // Mount the subtree now if boot has not realized it yet, so it commits - // during the slide instead of after the settle. - realizeMobileSidebar(); + // during the slide instead of after the settle. Transition priority: + // the realize commit (ProjectList/ThreadRow, thousands of lines on a + // cold route) must not block this tap's frame — the drag-style write + // below composites the slide first and the subtree mounts + // interruptibly during the settle window. If the settle commit beats + // the transition, the render-phase latch below realizes it there. + React.startTransition(() => { + realizeMobileSidebar(); + }); applySidebarMobileDragStyles({ progress: 1, settling: true }); mobileSettleTimeoutRef.current = window.setTimeout(() => { mobileSettleTimeoutRef.current = null; diff --git a/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts b/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts index a4a849dee4..d2d1822cf3 100644 --- a/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts +++ b/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts @@ -127,6 +127,14 @@ interface TimelineInvalidationQueryKeysArgs { } interface ScheduleTrailingActiveRefetchArgs { + /** + * Match `queryKey` exactly instead of as a prefix. Leaf thread-list keys + * enumerated from the cache must be exact: list filters are sparse, so a + * project list key is a prefix of that project's forks-row key, and a + * prefix match would refetch the forks list with the project list and then + * again from the forks key's own run. + */ + exact: boolean; queryClient: QueryClient; queryKey: QueryKey; } @@ -152,6 +160,8 @@ const throttledActiveRefetchEntries = new WeakMap< >(); interface ThrottledActiveRefetchArgs { + /** See {@link ScheduleTrailingActiveRefetchArgs.exact}. */ + exact: boolean; minIntervalMs: number; queryClient: QueryClient; queryKey: QueryKey; @@ -167,6 +177,17 @@ interface ThrottledActiveRefetchArgs { */ const WORK_STATUS_REFETCH_MIN_INTERVAL_MS = 1_000; +/** + * A `status-changed` push without a row snapshot falls back to refetching the + * active thread lists. Bare pushes arrive in bursts — writers inside a + * transaction publish one per thread, and a host disconnect used to publish + * one per thread on the host — and every full list response is ~1 KB per + * unarchived thread, so the fallback refetch is throttled to one per second + * per query. Rows still go stale immediately; only the active refetch + * coalesces. + */ +const THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS = 1_000; + /** * The trailing refetch is self-clocking: it fires as soon as the in-flight * fetch settles and any event arrived meanwhile. During a streaming turn events @@ -198,10 +219,11 @@ function timelineInvalidationKey(queryKey: QueryKey): string { function hasActiveFetchingQueries( queryClient: QueryClient, queryKey: QueryKey, + exact: boolean, ): boolean { return queryClient .getQueryCache() - .findAll({ queryKey, type: "active" }) + .findAll({ exact, queryKey, type: "active" }) .some((query) => query.state.fetchStatus !== "idle"); } @@ -215,18 +237,22 @@ function hasActiveQueries( } function refetchActiveQueriesWithoutCanceling({ + exact, queryClient, queryKey, }: ScheduleTrailingActiveRefetchArgs): void { - const hadActiveFetch = hasActiveFetchingQueries(queryClient, queryKey); + const hadActiveFetch = hasActiveFetchingQueries(queryClient, queryKey, exact); void queryClient - .refetchQueries({ queryKey, type: "active" }, { cancelRefetch: false }) + .refetchQueries( + { exact, queryKey, type: "active" }, + { cancelRefetch: false }, + ) .catch(() => { // Individual query state already captures the refetch error. }); if (hadActiveFetch) { // A change that raced the in-flight read must not be lost. - scheduleTrailingActiveRefetch({ queryClient, queryKey }); + scheduleTrailingActiveRefetch({ exact, queryClient, queryKey }); } } @@ -237,11 +263,12 @@ function refetchActiveQueriesWithoutCanceling({ * coalesce into one trailing refetch. Never cancels an in-flight fetch. */ function invalidateQueryKeyWithThrottledActiveRefetch({ + exact, minIntervalMs, queryClient, queryKey, }: ThrottledActiveRefetchArgs): void { - queryClient.invalidateQueries({ queryKey, refetchType: "none" }); + queryClient.invalidateQueries({ exact, queryKey, refetchType: "none" }); const scheduleKey = timelineInvalidationKey(queryKey); let entries = throttledActiveRefetchEntries.get(queryClient); @@ -256,7 +283,7 @@ function invalidateQueryKeyWithThrottledActiveRefetch({ } const run = () => { entries.set(scheduleKey, { lastRunAt: Date.now(), timer: null }); - refetchActiveQueriesWithoutCanceling({ queryClient, queryKey }); + refetchActiveQueriesWithoutCanceling({ exact, queryClient, queryKey }); }; const lastRunAt = entry?.lastRunAt ?? Number.NEGATIVE_INFINITY; const delayMs = Math.max(0, lastRunAt + minIntervalMs - Date.now()); @@ -268,6 +295,7 @@ function invalidateQueryKeyWithThrottledActiveRefetch({ } function scheduleTrailingActiveRefetch({ + exact, queryClient, queryKey, }: ScheduleTrailingActiveRefetchArgs): void { @@ -288,7 +316,7 @@ function scheduleTrailingActiveRefetch({ const waitingSince = Date.now(); const unsubscribe = queryClient.getQueryCache().subscribe(() => { - if (hasActiveFetchingQueries(queryClient, queryKey)) { + if (hasActiveFetchingQueries(queryClient, queryKey, exact)) { return; } @@ -298,7 +326,10 @@ function scheduleTrailingActiveRefetch({ const timer = setTimeout(() => { unsubscribers.delete(scheduleKey); void queryClient - .refetchQueries({ queryKey, type: "active" }, { cancelRefetch: false }) + .refetchQueries( + { exact, queryKey, type: "active" }, + { cancelRefetch: false }, + ) .catch(() => { // Individual query state already captures the refetch error. }); @@ -333,12 +364,16 @@ function invalidateQueryKeysWithoutCancelingActiveFetches({ queryKeys, }: TimelineInvalidationQueryKeysArgs): void { for (const queryKey of queryKeys) { - const hadActiveFetch = hasActiveFetchingQueries(queryClient, queryKey); - // Avoid aborting the active timeline request on every event batch, but keep - // one trailing refetch so an event that raced the in-flight read is not lost. + const hadActiveFetch = hasActiveFetchingQueries( + queryClient, + queryKey, + false, + ); + // Avoid aborting the active request on every event batch, but keep one + // trailing refetch so an event that raced the in-flight read is not lost. queryClient.invalidateQueries({ queryKey }, { cancelRefetch: false }); if (hadActiveFetch) { - scheduleTrailingActiveRefetch({ queryClient, queryKey }); + scheduleTrailingActiveRefetch({ exact: false, queryClient, queryKey }); } } } @@ -831,6 +866,56 @@ function dirtyActiveThreadListQueries({ return [sidebarNavigationQueryKey(), threadSearchQueryKeyPrefix()]; } +/** + * Same scope as {@link dirtyActiveThreadListQueries}, but the active refetches + * run through the throttle machinery: everything goes stale immediately (a + * remount refetches), while active list/sidebar/search observers refetch at + * most once per {@link THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS}, + * with later changes coalescing into one trailing refetch and no fetch in + * flight ever cancelled. Archived pages keep their stale-only treatment. + */ +function dirtyActiveThreadListQueriesWithThrottledRefetch({ + projectId, + queryClient, +}: ThreadRealtimeDirtyContext): void { + const listQueryKeys = projectId + ? [ + ...getCachedProjectThreadListInvalidationQueryKeys({ + projectId, + queryClient, + }), + ...getCachedGlobalThreadListInvalidationQueryKeys({ queryClient }), + ] + : getCachedThreadListQueryKeys(queryClient); + for (const queryKey of listQueryKeys) { + if (isArchivedThreadListQueryKey(queryKey)) { + queryClient.invalidateQueries({ + exact: true, + queryKey, + refetchType: "none", + }); + continue; + } + invalidateQueryKeyWithThrottledActiveRefetch({ + exact: true, + minIntervalMs: THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS, + queryClient, + queryKey, + }); + } + for (const queryKey of [ + sidebarNavigationQueryKey(), + threadSearchQueryKeyPrefix(), + ]) { + invalidateQueryKeyWithThrottledActiveRefetch({ + exact: false, + minIntervalMs: THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS, + queryClient, + queryKey, + }); + } +} + function dirtyThreadListQueriesForBackgroundActivity( context: ThreadRealtimeDirtyContext, ): QueryKey[] { @@ -895,7 +980,9 @@ function dirtyThreadSearchQueries(): QueryKey[] { * `events-appended` batches. Re-issuing (and, by default, aborting) the open * search request on each 50-100 ms flush can starve it forever on a slow link, * so search only goes stale when a turn completes, once per flush, and without - * cancelling a request in flight. Thread list changes cover the rest. + * cancelling a request in flight — a request already running read the index + * before the turn settled, so one trailing refetch follows it. Thread list + * changes cover the rest. */ function dirtyThreadSearchQueriesForCompletedTurn({ eventTypes, @@ -908,10 +995,10 @@ function dirtyThreadSearchQueriesForCompletedTurn({ if (!flushOnce("thread-search:turn-completed")) { return; } - queryClient.invalidateQueries( - { queryKey: threadSearchQueryKeyPrefix() }, - { cancelRefetch: false }, - ); + invalidateQueryKeysWithoutCancelingActiveFetches({ + queryClient, + queryKeys: [threadSearchQueryKeyPrefix()], + }); } function dirtyThreadTimelineQueries({ @@ -1108,24 +1195,35 @@ function patchThreadListPendingInteractionState({ * patched in place. The alternative is what the fallback still does for * pushes without the row (older servers, writers inside a transaction that * cannot resolve the runtime): refetch every active thread list plus the - * sidebar bootstrap, which is ~1 KB per unarchived thread, twice per turn. + * sidebar bootstrap, which is ~1 KB per unarchived thread — throttled to one + * active refetch per second so a burst of bare pushes coalesces. * * A list fetch already in flight read the database before this transition * and would overwrite the patch when it lands, so those queries are * invalidated, which cancels and restarts them. */ -function patchThreadListStatusState( - context: ThreadRealtimeDirtyContext, -): QueryKey[] { - const { queryClient, statusChange, threadId } = context; +function patchThreadListStatusState(context: ThreadRealtimeDirtyContext): void { + const { flushOnce, queryClient, statusChange, threadId } = context; if (!threadId || !statusChange) { - return dirtyActiveThreadListQueries(context); + dirtyActiveThreadListQueriesWithThrottledRefetch(context); + return; } updateCachedThreadListStatusState(queryClient, threadId, statusChange); for (const queryKey of getFetchingThreadListQueryKeys(queryClient)) { queryClient.invalidateQueries({ exact: true, queryKey }); } - return [threadSearchQueryKeyPrefix()]; // Result rows render status but are not list-shaped. + // Result rows render status but are not list-shaped, so search refreshes + // rather than patches — once per flush and without aborting a request in + // flight: status changes ride the immediate path, and the default + // cancelling invalidation could starve an open search on a slow link. A + // request already in flight read the index before this transition, and + // landing it clears the invalidation, so one trailing refetch follows it. + if (flushOnce("thread-search:status-changed")) { + invalidateQueryKeysWithoutCancelingActiveFetches({ + queryClient, + queryKeys: [threadSearchQueryKeyPrefix()], + }); + } } function dirtyEnvironmentRecordQueries( @@ -1151,6 +1249,7 @@ function dirtyEnvironmentLiveWorkspaceStateQueries({ queryClient, }: EnvironmentRealtimeDirtyContext): void { invalidateQueryKeyWithThrottledActiveRefetch({ + exact: false, minIntervalMs: WORK_STATUS_REFETCH_MIN_INTERVAL_MS, queryClient, queryKey: environmentWorkStatusQueryKeyPrefix(environmentId), diff --git a/apps/app/src/hooks/queries/query-policies.ts b/apps/app/src/hooks/queries/query-policies.ts index 803f186ffb..4e6ca17406 100644 --- a/apps/app/src/hooks/queries/query-policies.ts +++ b/apps/app/src/hooks/queries/query-policies.ts @@ -21,12 +21,25 @@ export const SERVER_SESSION_QUERY_POLICY = { staleTime: SERVER_SESSION_STALE_TIME_MS, } as const; +/** + * Live values with no realtime change kind (provider usage limits): focus and + * reconnect are their only freshness sources, so the explicit `true`s + * deliberately bypass the app-level lost-realtime-coverage gate that + * `createAppQueryClient` applies to the defaults. + */ export const FOCUS_OWNED_LIVE_QUERY_POLICY = { refetchOnReconnect: true, refetchOnWindowFocus: true, staleTime: FOCUS_OWNED_LIVE_STALE_TIME_MS, } as const; +/** + * Explicit resume opt-in for queries whose realtime coverage has gaps: thread + * tabs are absent from the reconnect-watermark catch-up list, and the thread + * host file preview backs an open pane that must not keep stale bytes after + * an offline stretch. Deliberately bypasses the app-level + * lost-realtime-coverage gate (per-query options win over the defaults). + */ export const RESUME_REFETCH_QUERY_POLICY = { refetchOnReconnect: true, refetchOnWindowFocus: true, diff --git a/apps/app/src/hooks/realtime-cache-effects.coarse-pointer.test.ts b/apps/app/src/hooks/realtime-cache-effects.coarse-pointer.test.ts new file mode 100644 index 0000000000..01bb06f7ef --- /dev/null +++ b/apps/app/src/hooks/realtime-cache-effects.coarse-pointer.test.ts @@ -0,0 +1,68 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createAppQueryClient } from "@/lib/query-client"; +import { threadTimelineQueryKey } from "./queries/query-keys"; + +/** + * Kept apart from `realtime-cache-effects.test.ts`: the pointer class is read + * from `matchMedia` once at module init, so the coarse branch needs a fresh + * module instance with a stubbed window, and `vi.stubGlobal`/`vi.resetModules` + * move a file out of the shared vitest worker (see vitest.shared.ts). Only + * this case pays for the module-graph re-import; the main suite stays shared. + */ +describe("createRealtimeCacheEffects on coarse pointers", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("widens the thread invalidation debounce on coarse pointers", async () => { + vi.useFakeTimers(); + // `location` rides along because the re-imported graph reaches the sdk + // module, which resolves its base URL from the window at init. + vi.stubGlobal("window", { + location: { origin: "http://localhost" }, + matchMedia: (query: string) => ({ + matches: query === "(pointer: coarse)", + }), + }); + vi.resetModules(); + try { + const coarseModule = await import("./realtime-cache-effects"); + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + gcTime: Infinity, + retry: false, + }, + }, + showMutationErrorToasts: false, + }); + const effects = coarseModule.createRealtimeCacheEffects({ queryClient }); + const timelineKey = threadTimelineQueryKey("thr_1"); + queryClient.setQueryData(timelineKey, { rows: [] }); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { + eventTypes: ["item/agentMessage/delta"], + projectId: "project-1", + }, + changes: ["events-appended"], + }); + + // The fine-pointer cadence would have flushed at 50 ms. + vi.advanceTimersByTime(50); + expect(queryClient.getQueryState(timelineKey)?.isInvalidated).not.toBe( + true, + ); + vi.advanceTimersByTime(100); + expect(queryClient.getQueryState(timelineKey)?.isInvalidated).toBe(true); + + effects.dispose(); + } finally { + vi.unstubAllGlobals(); + vi.resetModules(); + } + }); +}); diff --git a/apps/app/src/hooks/realtime-cache-effects.test.ts b/apps/app/src/hooks/realtime-cache-effects.test.ts index 284b7e378e..1a11ca700e 100644 --- a/apps/app/src/hooks/realtime-cache-effects.test.ts +++ b/apps/app/src/hooks/realtime-cache-effects.test.ts @@ -42,6 +42,7 @@ import { import { pluginContributionsQueryKey } from "./queries/query-keys"; import { createRealtimeCacheEffects, + resolveThreadInvalidationDebounce, type RealtimeCacheEffectsVisibility, } from "./realtime-cache-effects"; import { @@ -505,7 +506,7 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); - it("refreshes an open search once per flush without aborting the request in flight", async () => { + it("refreshes an open search once per flush without aborting the request in flight, then once it settles", async () => { vi.useFakeTimers(); const visibility = createFakeVisibility(); const { effects, queryClient } = @@ -559,16 +560,118 @@ describe("createRealtimeCacheEffects", () => { expect(signals[0]?.aborted).toBe(false); expect(searchQueryFn).toHaveBeenCalledTimes(1); + // That request read the index before the turns completed, and landing + // it clears the invalidation: one trailing refetch picks up the newly + // indexed content. resolveFetches[0]?.({ active: { results: [], total: 0 }, archived: { results: [], total: 0 }, }); await vi.advanceTimersByTimeAsync(0); + expect(searchQueryFn).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1_000); + expect(searchQueryFn).toHaveBeenCalledTimes(2); + expect(signals[0]?.aborted).toBe(false); + + resolveFetches[1]?.({ + active: { results: [], total: 0 }, + archived: { results: [], total: 0 }, + }); + await vi.advanceTimersByTimeAsync(2_000); + expect(searchQueryFn).toHaveBeenCalledTimes(2); + invalidateSpy.mockRestore(); unsubscribeSearch(); effects.dispose(); }); + it("refetches an in-flight search once it settles after a status change instead of aborting it", async () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const threadSearchKey = threadSearchQueryKey({ + limitPerGroup: 20, + query: "needle", + }); + const idleResponse = { + active: { results: [{ id: "thr_1", status: "idle" }], total: 1 }, + archived: { results: [], total: 0 }, + }; + const activeResponse = { + active: { results: [{ id: "thr_1", status: "active" }], total: 1 }, + archived: { results: [], total: 0 }, + }; + // Cached data matters: the default cancelling invalidation only aborts + // and re-issues a fetch when the query already holds data — exactly the + // open-search-refreshing case a streaming turn's status flips would starve. + queryClient.setQueryData(threadSearchKey, idleResponse); + const signals: AbortSignal[] = []; + const resolveFetches: Array<(value: unknown) => void> = []; + const searchQueryFn = vi.fn(({ signal }: { signal: AbortSignal }) => { + signals.push(signal); + return new Promise((resolve) => { + resolveFetches.push(resolve); + }); + }); + const searchObserver = new QueryObserver(queryClient, { + queryKey: threadSearchKey, + queryFn: searchQueryFn, + staleTime: Infinity, + }); + const unsubscribeSearch = searchObserver.subscribe(() => {}); + void searchObserver.refetch(); + await vi.advanceTimersByTimeAsync(0); + expect(searchQueryFn).toHaveBeenCalledTimes(1); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { + projectId: "project-1", + statusChange: { + activity: NO_THREAD_ACTIVITY, + latestAttentionAt: 100, + runtime: { + displayStatus: "active", + hostReconnectGraceExpiresAt: null, + }, + status: "active", + updatedAt: 200, + }, + }, + changes: ["status-changed"], + }); + await vi.advanceTimersByTimeAsync(0); + + // Status changes ride the immediate path, so a cancelling invalidation + // here could starve an open search forever; the request keeps running. + expect(signals[0]?.aborted).toBe(false); + expect(searchQueryFn).toHaveBeenCalledTimes(1); + + // The request read the index before the transition, so its response + // still carries the old status. Landing it clears the invalidation, which + // would lose the change for good without a trailing refetch. + resolveFetches[0]?.(idleResponse); + await vi.advanceTimersByTimeAsync(0); + expect(queryClient.getQueryData(threadSearchKey)).toEqual(idleResponse); + await vi.advanceTimersByTimeAsync(1_000); + expect(searchQueryFn).toHaveBeenCalledTimes(2); + expect(signals[0]?.aborted).toBe(false); + + resolveFetches[1]?.(activeResponse); + await vi.advanceTimersByTimeAsync(0); + expect(queryClient.getQueryData(threadSearchKey)).toEqual(activeResponse); + expect(queryClient.getQueryState(threadSearchKey)?.isInvalidated).toBe( + false, + ); + // One trailing read is enough: nothing changed while it ran. + await vi.advanceTimersByTimeAsync(2_000); + expect(searchQueryFn).toHaveBeenCalledTimes(2); + + unsubscribeSearch(); + effects.dispose(); + }); + it("marks the timeline of an unviewed thread stale without scheduling a refetch", async () => { vi.useFakeTimers(); const { effects, queryClient } = createRealtimeEffectsTestContext(); @@ -2082,6 +2185,113 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("throttles the metadata-less status fallback to one active refetch per second", async () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const sidebarNavigationKey = sidebarNavigationQueryKey(); + const sidebarQueryFn = vi.fn(async () => ({ + projects: [{ threads: [{ id: "thr_1", status: "idle" }] }], + personalProject: { threads: [] }, + })); + const observer = new QueryObserver(queryClient, { + queryKey: sidebarNavigationKey, + queryFn: sidebarQueryFn, + staleTime: Infinity, + }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + expect(sidebarQueryFn).toHaveBeenCalledTimes(1); + + const emitBareStatusChange = () => { + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { projectId: "project-1" }, + changes: ["status-changed"], + }); + }; + + // The first bare push after a quiet period still refetches immediately. + emitBareStatusChange(); + await vi.advanceTimersByTimeAsync(0); + expect(sidebarQueryFn).toHaveBeenCalledTimes(2); + + // A second push inside the same second coalesces instead of fanning out… + await vi.advanceTimersByTimeAsync(100); + emitBareStatusChange(); + await vi.advanceTimersByTimeAsync(100); + expect(sidebarQueryFn).toHaveBeenCalledTimes(2); + // …while the row still goes stale immediately for the next mount. + expect(queryClient.getQueryState(sidebarNavigationKey)?.isInvalidated).toBe( + true, + ); + + // The coalesced change lands as one trailing refetch at the 1s boundary. + await vi.advanceTimersByTimeAsync(1_000); + expect(sidebarQueryFn).toHaveBeenCalledTimes(3); + + unsubscribe(); + effects.dispose(); + }); + + it("refetches a project list and its forks list once each for a bare status change", async () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const projectListKey = threadListQueryKey({ + archived: false, + projectId: "project-1", + }); + // The forks row's filters extend the project list's, so a partial key + // match on the project list key also selects the forks query. + const forksListKey = threadListQueryKey({ + archived: false, + originKind: "fork", + projectId: "project-1", + sourceThreadId: "thr_1", + }); + const createListQueryFn = () => + vi.fn( + () => + new Promise((resolve) => { + setTimeout(() => resolve([]), 20); + }), + ); + const projectQueryFn = createListQueryFn(); + const forksQueryFn = createListQueryFn(); + const projectObserver = new QueryObserver(queryClient, { + queryKey: projectListKey, + queryFn: projectQueryFn, + staleTime: Infinity, + }); + const forksObserver = new QueryObserver(queryClient, { + queryKey: forksListKey, + queryFn: forksQueryFn, + staleTime: Infinity, + }); + const unsubscribeProject = projectObserver.subscribe(() => {}); + const unsubscribeForks = forksObserver.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(20); + expect(projectQueryFn).toHaveBeenCalledTimes(1); + expect(forksQueryFn).toHaveBeenCalledTimes(1); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_2", + metadata: { projectId: "project-1" }, + changes: ["status-changed"], + }); + await vi.advanceTimersByTimeAsync(1_500); + + expect(projectQueryFn).toHaveBeenCalledTimes(2); + expect(forksQueryFn).toHaveBeenCalledTimes(2); + + unsubscribeProject(); + unsubscribeForks(); + effects.dispose(); + }); + it("refetches over a patched row when a bare status-changed arrives while visible", async () => { // Stop requests, command failures and host interruptions push the bare // kind. On the visible path status-changed never enters the debounce @@ -2446,6 +2656,17 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("keeps the fine-pointer cadence and widens it for coarse pointers", () => { + expect(resolveThreadInvalidationDebounce(false)).toEqual({ + debounceMs: 50, + maxWaitMs: 200, + }); + expect(resolveThreadInvalidationDebounce(true)).toEqual({ + debounceMs: 150, + maxWaitMs: 400, + }); + }); + it("applies the reconnect watermark from the connected event", () => { const { effects, queryClient } = createRealtimeEffectsTestContext(); const disconnectedAt = Date.now(); diff --git a/apps/app/src/hooks/realtime-cache-effects.ts b/apps/app/src/hooks/realtime-cache-effects.ts index 215748164a..5e13dc4eff 100644 --- a/apps/app/src/hooks/realtime-cache-effects.ts +++ b/apps/app/src/hooks/realtime-cache-effects.ts @@ -34,8 +34,40 @@ import { partitionThreadChangesByFlushPriority, } from "./cache-owners/realtime-cache-registry"; -const INVALIDATION_DEBOUNCE_MS = 50; -const INVALIDATION_MAX_WAIT_MS = 200; +interface ThreadInvalidationDebounce { + debounceMs: number; + maxWaitMs: number; +} + +/** + * Streaming publishes arrive up to every 50 ms per thread, and each flush + * reconciles the whole unwindowed timeline/list state. Desktops absorb the + * 50/200 cadence (up to 20 reconciles/s); on coarse-pointer (touch) devices + * the same cadence competes with scroll and input handling on a phone core, + * so the window widens to 150/400 — still well inside perceived-live + * territory. Exported for tests; production reads the pointer class once at + * module init (it does not change mid-session). + */ +export function resolveThreadInvalidationDebounce( + isCoarsePointer: boolean, +): ThreadInvalidationDebounce { + return isCoarsePointer + ? { debounceMs: 150, maxWaitMs: 400 } + : { debounceMs: 50, maxWaitMs: 200 }; +} + +function detectCoarsePointer(): boolean { + return ( + typeof window !== "undefined" && + typeof window.matchMedia === "function" && + window.matchMedia("(pointer: coarse)").matches + ); +} + +const { + debounceMs: INVALIDATION_DEBOUNCE_MS, + maxWaitMs: INVALIDATION_MAX_WAIT_MS, +} = resolveThreadInvalidationDebounce(detectCoarsePointer()); const ENVIRONMENT_INVALIDATION_DEBOUNCE_MS = 250; const ENVIRONMENT_INVALIDATION_MAX_WAIT_MS = 500; diff --git a/apps/app/src/hooks/useSecondTick.test.ts b/apps/app/src/hooks/useSecondTick.test.ts new file mode 100644 index 0000000000..edc8ac3383 --- /dev/null +++ b/apps/app/src/hooks/useSecondTick.test.ts @@ -0,0 +1,71 @@ +// @vitest-environment jsdom + +import { act, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useSecondTick } from "./useSecondTick"; + +function setDocumentVisibility(state: "hidden" | "visible"): void { + Object.defineProperty(document, "visibilityState", { + configurable: true, + value: state, + }); + document.dispatchEvent(new Event("visibilitychange")); +} + +describe("useSecondTick", () => { + afterEach(() => { + vi.useRealTimers(); + Reflect.deleteProperty(document, "visibilityState"); + }); + + it("pauses the shared ticker while hidden and jumps to now on resume", () => { + vi.useFakeTimers(); + const { result, unmount } = renderHook(() => useSecondTick()); + const initial = result.current; + + act(() => { + vi.advanceTimersByTime(1_000); + }); + expect(result.current).toBe(initial + 1_000); + + // Hidden: the interval stops entirely — no timer wakes a suspended phone + // to re-render durations nothing can see. + act(() => { + setDocumentVisibility("hidden"); + }); + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(result.current).toBe(initial + 1_000); + + // Visible again: one immediate tick jumps durations to current truth + // instead of waiting out the next second, then the cadence resumes. + act(() => { + setDocumentVisibility("visible"); + }); + expect(result.current).toBe(initial + 6_000); + + act(() => { + vi.advanceTimersByTime(1_000); + }); + expect(result.current).toBe(initial + 7_000); + + unmount(); + }); + + it("shares one interval across subscribers and stops with the last one", () => { + vi.useFakeTimers(); + const first = renderHook(() => useSecondTick()); + const second = renderHook(() => useSecondTick()); + + act(() => { + vi.advanceTimersByTime(1_000); + }); + // One shared tick value, not two phase-shifted timers. + expect(first.result.current).toBe(second.result.current); + + first.unmount(); + second.unmount(); + expect(vi.getTimerCount()).toBe(0); + }); +}); diff --git a/apps/app/src/hooks/useSecondTick.ts b/apps/app/src/hooks/useSecondTick.ts index e96c0b3fe0..aa6c1003e4 100644 --- a/apps/app/src/hooks/useSecondTick.ts +++ b/apps/app/src/hooks/useSecondTick.ts @@ -1,4 +1,8 @@ import { useSyncExternalStore } from "react"; +import { + isDocumentVisible, + subscribeToDocumentVisibility, +} from "@/lib/document-visibility"; /** * One 1 Hz ticker shared by every live-duration label. Each label used to own @@ -6,27 +10,61 @@ import { useSyncExternalStore } from "react"; * mounted that is many timers firing at slightly different phases, each a * separate render. One interval, one notification per second, and it stops * when the last subscriber leaves. + * + * The interval also stops while the document is hidden: nothing it drives can + * be seen, and on phones the pending timer only queues work for the resume. + * The first tick after becoming visible fires immediately so durations jump + * to the current truth instead of waiting out the next second. */ const listeners = new Set<() => void>(); let lastTickMs = 0; let intervalId: ReturnType | null = null; +let unsubscribeVisibility: (() => void) | null = null; function tick(): void { lastTickMs = Date.now(); for (const listener of listeners) listener(); } +function startInterval(): void { + if (intervalId === null && isDocumentVisible()) { + intervalId = setInterval(tick, 1_000); + } +} + +function stopInterval(): void { + if (intervalId !== null) { + clearInterval(intervalId); + intervalId = null; + } +} + +function handleVisibilityChange(): void { + if (!isDocumentVisible()) { + stopInterval(); + return; + } + if (listeners.size > 0 && intervalId === null) { + tick(); + startInterval(); + } +} + function subscribe(listener: () => void): () => void { if (listeners.size === 0) { lastTickMs = Date.now(); - intervalId = setInterval(tick, 1_000); + unsubscribeVisibility = subscribeToDocumentVisibility( + handleVisibilityChange, + ); + startInterval(); } listeners.add(listener); return () => { listeners.delete(listener); - if (listeners.size === 0 && intervalId !== null) { - clearInterval(intervalId); - intervalId = null; + if (listeners.size === 0) { + stopInterval(); + unsubscribeVisibility?.(); + unsubscribeVisibility = null; } }; } diff --git a/apps/app/src/lib/query-client.test.ts b/apps/app/src/lib/query-client.test.ts index 66b1b187da..6d49d3e16f 100644 --- a/apps/app/src/lib/query-client.test.ts +++ b/apps/app/src/lib/query-client.test.ts @@ -194,6 +194,87 @@ describe("createAppQueryClient", () => { queryClient.clear(); }); + it("keeps the default reconnect refetch when no gate is configured", async () => { + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + showMutationErrorToasts: false, + }); + queryClient.mount(); + + const queryFn = vi.fn(() => Promise.resolve("data")); + const observer = new QueryObserver(queryClient, { + queryKey: ["reconnect-ungated"], + queryFn, + staleTime: 0, + }); + const unsubscribe = observer.subscribe(() => {}); + + await vi.waitFor(() => { + expect(observer.getCurrentResult().data).toBe("data"); + }); + + window.dispatchEvent(new Event("offline")); + window.dispatchEvent(new Event("online")); + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + + unsubscribe(); + queryClient.unmount(); + queryClient.clear(); + }); + + it("skips the default reconnect refetch while the gate reports realtime coverage", async () => { + let realtimeConnected = true; + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + shouldRefetchOnWindowFocus: () => !realtimeConnected, + showMutationErrorToasts: false, + }); + queryClient.mount(); + + const queryFn = vi.fn(() => Promise.resolve("data")); + const observer = new QueryObserver(queryClient, { + queryKey: ["reconnect-gated"], + queryFn, + // Instantly stale so a permitted reconnect refetch always fires. + staleTime: 0, + }); + const unsubscribe = observer.subscribe(() => {}); + + await vi.waitFor(() => { + expect(observer.getCurrentResult().data).toBe("data"); + }); + expect(queryFn).toHaveBeenCalledTimes(1); + + // Connected: realtime owns freshness, so the browser `online` blip that + // mobile Safari fires around suspensions must not refetch. + window.dispatchEvent(new Event("offline")); + window.dispatchEvent(new Event("online")); + await Promise.resolve(); + expect(queryFn).toHaveBeenCalledTimes(1); + + // Coverage lost: the reconnect refetch is the freshness fallback again. + realtimeConnected = false; + window.dispatchEvent(new Event("offline")); + window.dispatchEvent(new Event("online")); + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + + unsubscribe(); + queryClient.unmount(); + queryClient.clear(); + }); + it("resumes a suspend-cancelled fetch that no focus refetch would restart", async () => { const queryClient = createAppQueryClient({ defaultOptions: { diff --git a/apps/app/src/lib/query-client.ts b/apps/app/src/lib/query-client.ts index 5f7c905576..e7eeb00063 100644 --- a/apps/app/src/lib/query-client.ts +++ b/apps/app/src/lib/query-client.ts @@ -18,14 +18,16 @@ interface CreateAppQueryClientOptions { defaultOptions?: QueryClientConfig["defaultOptions"]; showMutationErrorToasts?: boolean; /** - * Gate for the default focus refetch. Focus refetch is the freshness - * fallback for when realtime coverage is lost; while the socket is - * connected, change events keep the cache correct and the reconnect - * watermark repairs any gap, so a focus event (every phone unlock and - * app switch) must not refetch every active query on top of that wave. - * Defaults to always refetching. A `defaultOptions.queries.refetchOnWindowFocus` - * passed alongside this gate wins over it (caller defaults are spread last), - * so pass one or the other. + * Gate for the default focus and reconnect refetches. Both are the + * freshness fallback for when realtime coverage is lost; while the socket + * is connected, change events keep the cache correct and the reconnect + * watermark repairs any gap, so neither a focus event (every phone unlock + * and app switch) nor a browser `online` event (mobile Safari re-fires it + * around the same suspensions) must refetch every active query on top of + * that wave. Defaults to always refetching. A + * `defaultOptions.queries.refetchOnWindowFocus`/`refetchOnReconnect` + * passed alongside this gate wins over it (caller defaults are spread + * last), so pass one or the other. */ shouldRefetchOnWindowFocus?: () => boolean; } @@ -149,6 +151,10 @@ export function createAppQueryClient( shouldRefetchOnWindowFocus === undefined ? true : () => shouldRefetchOnWindowFocus(), + refetchOnReconnect: + shouldRefetchOnWindowFocus === undefined + ? true + : () => shouldRefetchOnWindowFocus(), retry: shouldRetryTransientReadQuery, retryDelay: TRANSIENT_READ_RETRY_DELAY_MS, ...defaultOptions?.queries, diff --git a/apps/app/src/lib/shared-resize-observer.test.tsx b/apps/app/src/lib/shared-resize-observer.test.tsx new file mode 100644 index 0000000000..6492470593 --- /dev/null +++ b/apps/app/src/lib/shared-resize-observer.test.tsx @@ -0,0 +1,192 @@ +// @vitest-environment jsdom + +import { act, cleanup, render } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { ExpandablePanel } from "@/components/ui/disclosure"; +import { observeSharedResize } from "./shared-resize-observer"; + +class ResizeObserverStub implements ResizeObserver { + static instances: ResizeObserverStub[] = []; + + readonly observedTargets: Element[] = []; + + constructor(readonly callback: ResizeObserverCallback) { + ResizeObserverStub.instances.push(this); + } + + observe: ResizeObserver["observe"] = vi.fn((target: Element) => { + this.observedTargets.push(target); + }); + unobserve: ResizeObserver["unobserve"] = vi.fn(); + disconnect: ResizeObserver["disconnect"] = vi.fn(); +} + +afterEach(() => { + ResizeObserverStub.instances.length = 0; + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +function lastObserver(): ResizeObserverStub { + const observer = ResizeObserverStub.instances.at(-1); + if (!observer) { + throw new Error("No ResizeObserver was installed"); + } + return observer; +} + +function makeEntry(target: Element, blockSize: number): ResizeObserverEntry { + return { + target, + contentRect: new DOMRect(0, 0, 200, blockSize), + borderBoxSize: [{ blockSize, inlineSize: 200 }], + contentBoxSize: [{ blockSize, inlineSize: 200 }], + devicePixelContentBoxSize: [{ blockSize, inlineSize: 200 }], + }; +} + +describe("observeSharedResize", () => { + it("runs every registration's read before any write within a batch", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const order: string[] = []; + const first = document.createElement("div"); + const second = document.createElement("div"); + const unobserveFirst = observeSharedResize(first, { + read: () => { + order.push("read:first"); + return "first"; + }, + write: (value) => order.push(`write:${value}`), + }); + const unobserveSecond = observeSharedResize(second, { + read: () => { + order.push("read:second"); + return "second"; + }, + write: (value) => order.push(`write:${value}`), + }); + + // Two registrations, one observer: the whole point of sharing. + expect(ResizeObserverStub.instances).toHaveLength(1); + lastObserver().callback( + [makeEntry(first, 10), makeEntry(second, 20)], + lastObserver(), + ); + + expect(order).toEqual([ + "read:first", + "read:second", + "write:first", + "write:second", + ]); + unobserveFirst(); + unobserveSecond(); + }); + + it("re-syncs every registration when a synthetic batch carries no entries", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const order: string[] = []; + const seenEntries: (ResizeObserverEntry | undefined)[] = []; + const targets = [document.createElement("div"), document.createElement("div")]; + const unobservers = targets.map((target, index) => + observeSharedResize(target, { + read: (entry) => { + seenEntries.push(entry); + order.push(`read:${index}`); + return index; + }, + write: (value) => order.push(`write:${value}`), + }), + ); + + lastObserver().callback([], lastObserver()); + + // No entries means no target information: every registration re-syncs + // from live layout, still phased. + expect(order).toEqual(["read:0", "read:1", "write:0", "write:1"]); + expect(seenEntries).toEqual([undefined, undefined]); + for (const unobserve of unobservers) unobserve(); + }); + + it("releases the shared observer once the last registration leaves", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const target = document.createElement("div"); + const phases = { read: () => null, write: () => {} }; + + const unobserve = observeSharedResize(target, phases); + const observer = lastObserver(); + expect(observer.observe).toHaveBeenCalledWith(target); + + unobserve(); + expect(observer.unobserve).toHaveBeenCalledWith(target); + expect(observer.disconnect).toHaveBeenCalled(); + + // The next registration installs a fresh observer, so per-test + // `ResizeObserver` stubs take effect. + const unobserveAgain = observeSharedResize(target, phases); + expect(ResizeObserverStub.instances).toHaveLength(2); + unobserveAgain(); + }); +}); + +describe("ExpandablePanel on the shared observer", () => { + function renderTwoPanels() { + return render( + <> + First summary} + > + First body + + Second summary} + > + Second body + + , + ); + } + + it("mounts many panels onto one observer and sizes each from its own entry", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const view = renderTwoPanels(); + + // Two panels, one shared observer, one observation per panel body. + expect(ResizeObserverStub.instances).toHaveLength(1); + expect(lastObserver().observedTargets).toHaveLength(2); + + const regionOf = (text: string) => { + const region = + view.getByText(text).parentElement?.parentElement?.parentElement; + if (!region) { + throw new Error("Panel body region was not rendered"); + } + return region; + }; + const firstRegion = regionOf("First body"); + const secondRegion = regionOf("Second body"); + const [firstTarget, secondTarget] = lastObserver().observedTargets; + if (!firstTarget || !secondTarget) { + throw new Error("Panel bodies were not observed"); + } + + // One batch resizing both panels sizes each region from its own entry's + // border box — no per-panel layout read. + act(() => { + lastObserver().callback( + [makeEntry(firstTarget, 40), makeEntry(secondTarget, 60)], + lastObserver(), + ); + }); + + expect(firstRegion.style.height).toBe("40px"); + expect(secondRegion.style.height).toBe("60px"); + }); +}); diff --git a/apps/app/src/lib/shared-resize-observer.ts b/apps/app/src/lib/shared-resize-observer.ts new file mode 100644 index 0000000000..f0c0f21707 --- /dev/null +++ b/apps/app/src/lib/shared-resize-observer.ts @@ -0,0 +1,143 @@ +/** + * One module-level ResizeObserver shared by every registered element, with + * each delivery dispatched in two phases: every registration's `read` runs + * before any registration's `write`. + * + * Per-component observers whose callbacks interleave a layout read with a + * style write defeat the browser's batching: when one event resizes N + * observed elements at once (viewport resize, iOS keyboard, font swap), each + * callback's read forces layout against the previous callback's write — N + * synchronous layout passes over the document. Phasing the shared batch + * bounds that at one forced layout no matter how many elements resized. + * + * Same registry shape as `conversation-message-overflow.tsx`'s shared + * overflow observer; this module generalizes it to arbitrary read/write + * pairs and is the sanctioned pattern for per-row measurement. + */ + +export interface SharedResizePhases { + /** + * Gather everything `write` needs, preferring the entry's already-measured + * boxes over live layout reads. Runs with `undefined` when the dispatch + * carries no entry for the target (a broadcast re-sync) — read live layout + * (`offsetHeight`) then. Must not write styles. + */ + read: (entry: ResizeObserverEntry | undefined) => T; + /** Apply the value `read` produced. Must not read layout. */ + write: (value: T) => void; +} + +/** + * Type-erased registration: `read` closes over its typed value by returning + * the matching `write` as a thunk, so the registry needs no generics. + */ +interface RegisteredPhases { + read: (entry: ResizeObserverEntry | undefined) => () => void; +} + +interface PhaseDispatch { + registration: RegisteredPhases; + entry: ResizeObserverEntry | undefined; +} + +const phasesByTarget = new Map>(); +let sharedResizeObserver: ResizeObserver | null = null; + +function collectDispatches( + entries: readonly ResizeObserverEntry[], +): PhaseDispatch[] { + if (entries.length === 0) { + // The platform always delivers at least one entry. An empty batch only + // comes from synthetic dispatch (test stubs, polyfills) and carries no + // target information, so conservatively re-sync every registration from + // live layout. + return [...phasesByTarget.values()].flatMap((registrations) => + [...registrations].map((registration) => ({ + registration, + entry: undefined, + })), + ); + } + const dispatches: PhaseDispatch[] = []; + for (const entry of entries) { + for (const registration of phasesByTarget.get(entry.target) ?? []) { + dispatches.push({ registration, entry }); + } + } + return dispatches; +} + +function dispatchPhased(dispatches: readonly PhaseDispatch[]): void { + // Complete every read before any write can dirty layout for the next one. + const writes = dispatches.map(({ registration, entry }) => + registration.read(entry), + ); + for (const write of writes) { + write(); + } +} + +function getSharedResizeObserver(): ResizeObserver { + sharedResizeObserver ??= new ResizeObserver((entries) => { + dispatchPhased(collectDispatches(entries)); + }); + return sharedResizeObserver; +} + +/** + * Observe `target` on the shared observer. Returns the unregister function; + * the last unregistration for a target unobserves it, and the last overall + * releases the observer entirely (so per-test `ResizeObserver` stubs take + * effect on the next registration). + */ +export function observeSharedResize( + target: Element, + phases: SharedResizePhases, +): () => void { + const registration: RegisteredPhases = { + read: (entry) => { + const value = phases.read(entry); + return () => phases.write(value); + }, + }; + let registrations = phasesByTarget.get(target); + const isFirstForTarget = registrations === undefined; + if (registrations === undefined) { + registrations = new Set(); + phasesByTarget.set(target, registrations); + } + registrations.add(registration); + if (isFirstForTarget) { + // Register before observing: the initial observation can deliver + // synchronously in some environments and must reach this registration. + getSharedResizeObserver().observe(target); + } + + return () => { + const currentRegistrations = phasesByTarget.get(target); + currentRegistrations?.delete(registration); + if (currentRegistrations?.size === 0) { + phasesByTarget.delete(target); + sharedResizeObserver?.unobserve?.(target); + if (phasesByTarget.size === 0) { + sharedResizeObserver?.disconnect?.(); + sharedResizeObserver = null; + } + } + }; +} + +/** + * Border-box block size carried by an entry — `offsetHeight`'s metric without + * the layout read (the observer already measured this frame). The two must + * agree wherever an observer path and a direct path size the same element, or + * a padded box would get clipped by a content-box height. Returns `undefined` + * when the entry carries no usable box: entries cross a platform boundary and + * synthetic ones (test doubles, polyfills) omit boxes the spec guarantees — + * fall back to a live layout read then. + */ +export function observedBorderBoxBlockSize( + entry: ResizeObserverEntry, +): number | undefined { + return entry.borderBoxSize?.[0]?.blockSize ?? entry.contentRect?.height; +} diff --git a/apps/app/src/main.tsx b/apps/app/src/main.tsx index 4d1c8fd865..f28358c4db 100644 --- a/apps/app/src/main.tsx +++ b/apps/app/src/main.tsx @@ -31,8 +31,9 @@ Error.stackTraceLimit = 50; const queryClient = createAppQueryClient({ // While the realtime socket is connected, change events and the reconnect - // watermark own cache freshness; a focus refetch on top would re-request - // every active query on each phone unlock and app switch. + // watermark own cache freshness; a focus or browser-online refetch on top + // would re-request every active query on each phone unlock, app switch, + // and mobile-Safari `online` blip. shouldRefetchOnWindowFocus: () => wsManager.getConnectionState() !== "connected", }); diff --git a/apps/app/src/precompress-app-dist.test.ts b/apps/app/src/precompress-app-dist.test.ts index b661a1f6bd..7343760d4d 100644 --- a/apps/app/src/precompress-app-dist.test.ts +++ b/apps/app/src/precompress-app-dist.test.ts @@ -28,10 +28,14 @@ describe("app asset precompression", () => { const distDir = await mkdtemp(resolve(tmpdir(), "bb-precompress-test-")); const compressibleBody = Buffer.from("compressible bb asset\n".repeat(400)); const assetPath = resolve(distDir, "app.js"); + // The document itself: without a sidecar every cold navigation on the + // relayed mobile path ships the shell uncompressed through the tunnel. + const documentPath = resolve(distDir, "index.html"); const smallPath = resolve(distDir, "small.js"); const binaryPath = resolve(distDir, "image.png"); await Promise.all([ writeFile(assetPath, compressibleBody), + writeFile(documentPath, compressibleBody), writeFile(smallPath, "small"), writeFile(binaryPath, compressibleBody), ]); @@ -41,13 +45,16 @@ describe("app asset precompression", () => { distDir, ]); - expect(stdout).toContain("precompressed 1 files (1 br, 1 gzip)"); + expect(stdout).toContain("precompressed 2 files (2 br, 2 gzip)"); await expect( decompressBrotli(await readFile(`${assetPath}.br`)), ).resolves.toEqual(compressibleBody); await expect( decompressGzip(await readFile(`${assetPath}.gz`)), ).resolves.toEqual(compressibleBody); + await expect( + decompressBrotli(await readFile(`${documentPath}.br`)), + ).resolves.toEqual(compressibleBody); await expect(pathExists(`${smallPath}.br`)).resolves.toBe(false); await expect(pathExists(`${binaryPath}.br`)).resolves.toBe(false); }); diff --git a/apps/app/src/views/RootComposeMobileRecents.tsx b/apps/app/src/views/RootComposeMobileRecents.tsx index 1ad8475780..881c9c53f9 100644 --- a/apps/app/src/views/RootComposeMobileRecents.tsx +++ b/apps/app/src/views/RootComposeMobileRecents.tsx @@ -1,6 +1,6 @@ import { useMemo } from "react"; -import { Link } from "react-router-dom"; import type { ThreadListEntry } from "@bb/domain"; +import { RouteAnchor } from "@/components/ui/app-route-anchor"; import { ThreadStatusGlyph } from "@/components/sidebar/ThreadRow"; import { SIDEBAR_WORKING_STATUS_COLOR_CLASS } from "@/components/sidebar/sidebarRowClasses"; import { CHROME_SECTION_LABEL_CLASS } from "@bb/shared-ui/chrome-style-tokens"; @@ -117,8 +117,11 @@ function MobileRecentThreadRow({ ); return (
  • - - +
    • ); } diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx index 96314a8f48..0e87c509a8 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx @@ -783,6 +783,51 @@ describe("SplitThreadArea", () => { await waitFor(() => expect(hiddenScroller.scrollTop).toBe(0)); }); + it("stops the restore loop once positions settle instead of burning 30 frames", async () => { + renderSplitArea({ + path: threadPath("thr-a"), + layout: twoPaneLayout("pane-1"), + }); + const hiddenScroller = screen.getByTestId("scroll-thr-b"); + hiddenScroller.scrollTop = 12; + fireEvent.scroll(hiddenScroller); + + let scrollTopValue = 12; + const writes: number[] = []; + Object.defineProperty(hiddenScroller, "scrollTop", { + configurable: true, + get: () => scrollTopValue, + set: (value: number) => { + writes.push(value); + scrollTopValue = value; + }, + }); + + // Maximize: the tracked element already sits at its saved offset, so the + // pre-paint restore and the first frame find nothing to correct and the + // loop must end without a single scroll write (each write would force + // layout every frame for half a second). + fireEvent.click(screen.getByTestId("maximize-thr-a")); + await new Promise((resolve) => setTimeout(resolve, 600)); + expect(writes).toHaveLength(0); + + // Restore, with the scroller reporting 0 on every read — an adversary + // that keeps normalizing the position. The loop corrects before paint and + // on each frame, but gives up at the frame cap instead of running all 30. + Object.defineProperty(hiddenScroller, "scrollTop", { + configurable: true, + get: () => 0, + set: (value: number) => { + writes.push(value); + }, + }); + fireEvent.click(screen.getByTestId("maximize-thr-a")); + await new Promise((resolve) => setTimeout(resolve, 600)); + expect(writes.length).toBeGreaterThan(0); + // Pre-paint restore + at most 5 frames. + expect(writes.length).toBeLessThanOrEqual(6); + }); + it("toggles the focused pane through the discoverable app command", async () => { const store = renderSplitArea({ path: threadPath("thr-b"), diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.tsx index 8d0c53b721..e6e122ca8d 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.tsx @@ -215,30 +215,44 @@ function usePreservedSplitScrollPositions(maximizedPaneId: string | null) { } previousMaximizedPaneIdRef.current = maximizedPaneId; - const restore = () => { + /** Reapplies saved positions; true when any element needed correction. */ + const restore = (): boolean => { const workspace = workspaceRef.current; + let corrected = false; for (const [element, position] of positionsRef.current) { if (workspace === null || !workspace.contains(element)) { positionsRef.current.delete(element); continue; } + if ( + element.scrollLeft === position.left && + element.scrollTop === position.top + ) { + continue; + } element.scrollLeft = position.left; element.scrollTop = position.top; + corrected = true; } + return corrected; }; // Restore before paint, then briefly across animation frames so passive // timeline effects, virtualization, and browser scroll anchoring cannot - // overwrite the saved position while pane visibility settles. + // overwrite the saved position while pane visibility settles. Each frame + // forces layout on every tracked scroller, so the loop ends after the + // first frame with nothing to correct; the frame cap bounds the + // pathological case where something keeps fighting the restore. restore(); let frame: number | null = null; - let framesRemaining = 30; + let framesRemaining = 5; const restoreUntilSettled = () => { - restore(); + const corrected = restore(); framesRemaining -= 1; - if (framesRemaining > 0) { - frame = window.requestAnimationFrame(restoreUntilSettled); - } + frame = + corrected && framesRemaining > 0 + ? window.requestAnimationFrame(restoreUntilSettled) + : null; }; frame = window.requestAnimationFrame(restoreUntilSettled); return () => { diff --git a/apps/app/src/vite-font-preload.test.ts b/apps/app/src/vite-font-preload.test.ts index 0d6f401274..f1f2b9d631 100644 --- a/apps/app/src/vite-font-preload.test.ts +++ b/apps/app/src/vite-font-preload.test.ts @@ -1,5 +1,10 @@ +import { existsSync, readFileSync } from "node:fs"; +import { resolve } from "node:path"; import { describe, expect, it } from "vitest"; -import { resolveFontPreloadTags } from "../vite-font-preload.js"; +import { + reorderHeadForFirstPaint, + resolveFontPreloadTags, +} from "../vite-font-preload.js"; const bundle = [ "assets/index-rXrqkkAU.js", @@ -37,3 +42,156 @@ describe("resolveFontPreloadTags", () => { expect(resolveFontPreloadTags(["assets/index-abc.js"], "/")).toEqual([]); }); }); + +/** The head layout Vite emits: theme script, then entry + preloads + css. */ +const builtHtml = [ + "", + '', + '', + '', + '', + '', + "", +].join(""); + +describe("reorderHeadForFirstPaint", () => { + const fontTags = resolveFontPreloadTags(bundle, "/"); + + it("moves the stylesheet and font preload ahead of the script and preload block", () => { + const html = reorderHeadForFirstPaint(builtHtml, fontTags); + + const themeAt = html.indexOf("bb.theme"); + const fontAt = html.search(/]*as="font"/); + const stylesheetAt = html.search(/]*rel="stylesheet"/); + const entryAt = html.search(/"; + expect(reorderHeadForFirstPaint(bare, [])).toBe(bare); + }); + + it("refuses to move the stylesheet ahead of the pre-paint theme script", () => { + const themeless = builtHtml.replace("bb.theme", "bb.other"); + expect(() => reorderHeadForFirstPaint(themeless, fontTags)).toThrow( + /pre-paint theme script/, + ); + }); +}); + +/** + * The document Vite 8 hands post-order transforms for the real index.html: + * the source entry tag is lifted out of , and the built entry, its + * modulepreloads and the stylesheet are appended to in that order, + * one per indented line. Built from the committed source so the test runs + * without a build (dist/ is absent on the CI test runners) and still covers + * the real pre-paint theme script and Vite's whitespace around the tags. + */ +function viteEmittedIndexHtml(): string { + const source = readFileSync( + resolve(import.meta.dirname, "../index.html"), + "utf8", + ); + const sourceEntryTag = + /[ \t]*', + '', + '', + '', + ] + .map((tag) => ` ${tag}\n`) + .join(""); + return source + .replace(sourceEntryTag, "") + .replace(" ", `${injected} `); +} + +describe("reorderHeadForFirstPaint on the document Vite emits from index.html", () => { + it("front-loads the stylesheet and font preload, after the theme script and ahead of every script and modulepreload", () => { + const emitted = viteEmittedIndexHtml(); + const html = reorderHeadForFirstPaint( + emitted, + resolveFontPreloadTags(bundle, "/"), + ); + + const themeScriptAt = html.indexOf("bb.theme"); + const fontPreloadAt = html.search(/]*as="font"/); + const stylesheetAt = html.search(/]*rel="stylesheet"/); + const entryAt = html.search(/