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..a987dee6bb 100644 --- a/apps/app/src/components/layout/useMobileVisualViewportHeight.ts +++ b/apps/app/src/components/layout/useMobileVisualViewportHeight.ts @@ -55,7 +55,22 @@ export function useMobileVisualViewportHeight( if (!shell || !shellHeightRoot || !enabled || !visualViewport) return; let animationFrame: number | null = null; + // The override last written to the shell, or null while none is applied. + // Writing shell `top`/`height` and the inherited `--bb-shell-height` + // invalidates computed style for the whole app tree, and passes run at + // visual-viewport event cadence (keyboard animation, URL-bar collapse), + // so a pass that recomputes unchanged geometry must not write at all. + let appliedOverride: { top: number; height: number } | null = null; + // Reading `document.body.clientHeight` forces a full-document layout. The + // shell's containing block only changes when the layout viewport does, so + // cache the read and mark it stale only on triggers that can resize the + // layout viewport — never on visualViewport ticks, which move or resize + // only the visual viewport. + let shellContainingBlockHeight = 0; + let shellContainingBlockHeightStale = true; const clearViewportOverride = () => { + if (appliedOverride === null) return; + appliedOverride = null; shell.style.removeProperty("top"); shell.style.removeProperty("height"); shellHeightRoot.style.removeProperty("--bb-shell-height"); @@ -68,12 +83,15 @@ export function useMobileVisualViewportHeight( } const visualViewportHeight = Math.round(visualViewport.height); - // `documentElement.clientHeight` is the visible viewport height for the - // root element, even when that root's actual CSS box extends behind an - // Android in-app browser toolbar. The body inherits the root box and - // therefore exposes the containing-block height the app shell really - // receives. - const shellContainingBlockHeight = document.body.clientHeight; + if (shellContainingBlockHeightStale) { + // `documentElement.clientHeight` is the visible viewport height for the + // root element, even when that root's actual CSS box extends behind an + // Android in-app browser toolbar. The body inherits the root box and + // therefore exposes the containing-block height the app shell really + // receives. + shellContainingBlockHeight = document.body.clientHeight; + shellContainingBlockHeightStale = false; + } const hasVisualViewportPan = visualViewport.offsetTop > 1 || window.scrollY > 0; if ( @@ -91,7 +109,16 @@ export function useMobileVisualViewportHeight( // compensation below also handles a visual-viewport-only pan. window.scrollTo(0, 0); } - shell.style.top = `${getVisualViewportPageTop(visualViewport)}px`; + const shellTop = getVisualViewportPageTop(visualViewport); + if ( + appliedOverride !== null && + appliedOverride.top === shellTop && + appliedOverride.height === visualViewportHeight + ) { + return; + } + appliedOverride = { top: shellTop, height: visualViewportHeight }; + shell.style.top = `${shellTop}px`; shell.style.height = `${visualViewportHeight}px`; // Fixed-position descendants cannot inherit the shell element's pixel // height. Publish the same correction through the existing shell-height @@ -108,6 +135,28 @@ export function useMobileVisualViewportHeight( } animationFrame = window.requestAnimationFrame(updateHeight); }; + // For triggers where the layout viewport may have changed: window resize, + // rotation, and an editor gaining focus — the pass that sizes the shell + // for the arriving keyboard must start from the real containing block, + // and these triggers are rare enough that the forced layout is fine. + const scheduleContainingBlockUpdate = () => { + shellContainingBlockHeightStale = true; + scheduleUpdate(); + }; + const handleVisualViewportScroll = () => { + // Keyboard-less visual-viewport pans (URL-bar collapse, momentum + // settling) don't change the containing block and need no override — + // the pan compensation exists for the keyboard focus-reveal pan. Only + // an already-applied override still has to track pans, because embedded + // browsers apply one without any keyboard. + if ( + appliedOverride === null && + !isKeyboardFocusTarget(document.activeElement) + ) { + return; + } + scheduleUpdate(); + }; // Safari with its bottom toolbar visible does not update the visual // viewport until the keyboard animation ends. Restore the normal shell @@ -124,13 +173,16 @@ export function useMobileVisualViewportHeight( }; const handleFocusIn = (event: FocusEvent) => { if (!isKeyboardFocusTarget(event.target)) return; - scheduleUpdate(); + // Programmatic focus (composer autofocus) can be the only trigger for a + // keyboard, so this must always schedule a full, freshly measured pass. + scheduleContainingBlockUpdate(); }; updateHeight(); visualViewport.addEventListener("resize", scheduleUpdate); - visualViewport.addEventListener("scroll", scheduleUpdate); - window.addEventListener("resize", scheduleUpdate); + visualViewport.addEventListener("scroll", handleVisualViewportScroll); + window.addEventListener("resize", scheduleContainingBlockUpdate); + window.addEventListener("orientationchange", scheduleContainingBlockUpdate); if (restoreImmediatelyOnKeyboardDismissal) { document.addEventListener("focusout", handleFocusOut); document.addEventListener("focusin", handleFocusIn); @@ -138,8 +190,12 @@ export function useMobileVisualViewportHeight( return () => { visualViewport.removeEventListener("resize", scheduleUpdate); - visualViewport.removeEventListener("scroll", scheduleUpdate); - window.removeEventListener("resize", scheduleUpdate); + visualViewport.removeEventListener("scroll", handleVisualViewportScroll); + window.removeEventListener("resize", scheduleContainingBlockUpdate); + window.removeEventListener( + "orientationchange", + scheduleContainingBlockUpdate, + ); if (restoreImmediatelyOnKeyboardDismissal) { document.removeEventListener("focusout", handleFocusOut); document.removeEventListener("focusin", handleFocusIn); diff --git a/apps/app/src/components/thread/timeline/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/SelectableMessageProse.events.test.tsx b/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx index c832b02ff7..69f3753c86 100644 --- a/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx +++ b/apps/app/src/components/thread/timeline/SelectableMessageProse.events.test.tsx @@ -410,4 +410,24 @@ describe("SelectableMessageProse", () => { ), ); }); + + it("registers the shared pointer listeners as passive", () => { + const addSpy = vi.spyOn(document, "addEventListener"); + const view = render( + Answer prose, + ); + + // None of the pointer handlers call preventDefault; the passive flag is a + // perf contract (it keeps taps off the blocking-handler list), so pin it. + const optionsByType = new Map( + addSpy.mock.calls.map(([type, , options]) => [type, options]), + ); + for (const type of ["pointerdown", "pointerup", "pointercancel"]) { + expect(optionsByType.get(type), type).toEqual({ passive: true }); + } + + // Detach still matches (removeEventListener ignores `passive`): the + // shared-listener teardown test above covers the counts. + view.unmount(); + }); }); diff --git a/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx b/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx index 95e197a5c1..b987daf572 100644 --- a/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx +++ b/apps/app/src/components/thread/timeline/SelectableMessageProse.tsx @@ -406,9 +406,17 @@ function handleSharedKeyUp(): void { } function attachSharedDocumentListeners(): void { - document.addEventListener("pointerdown", handleSharedPointerDown); - document.addEventListener("pointerup", handleSharedPointerRelease); - document.addEventListener("pointercancel", handleSharedPointerCancel); + // Passive: none of the pointer handlers call preventDefault, so declare it + // and keep every tap off the compositor's blocking-handler list. + document.addEventListener("pointerdown", handleSharedPointerDown, { + passive: true, + }); + document.addEventListener("pointerup", handleSharedPointerRelease, { + passive: true, + }); + document.addEventListener("pointercancel", handleSharedPointerCancel, { + passive: true, + }); document.addEventListener("mouseup", handleSharedPointerRelease); document.addEventListener("selectionchange", handleSharedSelectionChange); document.addEventListener("keyup", handleSharedKeyUp); diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.actions.test.tsx index 092b1798f3..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} - - ); - }} - /> -
+ }} + /> +
+
); } diff --git a/apps/app/src/components/ui/app-route-anchor.test.tsx b/apps/app/src/components/ui/app-route-anchor.test.tsx new file mode 100644 index 0000000000..f346393344 --- /dev/null +++ b/apps/app/src/components/ui/app-route-anchor.test.tsx @@ -0,0 +1,65 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { MemoryRouter, useLocation } from "react-router-dom"; +import { afterEach, describe, expect, it } from "vitest"; +import { + RouteAnchor, + RouteNavigationProvider, + useIsRouteNavigationPending, +} from "./app-route-anchor"; + +afterEach(() => { + cleanup(); +}); + +interface NavigationSample { + isPending: boolean; + pathname: string; +} + +const samples: NavigationSample[] = []; + +/** + * Records every committed (isPending, pathname) pair. The pairs prove commit + * ordering: a `{ isPending: true, pathname: }` sample means the pending + * flag painted while the previous route was still on screen, i.e. the tap's + * event did not flush the destination route synchronously. + */ +function NavigationSampler() { + const isPending = useIsRouteNavigationPending(); + const { pathname } = useLocation(); + samples.push({ isPending, pathname }); + return null; +} + +describe("RouteAnchor transition navigation", () => { + it("swaps the route in a later commit than the tap and signals pending in between", () => { + samples.length = 0; + render( + + + + open thr-new + + , + ); + expect(samples).toEqual([ + { isPending: false, pathname: "/threads/thr-old" }, + ]); + + fireEvent.click(screen.getByRole("link", { name: "open thr-new" })); + + // The tap's urgent commit shows the pending affordance with the old route + // still mounted; the destination route lands in a follow-up transition + // commit, which also clears the pending flag. + expect(samples).toContainEqual({ + isPending: true, + pathname: "/threads/thr-old", + }); + expect(samples.at(-1)).toEqual({ + isPending: false, + pathname: "/threads/thr-new", + }); + }); +}); diff --git a/apps/app/src/components/ui/app-route-anchor.tsx b/apps/app/src/components/ui/app-route-anchor.tsx index 341a02db2f..72658a943d 100644 --- a/apps/app/src/components/ui/app-route-anchor.tsx +++ b/apps/app/src/components/ui/app-route-anchor.tsx @@ -6,6 +6,7 @@ import { useLayoutEffect, useMemo, useRef, + useTransition, type ComponentPropsWithoutRef, type MouseEvent as ReactMouseEvent, type ReactNode, @@ -36,6 +37,23 @@ type RouteNavigate = (path: string, options?: RouteNavigateOptions) => void; const RouteNavigationContext = createContext(null); +// Separate from RouteNavigationContext on purpose: the pending bit flips on +// every navigation, and folding it into the navigate context would re-render +// every navigate consumer (sidebar rows, thread actions) per navigation — +// the exact churn RouteNavigationContext exists to avoid. +const RouteNavigationPendingContext = createContext(false); + +/** + * True while a navigation started through {@link useRouteNavigate} or + * {@link RouteAnchor} is still rendering the destination route. Navigation + * runs at transition priority, so the previous route stays on screen for a + * beat; surfaces read this to show a lightweight pending affordance (e.g. + * keeping the tapped row's active state) instead of appearing unresponsive. + */ +export function useIsRouteNavigationPending(): boolean { + return useContext(RouteNavigationPendingContext); +} + /** * A `navigate` whose identity never changes and whose caller does not * subscribe to the router's location. @@ -94,13 +112,24 @@ export function RouteNavigationProvider({ useLayoutEffect(() => { navigateRef.current = navigate; }, [navigate]); - const navigateRoute = useCallback((path, options) => { - if (options === undefined) { - navigateRef.current(path); - return; - } - navigateRef.current(path, options); - }, []); + // Navigate at transition priority: a tap's urgent commit (active states, + // isNavigationPending) paints first, and the destination route renders in an + // interruptible follow-up commit instead of blocking the tap's frame. + // `startNavigationTransition` has a stable identity, so `navigateRoute` + // keeps the never-changing identity its consumers depend on. + const [isNavigationPending, startNavigationTransition] = useTransition(); + const navigateRoute = useCallback( + (path, options) => { + startNavigationTransition(() => { + if (options === undefined) { + navigateRef.current(path); + return; + } + navigateRef.current(path, options); + }); + }, + [startNavigationTransition], + ); useEffect(() => { const browserApi = getDesktopBrowserApi(); if (browserApi === null) { @@ -116,7 +145,9 @@ export function RouteNavigationProvider({ return ( - {children} + + {children} + ); } diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.coarse-pointer.test.tsx new file mode 100644 index 0000000000..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.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..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); @@ -284,6 +290,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 +331,20 @@ export function BottomAnchoredScrollBody({ // live geometry — the pre-cache behavior — instead of trusting a frozen // value that would classify every position as at-bottom. const resizeObserverHasDeliveredRef = useRef(false); + // The last box sizes the ResizeObserver reported for the scroll port and + // the content wrapper. Together they are the observer's own measurement of + // `scrollHeight - clientHeight`, letting the resize path refresh the cache + // without a forced layout (see handleScrollAreaResize). + const observedScrollGeometryRef = useRef<{ + scrollAreaClientHeight: number | null; + scrollContentHeight: number | null; + }>({ scrollAreaClientHeight: null, scrollContentHeight: null }); + // The row NodeList behind scroll-anchor capture, 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; @@ -359,6 +383,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 +411,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 +442,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 +470,7 @@ export function BottomAnchoredScrollBody({ } }; restoreFrameRef.current = window.requestAnimationFrame(runQueuedRestore); - }, [restoreBottomOnce]); + }, [restoreBottomOnce, restoreBottomFromCacheOnce]); const scrollToBottom = useCallback(() => { const scrollArea = scrollAreaRef.current; @@ -508,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 @@ -557,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, { @@ -567,6 +640,7 @@ export function BottomAnchoredScrollBody({ }); }, [ + getScrollAnchorRowsCached, hasRecentUserScrollIntent, readMaxScrollOffset, refreshMaxScrollOffset, @@ -575,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; @@ -592,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 @@ -805,48 +883,83 @@ export function BottomAnchoredScrollBody({ return true; }, [applyScrollRestore, queueBottomRestore]); - const handleScrollAreaResize = useCallback(() => { - const scrollArea = scrollAreaRef.current; - let shrankOntoBottomWhileDetached = false; - if (scrollArea) { - // The steady-state cache refresh: the observer watches both the scroll - // port and the content wrapper, so every legitimate - // scrollHeight/clientHeight change passes through here. The first - // delivery is also what makes the cache authoritative for hot-path - // reads (see resizeObserverHasDeliveredRef). - const previousMaxScrollOffset = maxScrollOffsetRef.current; - const cacheWasAuthoritative = resizeObserverHasDeliveredRef.current; - const maxScrollOffset = refreshMaxScrollOffset(scrollArea); - resizeObserverHasDeliveredRef.current = true; - shrankOntoBottomWhileDetached = - cacheWasAuthoritative && - !shouldStickToBottomRef.current && - maxScrollOffset < previousMaxScrollOffset && - isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop); - } - // While a restore is pending, the ResizeObserver is the settle signal; the - // bottom-restore is suppressed (stick-to-bottom is false) anyway. - if (advancePendingScrollRestore()) return; - if (shrankOntoBottomWhileDetached && scrollArea) { - // The detached mirror of the attach->detach edge in - // syncBottomStateFromScroll: a content shrink (collapsing a long tool - // output near the end) clamped a detached viewport onto the new, - // smaller maximum. The browser delivered that clamp's scroll event - // before this refresh, so the scroll handler classified it against the - // stale, larger cache and left the viewport detached. A live read used - // to re-attach on that very scroll event; do the same here, against - // fresh geometry, so streaming content keeps following the bottom. - attachToBottom(); - writeScrollAnchor(scrollArea); - } - queueBottomRestore(); - }, [ - advancePendingScrollRestore, - attachToBottom, - queueBottomRestore, - refreshMaxScrollOffset, - writeScrollAnchor, - ]); + const handleScrollAreaResize = useCallback( + (entries: ResizeObserverEntry[]) => { + const scrollArea = scrollAreaRef.current; + // Any observed size change may have mounted or unmounted timeline rows. + scrollAnchorRowsRef.current = null; + let shrankOntoBottomWhileDetached = false; + if (scrollArea) { + // The steady-state cache refresh: the observer watches both the scroll + // port and the content wrapper, so every legitimate + // scrollHeight/clientHeight change passes through here. The first + // delivery is also what makes the cache authoritative for hot-path + // reads (see resizeObserverHasDeliveredRef). + const previousMaxScrollOffset = maxScrollOffsetRef.current; + const cacheWasAuthoritative = resizeObserverHasDeliveredRef.current; + // Prefer the observer's own box sizes over a live + // scrollHeight/clientHeight read, which forces layout: the scroll + // port's content box is its client height (no padding, no horizontal + // scrollbar) and the content wrapper's border box is the scroll + // height, so the pair the observer just measured is the fresh + // geometry. Environments whose observer delivers no entries (test + // stubs) keep the live read. + const observedGeometry = observedScrollGeometryRef.current; + for (const entry of entries) { + if (entry.target === scrollArea) { + observedGeometry.scrollAreaClientHeight = + entry.contentBoxSize[0]?.blockSize ?? entry.contentRect.height; + } else if (entry.target === scrollContentRef.current) { + observedGeometry.scrollContentHeight = + entry.borderBoxSize[0]?.blockSize ?? entry.contentRect.height; + } + } + let maxScrollOffset: number; + if ( + observedGeometry.scrollAreaClientHeight !== null && + observedGeometry.scrollContentHeight !== null + ) { + maxScrollOffset = Math.max( + 0, + Math.round(observedGeometry.scrollContentHeight) - + Math.round(observedGeometry.scrollAreaClientHeight), + ); + maxScrollOffsetRef.current = maxScrollOffset; + } else { + maxScrollOffset = refreshMaxScrollOffset(scrollArea); + } + resizeObserverHasDeliveredRef.current = true; + shrankOntoBottomWhileDetached = + cacheWasAuthoritative && + !shouldStickToBottomRef.current && + maxScrollOffset < previousMaxScrollOffset && + isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop); + } + // While a restore is pending, the ResizeObserver is the settle signal; the + // bottom-restore is suppressed (stick-to-bottom is false) anyway. + if (advancePendingScrollRestore()) return; + if (shrankOntoBottomWhileDetached && scrollArea) { + // The detached mirror of the attach->detach edge in + // syncBottomStateFromScroll: a content shrink (collapsing a long tool + // output near the end) clamped a detached viewport onto the new, + // smaller maximum. The browser delivered that clamp's scroll event + // before this refresh, so the scroll handler classified it against the + // stale, larger cache and left the viewport detached. A live read used + // to re-attach on that very scroll event; do the same here, against + // fresh geometry, so streaming content keeps following the bottom. + attachToBottom(); + writeScrollAnchor(scrollArea); + } + queueBottomRestore(); + }, + [ + advancePendingScrollRestore, + attachToBottom, + queueBottomRestore, + refreshMaxScrollOffset, + writeScrollAnchor, + ], + ); // Begin restoring the saved scroll position on mount, before the listener // effect's `queueBottomRestore()` runs (a useEffect, which runs after layout @@ -930,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") { @@ -938,7 +1057,7 @@ export function BottomAnchoredScrollBody({ resizeObserver.observe(scrollContent); } - scrollArea.addEventListener("scroll", handleScrollWithTransientScrollbar, { + scrollArea.addEventListener("scroll", handleScrollEvent, { passive: true, }); scrollArea.addEventListener("wheel", markWheelScrollIntent, { @@ -964,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); @@ -986,6 +1102,7 @@ export function BottomAnchoredScrollBody({ endPointerScrollIntent, handleScroll, handleScrollAreaResize, + isPointerCoarse, markKeyboardScrollIntent, markTouchMoveScrollIntent, markTouchStartScrollIntent, 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..45d4027af3 100644 --- a/apps/app/src/components/ui/disclosure.tsx +++ b/apps/app/src/components/ui/disclosure.tsx @@ -1,5 +1,6 @@ import { useSetAtom } from "jotai"; import { + useDeferredValue, useEffect, useLayoutEffect, useMemo, @@ -8,10 +9,20 @@ import { type ReactNode, } from "react"; import { cn } from "@bb/shared-ui/lib/utils"; +import { + observedBorderBoxBlockSize, + observeSharedResize, +} from "@/lib/shared-resize-observer"; import { layoutAnimationInFlightCountAtom } from "./layoutAnimationAtoms.js"; import { CONTROL_HOVER_TRANSITION } from "@bb/shared-ui/motion"; const EXPANDABLE_PANEL_TRANSITION_MS = 200; + +/** Read half of the panel height sync, staged for the shared write phase. */ +interface PanelHeightSync { + isToggleAnimating: boolean; + heightPx: number; +} const useBrowserLayoutEffect = typeof window === "undefined" ? useEffect : useLayoutEffect; @@ -164,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 ( @@ -223,12 +249,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 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..ae3702c76d 100644 --- a/apps/app/src/components/ui/height-transition.tsx +++ b/apps/app/src/components/ui/height-transition.tsx @@ -8,6 +8,10 @@ import { subscribeToDocumentVisibility, } from "@/lib/document-visibility"; import { supportsScrollAnchoring } from "@/lib/scroll-anchoring-support"; +import { + observedBorderBoxBlockSize, + observeSharedResize, +} from "@/lib/shared-resize-observer"; import { layoutAnimationInFlightCountAtom } from "./layoutAnimationAtoms.js"; // Shared animation tokens for height transitions across the timeline. @@ -141,6 +145,21 @@ function cancelIntrinsicHeightRestore( resizeState.restoreTimerId = null; } +// The observer already measured the inner this frame, so reading the entry +// costs nothing, and the border box is the same metric as the offsetHeight +// used by the non-observer paths (initial mount, visibility snap) — the two +// must agree or a padded inner would get clipped by a content-box height. +// A dispatch without an entry (the shared observer's broadcast re-sync) or +// with a box-less synthetic one pays the offsetHeight read instead. +function getObservedInnerHeight( + entry: ResizeObserverEntry | undefined, + inner: HTMLElement, +): number { + const observed = + entry === undefined ? undefined : observedBorderBoxBlockSize(entry); + return observed ?? inner.offsetHeight; +} + interface HeightTransitionProps { visible: boolean; children: ReactNode; @@ -168,24 +187,34 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { let lastWidth: number | null = null; let pendingVisibilitySnap = false; const snapState: SnapState = { savedDuration: null, restoreFrame: null }; - const observer = new ResizeObserver((entries) => { - const entry = entries[0]; - if (!entry) return; - const { width, height } = entry.contentRect; - const widthChanged = lastWidth !== null && width !== lastWidth; - // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) - // is in flight, the inner is itself animating its size every frame. - // Running our own 180ms transition on top compounds the lag and drags - // scrollHeight, which the bottom-anchor sentinel then chases. - const layoutAnimationActive = - store.get(layoutAnimationInFlightCountAtom) > 0; - const snap = widthChanged || pendingVisibilitySnap || layoutAnimationActive; - pendingVisibilitySnap = false; - lastWidth = width; - const nextHeight = visible ? `${height}px` : "0px"; - applyHeight(wrapper, nextHeight, snap, snapState); + const unobserveInner = observeSharedResize(inner, { + read: (entry) => { + const width = entry?.contentRect?.width; + const widthChanged = + lastWidth !== null && width !== undefined && width !== lastWidth; + // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) + // is in flight, the inner is itself animating its size every frame. + // Running our own 180ms transition on top compounds the lag and drags + // scrollHeight, which the bottom-anchor sentinel then chases. + const layoutAnimationActive = + store.get(layoutAnimationInFlightCountAtom) > 0; + const snap = + widthChanged || pendingVisibilitySnap || layoutAnimationActive; + pendingVisibilitySnap = false; + if (width !== undefined) { + lastWidth = width; + } + return { + nextHeight: visible + ? `${getObservedInnerHeight(entry, inner)}px` + : "0px", + snap, + }; + }, + write: ({ nextHeight, snap }) => { + applyHeight(wrapper, nextHeight, snap, snapState); + }, }); - observer.observe(inner); // While a tab is hidden, ResizeObserver delivery is throttled and the CSS // height transition stays armed. If content grew during streaming, the // first observer fire after the user returns interpolates the full delta @@ -202,7 +231,7 @@ export function HeightTransition({ visible, children }: HeightTransitionProps) { const unsubscribeFromDocumentVisibility = subscribeToDocumentVisibility(onVisibility); return () => { - observer.disconnect(); + unobserveInner(); unsubscribeFromDocumentVisibility(); cleanupSnapState(wrapper, snapState); }; @@ -331,39 +360,51 @@ export function AutoHeightContainer({ initialSettleComplete = true; }, AUTO_HEIGHT_INITIAL_SETTLE_MS); }; - const observer = new ResizeObserver((entries) => { - const entry = entries[0]; - if (!entry) return; - const { width, height } = entry.contentRect; - const widthChanged = lastWidth !== null && width !== lastWidth; - // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) - // is in flight, the inner is itself animating its size every frame. - // Running our own 180ms transition on top compounds the lag and drags - // scrollHeight, which the bottom-anchor sentinel then chases. - const layoutAnimationActive = - store.get(layoutAnimationInFlightCountAtom) > 0; - const snap = - widthChanged || - pendingVisibilitySnap || - !initialSettleComplete || - layoutAnimationActive; - pendingVisibilitySnap = false; - lastWidth = width; - if (widthChanged || resizeState.usingIntrinsicHeight) { - enterIntrinsicHeightMode(wrapper, resizeState, snapState); - scheduleIntrinsicHeightRestore({ - inner, - resizeState, - snapState, - target: wrapper, - }); + const unobserveInner = observeSharedResize(inner, { + read: (entry) => { + const width = entry?.contentRect?.width; + const widthChanged = + lastWidth !== null && width !== undefined && width !== lastWidth; + // While a CSS layout animation (e.g. ExpandablePanel's grid expansion) + // is in flight, the inner is itself animating its size every frame. + // Running our own 180ms transition on top compounds the lag and drags + // scrollHeight, which the bottom-anchor sentinel then chases. + const layoutAnimationActive = + store.get(layoutAnimationInFlightCountAtom) > 0; + const snap = + widthChanged || + pendingVisibilitySnap || + !initialSettleComplete || + layoutAnimationActive; + pendingVisibilitySnap = false; + if (width !== undefined) { + lastWidth = width; + } + if (widthChanged || resizeState.usingIntrinsicHeight) { + return { useIntrinsicHeight: true as const }; + } + return { + useIntrinsicHeight: false as const, + nextHeight: `${getObservedInnerHeight(entry, inner)}px`, + snap, + }; + }, + write: (sync) => { + if (sync.useIntrinsicHeight) { + enterIntrinsicHeightMode(wrapper, resizeState, snapState); + scheduleIntrinsicHeightRestore({ + inner, + resizeState, + snapState, + target: wrapper, + }); + deferInitialSettleComplete(); + return; + } + applyHeight(wrapper, sync.nextHeight, sync.snap, snapState); deferInitialSettleComplete(); - return; - } - applyHeight(wrapper, `${height}px`, snap, snapState); - deferInitialSettleComplete(); + }, }); - observer.observe(inner); // See HeightTransition's matching block: a hidden tab pauses observer // delivery and the height transition, so content streamed in while the // tab was backgrounded would otherwise animate in over 180ms on return @@ -378,7 +419,7 @@ export function AutoHeightContainer({ subscribeToDocumentVisibility(onVisibility); return () => { snapToCurrentHeightRef.current = null; - observer.disconnect(); + unobserveInner(); unsubscribeFromDocumentVisibility(); window.clearTimeout(initialSettleTimerId); cancelIntrinsicHeightRestore(resizeState); diff --git a/apps/app/src/components/ui/sidebar.test.tsx b/apps/app/src/components/ui/sidebar.test.tsx index 86f26aff67..0c2932f1d5 100644 --- a/apps/app/src/components/ui/sidebar.test.tsx +++ b/apps/app/src/components/ui/sidebar.test.tsx @@ -8,6 +8,7 @@ import { screen, } from "@testing-library/react"; import { memo } from "react"; +import { flushSync } from "react-dom"; import { renderToString } from "react-dom/server"; import { afterEach, describe, expect, it, vi } from "vitest"; import { CompactViewportOverrideProvider } from "@bb/shared-ui/hooks/use-compact-viewport"; @@ -331,6 +332,37 @@ describe("mobile sidebar deferred realization", () => { expect(getMobilePanel()?.textContent).toContain("Sidebar content"); }); + it("keeps the realize commit out of the open tap's synchronous flush", () => { + vi.useFakeTimers(); + renderCompactSidebarHarness(); + const trigger = screen.getByRole("button", { name: "Toggle Sidebar" }); + + let panelStyledForSlideInTap = false; + let realizedInTapFlush = true; + act(() => { + // flushSync stands in for the tap's discrete event: it flushes only the + // urgent lane, so the transition-priority realize commit is still + // pending when the samples are taken and lands when act exits. + flushSync(() => { + trigger.click(); + }); + const panel = getMobilePanel(); + panelStyledForSlideInTap = panel?.style.translate === "0%"; + realizedInTapFlush = + panel?.textContent?.includes("Sidebar content") ?? false; + }); + + // The tap's own flush only starts the slide (inline drag styles); the + // subtree mounts in the interruptible commit that follows, so the first + // frame of the slide never waits on the realize commit. + expect(panelStyledForSlideInTap).toBe(true); + expect(realizedInTapFlush).toBe(false); + expect(getMobilePanel()?.textContent).toContain("Sidebar content"); + + settleMobileToggle(); + expect(getMobilePanel()?.dataset.state).toBe("open"); + }); + // The width is an inherited custom property unless registered otherwise // (theme.css registers it non-inherited). Either way it must be written on // the elements that read it and never on the provider wrapper: the wrapper diff --git a/apps/app/src/components/ui/sidebar.tsx b/apps/app/src/components/ui/sidebar.tsx index 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; diff --git a/apps/app/src/lib/shared-resize-observer.test.tsx b/apps/app/src/lib/shared-resize-observer.test.tsx new file mode 100644 index 0000000000..6492470593 --- /dev/null +++ b/apps/app/src/lib/shared-resize-observer.test.tsx @@ -0,0 +1,192 @@ +// @vitest-environment jsdom + +import { act, cleanup, render } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { ExpandablePanel } from "@/components/ui/disclosure"; +import { observeSharedResize } from "./shared-resize-observer"; + +class ResizeObserverStub implements ResizeObserver { + static instances: ResizeObserverStub[] = []; + + readonly observedTargets: Element[] = []; + + constructor(readonly callback: ResizeObserverCallback) { + ResizeObserverStub.instances.push(this); + } + + observe: ResizeObserver["observe"] = vi.fn((target: Element) => { + this.observedTargets.push(target); + }); + unobserve: ResizeObserver["unobserve"] = vi.fn(); + disconnect: ResizeObserver["disconnect"] = vi.fn(); +} + +afterEach(() => { + ResizeObserverStub.instances.length = 0; + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +function lastObserver(): ResizeObserverStub { + const observer = ResizeObserverStub.instances.at(-1); + if (!observer) { + throw new Error("No ResizeObserver was installed"); + } + return observer; +} + +function makeEntry(target: Element, blockSize: number): ResizeObserverEntry { + return { + target, + contentRect: new DOMRect(0, 0, 200, blockSize), + borderBoxSize: [{ blockSize, inlineSize: 200 }], + contentBoxSize: [{ blockSize, inlineSize: 200 }], + devicePixelContentBoxSize: [{ blockSize, inlineSize: 200 }], + }; +} + +describe("observeSharedResize", () => { + it("runs every registration's read before any write within a batch", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const order: string[] = []; + const first = document.createElement("div"); + const second = document.createElement("div"); + const unobserveFirst = observeSharedResize(first, { + read: () => { + order.push("read:first"); + return "first"; + }, + write: (value) => order.push(`write:${value}`), + }); + const unobserveSecond = observeSharedResize(second, { + read: () => { + order.push("read:second"); + return "second"; + }, + write: (value) => order.push(`write:${value}`), + }); + + // Two registrations, one observer: the whole point of sharing. + expect(ResizeObserverStub.instances).toHaveLength(1); + lastObserver().callback( + [makeEntry(first, 10), makeEntry(second, 20)], + lastObserver(), + ); + + expect(order).toEqual([ + "read:first", + "read:second", + "write:first", + "write:second", + ]); + unobserveFirst(); + unobserveSecond(); + }); + + it("re-syncs every registration when a synthetic batch carries no entries", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const order: string[] = []; + const seenEntries: (ResizeObserverEntry | undefined)[] = []; + const targets = [document.createElement("div"), document.createElement("div")]; + const unobservers = targets.map((target, index) => + observeSharedResize(target, { + read: (entry) => { + seenEntries.push(entry); + order.push(`read:${index}`); + return index; + }, + write: (value) => order.push(`write:${value}`), + }), + ); + + lastObserver().callback([], lastObserver()); + + // No entries means no target information: every registration re-syncs + // from live layout, still phased. + expect(order).toEqual(["read:0", "read:1", "write:0", "write:1"]); + expect(seenEntries).toEqual([undefined, undefined]); + for (const unobserve of unobservers) unobserve(); + }); + + it("releases the shared observer once the last registration leaves", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const target = document.createElement("div"); + const phases = { read: () => null, write: () => {} }; + + const unobserve = observeSharedResize(target, phases); + const observer = lastObserver(); + expect(observer.observe).toHaveBeenCalledWith(target); + + unobserve(); + expect(observer.unobserve).toHaveBeenCalledWith(target); + expect(observer.disconnect).toHaveBeenCalled(); + + // The next registration installs a fresh observer, so per-test + // `ResizeObserver` stubs take effect. + const unobserveAgain = observeSharedResize(target, phases); + expect(ResizeObserverStub.instances).toHaveLength(2); + unobserveAgain(); + }); +}); + +describe("ExpandablePanel on the shared observer", () => { + function renderTwoPanels() { + return render( + <> + First summary} + > + First body + + Second summary} + > + Second body + + , + ); + } + + it("mounts many panels onto one observer and sizes each from its own entry", () => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); + const view = renderTwoPanels(); + + // Two panels, one shared observer, one observation per panel body. + expect(ResizeObserverStub.instances).toHaveLength(1); + expect(lastObserver().observedTargets).toHaveLength(2); + + const regionOf = (text: string) => { + const region = + view.getByText(text).parentElement?.parentElement?.parentElement; + if (!region) { + throw new Error("Panel body region was not rendered"); + } + return region; + }; + const firstRegion = regionOf("First body"); + const secondRegion = regionOf("Second body"); + const [firstTarget, secondTarget] = lastObserver().observedTargets; + if (!firstTarget || !secondTarget) { + throw new Error("Panel bodies were not observed"); + } + + // One batch resizing both panels sizes each region from its own entry's + // border box — no per-panel layout read. + act(() => { + lastObserver().callback( + [makeEntry(firstTarget, 40), makeEntry(secondTarget, 60)], + lastObserver(), + ); + }); + + expect(firstRegion.style.height).toBe("40px"); + expect(secondRegion.style.height).toBe("60px"); + }); +}); diff --git a/apps/app/src/lib/shared-resize-observer.ts b/apps/app/src/lib/shared-resize-observer.ts new file mode 100644 index 0000000000..f0c0f21707 --- /dev/null +++ b/apps/app/src/lib/shared-resize-observer.ts @@ -0,0 +1,143 @@ +/** + * One module-level ResizeObserver shared by every registered element, with + * each delivery dispatched in two phases: every registration's `read` runs + * before any registration's `write`. + * + * Per-component observers whose callbacks interleave a layout read with a + * style write defeat the browser's batching: when one event resizes N + * observed elements at once (viewport resize, iOS keyboard, font swap), each + * callback's read forces layout against the previous callback's write — N + * synchronous layout passes over the document. Phasing the shared batch + * bounds that at one forced layout no matter how many elements resized. + * + * Same registry shape as `conversation-message-overflow.tsx`'s shared + * overflow observer; this module generalizes it to arbitrary read/write + * pairs and is the sanctioned pattern for per-row measurement. + */ + +export interface SharedResizePhases { + /** + * Gather everything `write` needs, preferring the entry's already-measured + * boxes over live layout reads. Runs with `undefined` when the dispatch + * carries no entry for the target (a broadcast re-sync) — read live layout + * (`offsetHeight`) then. Must not write styles. + */ + read: (entry: ResizeObserverEntry | undefined) => T; + /** Apply the value `read` produced. Must not read layout. */ + write: (value: T) => void; +} + +/** + * Type-erased registration: `read` closes over its typed value by returning + * the matching `write` as a thunk, so the registry needs no generics. + */ +interface RegisteredPhases { + read: (entry: ResizeObserverEntry | undefined) => () => void; +} + +interface PhaseDispatch { + registration: RegisteredPhases; + entry: ResizeObserverEntry | undefined; +} + +const phasesByTarget = new Map>(); +let sharedResizeObserver: ResizeObserver | null = null; + +function collectDispatches( + entries: readonly ResizeObserverEntry[], +): PhaseDispatch[] { + if (entries.length === 0) { + // The platform always delivers at least one entry. An empty batch only + // comes from synthetic dispatch (test stubs, polyfills) and carries no + // target information, so conservatively re-sync every registration from + // live layout. + return [...phasesByTarget.values()].flatMap((registrations) => + [...registrations].map((registration) => ({ + registration, + entry: undefined, + })), + ); + } + const dispatches: PhaseDispatch[] = []; + for (const entry of entries) { + for (const registration of phasesByTarget.get(entry.target) ?? []) { + dispatches.push({ registration, entry }); + } + } + return dispatches; +} + +function dispatchPhased(dispatches: readonly PhaseDispatch[]): void { + // Complete every read before any write can dirty layout for the next one. + const writes = dispatches.map(({ registration, entry }) => + registration.read(entry), + ); + for (const write of writes) { + write(); + } +} + +function getSharedResizeObserver(): ResizeObserver { + sharedResizeObserver ??= new ResizeObserver((entries) => { + dispatchPhased(collectDispatches(entries)); + }); + return sharedResizeObserver; +} + +/** + * Observe `target` on the shared observer. Returns the unregister function; + * the last unregistration for a target unobserves it, and the last overall + * releases the observer entirely (so per-test `ResizeObserver` stubs take + * effect on the next registration). + */ +export function observeSharedResize( + target: Element, + phases: SharedResizePhases, +): () => void { + const registration: RegisteredPhases = { + read: (entry) => { + const value = phases.read(entry); + return () => phases.write(value); + }, + }; + let registrations = phasesByTarget.get(target); + const isFirstForTarget = registrations === undefined; + if (registrations === undefined) { + registrations = new Set(); + phasesByTarget.set(target, registrations); + } + registrations.add(registration); + if (isFirstForTarget) { + // Register before observing: the initial observation can deliver + // synchronously in some environments and must reach this registration. + getSharedResizeObserver().observe(target); + } + + return () => { + const currentRegistrations = phasesByTarget.get(target); + currentRegistrations?.delete(registration); + if (currentRegistrations?.size === 0) { + phasesByTarget.delete(target); + sharedResizeObserver?.unobserve?.(target); + if (phasesByTarget.size === 0) { + sharedResizeObserver?.disconnect?.(); + sharedResizeObserver = null; + } + } + }; +} + +/** + * Border-box block size carried by an entry — `offsetHeight`'s metric without + * the layout read (the observer already measured this frame). The two must + * agree wherever an observer path and a direct path size the same element, or + * a padded box would get clipped by a content-box height. Returns `undefined` + * when the entry carries no usable box: entries cross a platform boundary and + * synthetic ones (test doubles, polyfills) omit boxes the spec guarantees — + * fall back to a live layout read then. + */ +export function observedBorderBoxBlockSize( + entry: ResizeObserverEntry, +): number | undefined { + return entry.borderBoxSize?.[0]?.blockSize ?? entry.contentRect?.height; +} diff --git a/apps/app/src/views/RootComposeMobileRecents.tsx b/apps/app/src/views/RootComposeMobileRecents.tsx index 1ad8475780..881c9c53f9 100644 --- a/apps/app/src/views/RootComposeMobileRecents.tsx +++ b/apps/app/src/views/RootComposeMobileRecents.tsx @@ -1,6 +1,6 @@ import { useMemo } from "react"; -import { Link } from "react-router-dom"; import type { ThreadListEntry } from "@bb/domain"; +import { RouteAnchor } from "@/components/ui/app-route-anchor"; import { ThreadStatusGlyph } from "@/components/sidebar/ThreadRow"; import { SIDEBAR_WORKING_STATUS_COLOR_CLASS } from "@/components/sidebar/sidebarRowClasses"; import { CHROME_SECTION_LABEL_CLASS } from "@bb/shared-ui/chrome-style-tokens"; @@ -117,8 +117,11 @@ function MobileRecentThreadRow({ ); return (
  • - - +
  • ); } diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx index 96314a8f48..0e87c509a8 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx @@ -783,6 +783,51 @@ describe("SplitThreadArea", () => { await waitFor(() => expect(hiddenScroller.scrollTop).toBe(0)); }); + it("stops the restore loop once positions settle instead of burning 30 frames", async () => { + renderSplitArea({ + path: threadPath("thr-a"), + layout: twoPaneLayout("pane-1"), + }); + const hiddenScroller = screen.getByTestId("scroll-thr-b"); + hiddenScroller.scrollTop = 12; + fireEvent.scroll(hiddenScroller); + + let scrollTopValue = 12; + const writes: number[] = []; + Object.defineProperty(hiddenScroller, "scrollTop", { + configurable: true, + get: () => scrollTopValue, + set: (value: number) => { + writes.push(value); + scrollTopValue = value; + }, + }); + + // Maximize: the tracked element already sits at its saved offset, so the + // pre-paint restore and the first frame find nothing to correct and the + // loop must end without a single scroll write (each write would force + // layout every frame for half a second). + fireEvent.click(screen.getByTestId("maximize-thr-a")); + await new Promise((resolve) => setTimeout(resolve, 600)); + expect(writes).toHaveLength(0); + + // Restore, with the scroller reporting 0 on every read — an adversary + // that keeps normalizing the position. The loop corrects before paint and + // on each frame, but gives up at the frame cap instead of running all 30. + Object.defineProperty(hiddenScroller, "scrollTop", { + configurable: true, + get: () => 0, + set: (value: number) => { + writes.push(value); + }, + }); + fireEvent.click(screen.getByTestId("maximize-thr-a")); + await new Promise((resolve) => setTimeout(resolve, 600)); + expect(writes.length).toBeGreaterThan(0); + // Pre-paint restore + at most 5 frames. + expect(writes.length).toBeLessThanOrEqual(6); + }); + it("toggles the focused pane through the discoverable app command", async () => { const store = renderSplitArea({ path: threadPath("thr-b"), diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.tsx index 8d0c53b721..e6e122ca8d 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.tsx @@ -215,30 +215,44 @@ function usePreservedSplitScrollPositions(maximizedPaneId: string | null) { } previousMaximizedPaneIdRef.current = maximizedPaneId; - const restore = () => { + /** Reapplies saved positions; true when any element needed correction. */ + const restore = (): boolean => { const workspace = workspaceRef.current; + let corrected = false; for (const [element, position] of positionsRef.current) { if (workspace === null || !workspace.contains(element)) { positionsRef.current.delete(element); continue; } + if ( + element.scrollLeft === position.left && + element.scrollTop === position.top + ) { + continue; + } element.scrollLeft = position.left; element.scrollTop = position.top; + corrected = true; } + return corrected; }; // Restore before paint, then briefly across animation frames so passive // timeline effects, virtualization, and browser scroll anchoring cannot - // overwrite the saved position while pane visibility settles. + // overwrite the saved position while pane visibility settles. Each frame + // forces layout on every tracked scroller, so the loop ends after the + // first frame with nothing to correct; the frame cap bounds the + // pathological case where something keeps fighting the restore. restore(); let frame: number | null = null; - let framesRemaining = 30; + let framesRemaining = 5; const restoreUntilSettled = () => { - restore(); + const corrected = restore(); framesRemaining -= 1; - if (framesRemaining > 0) { - frame = window.requestAnimationFrame(restoreUntilSettled); - } + frame = + corrected && framesRemaining > 0 + ? window.requestAnimationFrame(restoreUntilSettled) + : null; }; frame = window.requestAnimationFrame(restoreUntilSettled); return () => {