From 2ca2f1342c187024780fadb8ce3bb59215726211 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 09:15:36 +0200 Subject: [PATCH 01/11] Navigate app routes at transition priority with a pending signal Every route tap ran navigateRef.current() bare inside the click's discrete event. Wrap it in useTransition's startTransition inside RouteNavigationProvider so the tap's urgent commit paints first, and expose isPending through a separate RouteNavigationPendingContext (the navigate context identity stays stable, so navigate consumers still never re-render per navigation). Replace the raw react-router in RootComposeMobileRecents with RouteAnchor so the mobile recents rows take the same path. New test proves ordering: the tap's commit shows pending with the old route still mounted, and the destination lands in a later transition commit. Co-Authored-By: Claude Fable 5 --- .../components/ui/app-route-anchor.test.tsx | 65 +++++++++++++++++++ .../src/components/ui/app-route-anchor.tsx | 47 +++++++++++--- .../src/views/RootComposeMobileRecents.tsx | 11 ++-- 3 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 apps/app/src/components/ui/app-route-anchor.test.tsx 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/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 (
  • - - +
    • ); } From 61b765bbed47b322e1672794ca07684d82cc80be Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 13:45:02 +0200 Subject: [PATCH 02/11] Defer expander bodies out of the toggle's click commit Expanding a collapsed ExpandablePanel materialized the whole body subtree inside the click's discrete commit (button.inline-flex stalls in the hang ledger). Drive the expandedBody memo from useDeferredValue(isExpanded) so the caret/header flip paints in the tap's first frame and the body mounts in a follow-up interruptible commit. Header state, the closing-body ref retention, and the layout-animation signal stay on the urgent value; rows that mount already expanded still render their body immediately (useDeferredValue returns the live value on first render). New test fails before this change: with flushSync standing in for the tap's urgent flush, the body used to be mounted in that same commit. Co-Authored-By: Claude Fable 5 --- .../app/src/components/ui/disclosure.test.tsx | 66 ++++++++++++++++++- apps/app/src/components/ui/disclosure.tsx | 13 +++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/apps/app/src/components/ui/disclosure.test.tsx b/apps/app/src/components/ui/disclosure.test.tsx index 64df40de0d..69844dd0fb 100644 --- a/apps/app/src/components/ui/disclosure.test.tsx +++ b/apps/app/src/components/ui/disclosure.test.tsx @@ -1,6 +1,8 @@ // @vitest-environment jsdom -import { act, cleanup, render } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { useState } from "react"; +import { flushSync } from "react-dom"; import { afterEach, describe, expect, it, vi } from "vitest"; import { ExpandablePanel } from "./disclosure"; @@ -21,6 +23,7 @@ afterEach(() => { cleanup(); vi.unstubAllGlobals(); vi.restoreAllMocks(); + vi.useRealTimers(); }); function renderPanel(isExpanded: boolean) { @@ -85,3 +88,64 @@ describe("ExpandablePanel body height", () => { expect(region.style.transitionDuration).toBe("0s"); }); }); + +/** A toggleable panel driven by its own header, like a timeline tool row. */ +function TogglablePanel() { + const [isExpanded, setIsExpanded] = useState(false); + return ( + setIsExpanded((expanded) => !expanded)} + > + 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); + expect(screen.getByText("Expanded body")).toBeTruthy(); + + fireEvent.click(header); + + // The collapse animates from the still-rendered subtree: the body must + // stay mounted for the 200ms transition, then unmount. + expect(header.getAttribute("aria-expanded")).toBe("false"); + expect(screen.getByText("Expanded body")).toBeTruthy(); + + act(() => { + vi.advanceTimersByTime(200); + }); + expect(screen.queryByText("Expanded body")).toBeNull(); + }); +}); diff --git a/apps/app/src/components/ui/disclosure.tsx b/apps/app/src/components/ui/disclosure.tsx index e052238e02..1de25ef77a 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, @@ -223,12 +224,20 @@ 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]); // Signal to AutoHeightContainer / HeightTransition wrappers that a // CSS-driven layout animation is in flight, so they snap their wrapper to From e0c2d3c6c40d23802425c19f835a4787976a8724 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 13:45:11 +0200 Subject: [PATCH 03/11] Make the mobile viewport handler idempotent and cheap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On every iOS visual-viewport tick (keyboard animation, URL-bar collapse, momentum settling) the shell hook forced a full-document layout via document.body.clientHeight and rewrote shell top/height plus the inherited --bb-shell-height, invalidating computed style for the whole app tree at animation cadence. - Skip all style writes when a pass recomputes the geometry it already applied, and make clearViewportOverride a no-op while nothing is set. - Cache the containing-block height; re-read it only on triggers that can resize the layout viewport (window resize, orientationchange, focusin) — never on visualViewport ticks, which move only the visual viewport. - Gate visualViewport scroll ticks on keyboard focus or an applied override: keyboard-less URL-bar pans need no compensation, while embedded-browser overrides (applied without a keyboard) keep tracking. The focusout fast-restore, the native-layout early-exit, the pinch-zoom guard, and rAF coalescing are preserved; programmatic focus (composer autofocus) still triggers one freshly measured pass, covered by a new test. Co-Authored-By: Claude Fable 5 --- .../useMobileVisualViewportHeight.test.tsx | 142 ++++++++++++++++++ .../layout/useMobileVisualViewportHeight.ts | 79 ++++++++-- 2 files changed, 209 insertions(+), 12 deletions(-) diff --git a/apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx b/apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx index 7ec3b26225..fad08417d8 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,136 @@ 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. + act(() => editor.focus()); + act(() => { + 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..7ebb4de039 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,27 @@ export function useMobileVisualViewportHeight( } animationFrame = window.requestAnimationFrame(updateHeight); }; + // For triggers that can resize the layout viewport itself: window resize, + // rotation, and an editor gaining focus (the keyboard that follows may + // resize the layout viewport on Android's resizes-content path). + 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 +172,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 +189,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); From 7b22677671829cf0002a4698474e4935eb98c5ad Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 13:47:16 +0200 Subject: [PATCH 04/11] Realize the mobile sidebar at transition priority When boot's idle pre-realization has not run yet, the sidebar trigger's click flushed the whole ProjectList/ThreadRow subtree synchronously before the slide's first frame could composite. Wrap realizeMobileSidebar() in React.startTransition so the tap's flush only writes the inline drag styles (the slide starts immediately) and the subtree mounts interruptibly during the settle window. The drag-style write order is unchanged, and the settle commit's render-phase latch still realizes the subtree synchronously if it somehow lands first. The other flushSync sites in this file run after the settle window (deferred open/close commits and the swipe settle paths), not in the tap's critical path, and are deliberately untouched. New test fails before this change: with flushSync standing in for the tap's urgent flush, the subtree used to be realized in that same flush. Co-Authored-By: Claude Fable 5 --- apps/app/src/components/ui/sidebar.test.tsx | 32 +++++++++++++++++++++ apps/app/src/components/ui/sidebar.tsx | 11 +++++-- 2 files changed, 41 insertions(+), 2 deletions(-) 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 acd4df98b3..1e36dacf98 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; From 28f6f596a64624dfc4ee9eb5af1c2de3331f6270 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 13:48:59 +0200 Subject: [PATCH 05/11] Early-exit the maximize restore loop The maximize/restore scroll-preservation loop unconditionally rewrote scrollLeft/scrollTop on every tracked element for 30 animation frames, forcing layout each frame for half a second after every toggle. Make restore() compare before writing and report whether anything needed correction, stop the rAF loop after the first frame with zero corrections, and cap the loop at 5 frames. The pre-paint initial restore() stays. New test fails before this change: the settled case saw 31 scroll writes (pre-paint + 30 frames); now it sees none, and an adversarial scroller that keeps normalizing to zero is corrected at most 6 times. Co-Authored-By: Claude Fable 5 --- .../thread-detail/SplitThreadArea.test.tsx | 45 +++++++++++++++++++ .../views/thread-detail/SplitThreadArea.tsx | 28 +++++++++--- 2 files changed, 66 insertions(+), 7 deletions(-) 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 () => { From e4dc82bfa75063d8a174b6b714678092dc6649c6 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 13:52:11 +0200 Subject: [PATCH 06/11] Take the resize cascade's geometry from the observer entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every timeline size change re-entered layout up to five times: the scroll body's ResizeObserver delivery did a live scrollHeight/clientHeight refresh, the observed frame's bottom restore read again, and each of the three rAF settle-tail frames forced another layout — at streaming cadence on an unwindowed tree. - bottom-anchored-scroll-body: refresh the cached max offset from the ResizeObserver's own box sizes when the delivery carries entries (the scroll port's content box + the content wrapper's border box), falling back to the live read for entry-less deliveries (test stubs). The observed frame's restoreBottomOnce keeps its deliberate live read; the settle-tail frames now reuse the cache, and a tail frame that corrected drift arms exactly one live verification read on the next frame. - height-transition: size the wrapper from the entry's borderBoxSize (the same border-box metric as the offsetHeight used by the mount and snap paths) instead of the content rect; non-observer paths keep offsetHeight. The scroll-preservation contract suite passes unmodified. New settle-tail tests count geometry reads per tail frame and cover the entry-derived cache; a height-transition test pins the border-box sizing. Co-Authored-By: Claude Fable 5 --- ...-anchored-scroll-body.settle-tail.test.tsx | 292 ++++++++++++++++++ .../ui/bottom-anchored-scroll-body.tsx | 164 +++++++--- .../components/ui/height-transition.test.tsx | 41 +++ .../src/components/ui/height-transition.tsx | 24 +- 4 files changed, 472 insertions(+), 49 deletions(-) create mode 100644 apps/app/src/components/ui/bottom-anchored-scroll-body.settle-tail.test.tsx 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..85cae0d2b9 --- /dev/null +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.settle-tail.test.tsx @@ -0,0 +1,292 @@ +// @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 }; +} + +function makeResizeEntry( + target: Element, + blockSize: number, +): ResizeObserverEntry { + const boxSize: ResizeObserverSize = { blockSize, inlineSize: 100 }; + return { + target, + contentRect: new DOMRect(0, 0, 100, blockSize), + borderBoxSize: [boxSize], + contentBoxSize: [boxSize], + devicePixelContentBoxSize: [boxSize], + }; +} + +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. + getLatestResizeObserver().trigger([ + makeResizeEntry(scrollArea, 100), + makeResizeEntry(scrollContent, 900), + ]); + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + + // The derived max offset (800) is what scroll classification runs on: + // 797 is within the 4px threshold, so this scroll re-attaches — still + // without a live read. + scrollArea.scrollTop = 797; + 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, 100), + makeResizeEntry(scrollContent, 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 7da8501fb8..82fefb5500 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx @@ -284,6 +284,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; @@ -321,6 +325,14 @@ 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 }); const [isAtBottom, setIsAtBottom] = useState(true); const initialScrollRestoreRowId = useMemo(() => { if (scrollAnchorThreadId === undefined) return null; @@ -359,6 +371,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. @@ -386,6 +399,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. @@ -401,13 +430,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 = @@ -415,7 +458,7 @@ export function BottomAnchoredScrollBody({ } }; restoreFrameRef.current = window.requestAnimationFrame(runQueuedRestore); - }, [restoreBottomOnce]); + }, [restoreBottomOnce, restoreBottomFromCacheOnce]); const scrollToBottom = useCallback(() => { const scrollArea = scrollAreaRef.current; @@ -805,48 +848,81 @@ 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; + 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 diff --git a/apps/app/src/components/ui/height-transition.test.tsx b/apps/app/src/components/ui/height-transition.test.tsx index c90caa4a73..9fdbfd8c51 100644 --- a/apps/app/src/components/ui/height-transition.test.tsx +++ b/apps/app/src/components/ui/height-transition.test.tsx @@ -74,7 +74,48 @@ describe("HeightTransition", () => { }); }); +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..81aede0797 100644 --- a/apps/app/src/components/ui/height-transition.tsx +++ b/apps/app/src/components/ui/height-transition.tsx @@ -141,6 +141,14 @@ 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. +function getObservedInnerHeight(entry: ResizeObserverEntry): number { + return entry.borderBoxSize[0]?.blockSize ?? entry.contentRect.height; +} + interface HeightTransitionProps { visible: boolean; children: ReactNode; @@ -171,7 +179,7 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { const observer = new ResizeObserver((entries) => { const entry = entries[0]; if (!entry) return; - const { width, height } = entry.contentRect; + const { width } = 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. @@ -179,10 +187,11 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { // scrollHeight, which the bottom-anchor sentinel then chases. const layoutAnimationActive = store.get(layoutAnimationInFlightCountAtom) > 0; - const snap = widthChanged || pendingVisibilitySnap || layoutAnimationActive; + const snap = + widthChanged || pendingVisibilitySnap || layoutAnimationActive; pendingVisibilitySnap = false; lastWidth = width; - const nextHeight = visible ? `${height}px` : "0px"; + const nextHeight = visible ? `${getObservedInnerHeight(entry)}px` : "0px"; applyHeight(wrapper, nextHeight, snap, snapState); }); observer.observe(inner); @@ -334,7 +343,7 @@ export function AutoHeightContainer({ const observer = new ResizeObserver((entries) => { const entry = entries[0]; if (!entry) return; - const { width, height } = entry.contentRect; + const { width } = 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. @@ -360,7 +369,12 @@ export function AutoHeightContainer({ deferInitialSettleComplete(); return; } - applyHeight(wrapper, `${height}px`, snap, snapState); + applyHeight( + wrapper, + `${getObservedInnerHeight(entry)}px`, + snap, + snapState, + ); deferInitialSettleComplete(); }); observer.observe(inner); From 22f41aba2ca1e760810847584078e150cfdbb0fb Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 14:00:24 +0200 Subject: [PATCH 07/11] Trim per-scroll-event work on coarse pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every scroll event wrote data-scrollbar-scrolling (matched only by desktop ::-webkit-scrollbar rules) — a pure style invalidation on touch — and each throttled scroll-anchor sample re-ran querySelectorAll over the scroll subtree at 10 Hz while the browser was busy scrolling. - Skip the transient-scrollbar attribute when (pointer: coarse) matches. - Cache the scroll-anchor row NodeList in a ref; the existing ResizeObserver invalidates it, and an end-connectivity check covers windowed row swaps that keep the content size constant. - Raise the scroll-anchor capture throttle to 250ms on coarse pointers — restore-on-return needs the resting position (always carried by the trailing write), not mid-flick samples. Co-Authored-By: Claude Fable 5 --- ...chored-scroll-body.coarse-pointer.test.tsx | 256 ++++++++++++++++++ .../ui/bottom-anchored-scroll-body.tsx | 61 ++++- 2 files changed, 307 insertions(+), 10 deletions(-) create mode 100644 apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx 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..0863cf478e --- /dev/null +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx @@ -0,0 +1,256 @@ +// @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 that differ by pointer type: the transient-scrollbar +// attribute (desktop-scrollbar-only CSS) is skipped on coarse pointers, 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("never writes the transient scrollbar attribute", () => { + stubMediaQueries(new Set(["(pointer: coarse)"])); + const { scrollArea } = renderTimeline("coarse-thread", ["row-a"]); + + fireEvent.scroll(scrollArea); + + // The attribute only feeds desktop ::-webkit-scrollbar rules; on touch it + // would be a per-scroll-event style invalidation with no visible effect. + expect(scrollArea.hasAttribute("data-scrollbar-scrolling")).toBe(false); + }); + + 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"]); + + // Settle the first capture (immediate or trailing, depending on where the + // faked clock started) so the throttle's lastWriteAt equals now. + fireEvent.scroll(scrollArea); + vi.runOnlyPendingTimers(); + 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.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx index 82fefb5500..eb61643786 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. @@ -189,9 +194,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 +282,7 @@ export function BottomAnchoredScrollBody({ scrollAnchorThreadId, }: BottomAnchoredScrollBodyProps) { const store = useStore(); + const isPointerCoarse = usePointerCoarse(); const scrollAreaRef = useRef(null); const scrollContentRef = useRef(null); const shouldStickToBottomRef = useRef(true); @@ -333,6 +339,12 @@ export function BottomAnchoredScrollBody({ scrollAreaClientHeight: number | null; scrollContentHeight: number | null; }>({ scrollAreaClientHeight: null, scrollContentHeight: null }); + // The row NodeList behind scroll-anchor capture, so throttled samples don't + // repeat a querySelectorAll over the scroll subtree. Row-set changes surface + // as content size changes, so the ResizeObserver invalidates it; the + // connectivity check in getScrollAnchorRowsCached guards the windowed + // timeline, where a row swap at a window edge can keep the size constant. + const scrollAnchorRowsRef = useRef | null>(null); const [isAtBottom, setIsAtBottom] = useState(true); const initialScrollRestoreRowId = useMemo(() => { if (scrollAnchorThreadId === undefined) return null; @@ -551,6 +563,21 @@ 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 = getScrollAnchorRows(scrollArea); + scrollAnchorRowsRef.current = 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 @@ -600,7 +627,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, { @@ -610,6 +640,7 @@ export function BottomAnchoredScrollBody({ }); }, [ + getScrollAnchorRowsCached, hasRecentUserScrollIntent, readMaxScrollOffset, refreshMaxScrollOffset, @@ -618,12 +649,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; @@ -635,8 +670,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 @@ -851,6 +886,8 @@ export function BottomAnchoredScrollBody({ 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 @@ -1006,6 +1043,12 @@ export function BottomAnchoredScrollBody({ }, SCROLLBAR_IDLE_DELAY_MS); handleScroll(); }; + // The transient-scrollbar attribute only feeds the desktop-only + // ::-webkit-scrollbar rules; on coarse pointers (overlay scrollbars) the + // write would be a pure per-scroll-event style invalidation. + const handleScrollEvent = isPointerCoarse + ? handleScroll + : handleScrollWithTransientScrollbar; let resizeObserver: ResizeObserver | undefined; if (typeof ResizeObserver !== "undefined") { @@ -1014,7 +1057,7 @@ export function BottomAnchoredScrollBody({ resizeObserver.observe(scrollContent); } - scrollArea.addEventListener("scroll", handleScrollWithTransientScrollbar, { + scrollArea.addEventListener("scroll", handleScrollEvent, { passive: true, }); scrollArea.addEventListener("wheel", markWheelScrollIntent, { @@ -1040,10 +1083,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); @@ -1062,6 +1102,7 @@ export function BottomAnchoredScrollBody({ endPointerScrollIntent, handleScroll, handleScrollAreaResize, + isPointerCoarse, markKeyboardScrollIntent, markTouchMoveScrollIntent, markTouchStartScrollIntent, From d1d20fe6f799048fa874ec96e1fda0e6252765d6 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 14:00:24 +0200 Subject: [PATCH 08/11] Clarify why focus-driven viewport passes remeasure the containing block The previous comment credited Android's resizes-content path, but the focusin listener only exists on iOS WebKit. The real reason: the pass that sizes the shell for the arriving keyboard must start from the real containing block, and focus changes are rare enough to afford the read. Co-Authored-By: Claude Fable 5 --- .../src/components/layout/useMobileVisualViewportHeight.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/app/src/components/layout/useMobileVisualViewportHeight.ts b/apps/app/src/components/layout/useMobileVisualViewportHeight.ts index 7ebb4de039..a987dee6bb 100644 --- a/apps/app/src/components/layout/useMobileVisualViewportHeight.ts +++ b/apps/app/src/components/layout/useMobileVisualViewportHeight.ts @@ -135,9 +135,10 @@ export function useMobileVisualViewportHeight( } animationFrame = window.requestAnimationFrame(updateHeight); }; - // For triggers that can resize the layout viewport itself: window resize, - // rotation, and an editor gaining focus (the keyboard that follows may - // resize the layout viewport on Android's resizes-content path). + // 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(); From 647268178c4fc1261acbbcb2a19a006f356564fe Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 22:21:36 +0200 Subject: [PATCH 09/11] Phase timeline height syncs through one shared ResizeObserver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every ExpandablePanel and HeightTransition/AutoHeightContainer installed its own ResizeObserver whose callback interleaved a layout read with a style write, so one width/height event (iOS keyboard, drawer, orientation, font swap) forced a synchronous layout pass per mounted row. The new src/lib/shared-resize-observer.ts registry runs on a single module-level observer and dispatches each batch in phases — every registration's read completes before any write — bounding a whole batch at one forced layout. Panel body sizing now comes from the entry's border box (offsetHeight's metric, no layout read), preserving the transitionDuration snap semantics, the deferred-body realization, and the borderBoxSize sizing the existing suites pin. Co-Authored-By: Claude Fable 5 --- apps/app/src/components/ui/disclosure.tsx | 41 +++- .../src/components/ui/height-transition.tsx | 143 +++++++------ .../src/lib/shared-resize-observer.test.tsx | 192 ++++++++++++++++++ apps/app/src/lib/shared-resize-observer.ts | 143 +++++++++++++ 4 files changed, 453 insertions(+), 66 deletions(-) create mode 100644 apps/app/src/lib/shared-resize-observer.test.tsx create mode 100644 apps/app/src/lib/shared-resize-observer.ts diff --git a/apps/app/src/components/ui/disclosure.tsx b/apps/app/src/components/ui/disclosure.tsx index 1de25ef77a..45d4027af3 100644 --- a/apps/app/src/components/ui/disclosure.tsx +++ b/apps/app/src/components/ui/disclosure.tsx @@ -9,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; @@ -165,22 +175,37 @@ 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(); + return observeSharedResize(target, { + read: readHeightSync, + write: writeHeightSync, + }); }, [collapsedContent, isExpanded, renderedBody]); return ( diff --git a/apps/app/src/components/ui/height-transition.tsx b/apps/app/src/components/ui/height-transition.tsx index 81aede0797..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. @@ -145,8 +149,15 @@ function cancelIntrinsicHeightRestore( // 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. -function getObservedInnerHeight(entry: ResizeObserverEntry): number { - return entry.borderBoxSize[0]?.blockSize ?? entry.contentRect.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 { @@ -176,25 +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 } = 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 ? `${getObservedInnerHeight(entry)}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 @@ -211,7 +231,7 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { const unsubscribeFromDocumentVisibility = subscribeToDocumentVisibility(onVisibility); return () => { - observer.disconnect(); + unobserveInner(); unsubscribeFromDocumentVisibility(); cleanupSnapState(wrapper, snapState); }; @@ -340,44 +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 } = 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, - `${getObservedInnerHeight(entry)}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 @@ -392,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/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; +} From b1b2497bba745ade17f8f3b5a3f9a4e3fd3f9b3f Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Tue, 25 Aug 2026 08:32:38 +0200 Subject: [PATCH 10/11] Halve the per-message action-bar ResizeObservers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every message mounted two width observers: one for the action row's slot and one for the enclosing [data-message-column], even though the mobile overflow branch renders a constant layout that never reads the slot width, and the column width is the same number for every top-level row. useMeasuredWidth gains an `enabled` option (hook order stable, no observer constructed when disabled); the overflow branch disables the slot observer; and the top-level TimelineRowsList measures its root once and shares it through MessageColumnWidthContext, so one observer serves every bar. Without a provider (stories, unit renders) or inside nested, narrower lists — which shadow the context with null — a bar measures its own column exactly as before. Desktop inline/overflow layout is pinned by the existing width-driven tests. Co-Authored-By: Claude Fable 5 --- .../thread/timeline/MessageActionBar.test.tsx | 79 +++++++++ .../thread/timeline/MessageActionBar.tsx | 58 +++++-- .../ThreadTimelineRows.actions.test.tsx | 83 ++++++++++ .../thread/timeline/ThreadTimelineRows.tsx | 151 ++++++++++-------- 4 files changed, 296 insertions(+), 75 deletions(-) diff --git a/apps/app/src/components/thread/timeline/MessageActionBar.test.tsx b/apps/app/src/components/thread/timeline/MessageActionBar.test.tsx index 6eb9086c74..1d910deca8 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,84 @@ 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("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..e93ed78a09 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,29 @@ 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]` spans the full list width, so + * per-bar column observers would all report the same number. The top-level + * `TimelineRowsList` measures its root once and provides it here. `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 @@ -478,9 +505,21 @@ 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, + }); + const columnWidth = + sharedColumnWidth === null ? ownColumnWidth : sharedColumnWidth.width; // Touch-only: the hidden actions revealed in place by the "⋯" trigger. const [expanded, setExpanded] = useState(false); const expandedRowRef = useRef(null); @@ -601,7 +640,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/ThreadTimelineRows.actions.test.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx index 092b1798f3..d1d4444d5c 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,86 @@ 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(); + }); +}); diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx index c0d1ae1fc4..4808f7209e 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,98 @@ 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 spans this list's full width, so the bars read this shared + // measurement (MessageColumnWidthContext) 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} - - ); - }} - /> -
    + }} + /> +
    +     ); } From e1d74128e637a1c47f7ed3dc2f3a3c2d4c3f0ff9 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Tue, 25 Aug 2026 08:33:37 +0200 Subject: [PATCH 11/11] Mark the shared selection pointer listeners passive The shared document pointerdown/pointerup/pointercancel handlers in SelectableMessageProse never call preventDefault, but without the passive flag the browser must still treat every tap as potentially blocking. Declare { passive: true } on the three pointer listeners and pin the flag with a test; removal matching is unaffected (only the capture flag participates), so the shared teardown behavior is unchanged. Co-Authored-By: Claude Fable 5 --- .../SelectableMessageProse.events.test.tsx | 20 +++++++++++++++++++ .../timeline/SelectableMessageProse.tsx | 14 ++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) 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);