From f6f9a0f5ba83fd810623801e74d92d402cd43078 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 20 Dec 2025 21:03:08 -0600 Subject: [PATCH] fix: coalesce ResizeObserver scrolls to prevent 1px visual flakiness The useAutoScroll hook's ResizeObserver was setting scrollTop synchronously on every resize event. During async content rendering (Shiki highlighting, Mermaid diagrams, images), this caused multiple scrolls per frame with slightly different scrollHeight values, leading to 1px scroll offset flakiness in visual regression tests. Changes: - Add RAF coalescing to ResizeObserver callback in useAutoScroll - Simplify story helpers to use a single RAF wait after message load - Remove complex multi-frame scroll stabilization from stories --- src/browser/hooks/useAutoScroll.ts | 12 +++++------ src/browser/stories/App.bash.stories.tsx | 21 ++++++++++---------- src/browser/stories/App.markdown.stories.tsx | 21 +------------------- src/browser/stories/storyPlayHelpers.ts | 21 ++++++++++++++++++++ 4 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/browser/hooks/useAutoScroll.ts b/src/browser/hooks/useAutoScroll.ts index 1c3b987957..8c6c59475e 100644 --- a/src/browser/hooks/useAutoScroll.ts +++ b/src/browser/hooks/useAutoScroll.ts @@ -26,11 +26,11 @@ export function useAutoScroll() { const autoScrollRef = useRef(true); // Track the ResizeObserver so we can disconnect it when the element unmounts const observerRef = useRef(null); + // Track pending RAF to coalesce rapid resize events + const rafIdRef = useRef(null); // Sync ref with state to ensure callbacks always have latest value autoScrollRef.current = autoScroll; - // Track pending RAF to coalesce rapid resize events - const rafIdRef = useRef(null); // Callback ref for the inner content wrapper - sets up ResizeObserver when element mounts. // ResizeObserver fires when the content size changes (Shiki highlighting, Mermaid, images, etc.), @@ -52,10 +52,10 @@ export function useAutoScroll() { // Skip if auto-scroll is disabled (user scrolled up) if (!autoScrollRef.current || !contentRef.current) return; - // Defer layout read to next frame to avoid forcing synchronous layout - // during React's commit phase (which can cause 50-85ms layout thrashing) - if (rafIdRef.current !== null) return; // Coalesce rapid calls - rafIdRef.current = requestAnimationFrame(() => { + // Coalesce all resize events in a frame into one scroll operation. + // Without this, rapid resize events (Shiki highlighting, etc.) cause + // multiple scrolls per frame with slightly different scrollHeight values. + rafIdRef.current ??= requestAnimationFrame(() => { rafIdRef.current = null; if (autoScrollRef.current && contentRef.current) { contentRef.current.scrollTop = contentRef.current.scrollHeight; diff --git a/src/browser/stories/App.bash.stories.tsx b/src/browser/stories/App.bash.stories.tsx index d9c05c58bb..4a6dbe906a 100644 --- a/src/browser/stories/App.bash.stories.tsx +++ b/src/browser/stories/App.bash.stories.tsx @@ -17,7 +17,11 @@ import { createBashBackgroundTerminateTool, } from "./mockFactory"; import { setupSimpleChatStory } from "./storyHelpers"; -import { blurActiveElement, waitForChatInputAutofocusDone } from "./storyPlayHelpers.js"; +import { + blurActiveElement, + waitForChatInputAutofocusDone, + waitForChatMessagesLoaded, +} from "./storyPlayHelpers.js"; import { userEvent, waitFor } from "@storybook/test"; /** @@ -25,16 +29,8 @@ import { userEvent, waitFor } from "@storybook/test"; * Waits for messages to load, then clicks on the ▶ expand icons to expand tool details. */ async function expandAllBashTools(canvasElement: HTMLElement) { - // Wait for messages to finish loading (non-racy: uses actual loading state) - await waitFor( - () => { - const messageWindow = canvasElement.querySelector('[data-testid="message-window"]'); - if (!messageWindow || messageWindow.getAttribute("data-loaded") !== "true") { - throw new Error("Messages not loaded yet"); - } - }, - { timeout: 5000 } - ); + // Wait for messages to finish loading + await waitForChatMessagesLoaded(canvasElement); // Now find and expand all tool icons (scoped to the message window so we don't click unrelated ▶) const messageWindow = canvasElement.querySelector('[data-testid="message-window"]'); @@ -64,6 +60,9 @@ async function expandAllBashTools(canvasElement: HTMLElement) { } } + // One RAF to let any pending coalesced scroll complete after tool expansion + await new Promise((r) => requestAnimationFrame(r)); + // Avoid leaving focus on a tool header. await waitForChatInputAutofocusDone(canvasElement); blurActiveElement(); diff --git a/src/browser/stories/App.markdown.stories.tsx b/src/browser/stories/App.markdown.stories.tsx index 0f21864df2..2215f7d905 100644 --- a/src/browser/stories/App.markdown.stories.tsx +++ b/src/browser/stories/App.markdown.stories.tsx @@ -5,18 +5,7 @@ import { appMeta, AppWithMocks, type AppStory } from "./meta.js"; import { STABLE_TIMESTAMP, createUserMessage, createAssistantMessage } from "./mockFactory"; import { expect, waitFor } from "@storybook/test"; - -async function waitForChatMessagesLoaded(canvasElement: HTMLElement): Promise { - await waitFor( - () => { - const messageWindow = canvasElement.querySelector('[data-testid="message-window"]'); - if (!messageWindow || messageWindow.getAttribute("data-loaded") !== "true") { - throw new Error("Messages not loaded yet"); - } - }, - { timeout: 5000 } - ); -} +import { waitForChatMessagesLoaded } from "./storyPlayHelpers"; import { setupSimpleChatStory } from "./storyHelpers"; @@ -273,14 +262,6 @@ export const CodeBlocks: AppStory = { { timeout: 5000 } ); - // Scroll to bottom and wait a frame for ResizeObserver to settle. - // Shiki highlighting can trigger useAutoScroll's ResizeObserver, causing scroll jitter. - const scrollContainer = canvasElement.querySelector('[data-testid="message-window"]'); - if (scrollContainer) { - scrollContainer.scrollTop = scrollContainer.scrollHeight; - await new Promise((r) => requestAnimationFrame(r)); - } - const url = "https://github.com/coder/mux/pull/new/chat-autocomplete-b24r"; const container = await waitFor( () => { diff --git a/src/browser/stories/storyPlayHelpers.ts b/src/browser/stories/storyPlayHelpers.ts index 123e374711..ec93b139c4 100644 --- a/src/browser/stories/storyPlayHelpers.ts +++ b/src/browser/stories/storyPlayHelpers.ts @@ -1,5 +1,26 @@ import { waitFor } from "@storybook/test"; +/** + * Wait for chat messages to finish loading. + * + * Waits for data-loaded="true" on the message window, then one RAF + * to let any pending coalesced scroll from useAutoScroll complete. + */ +export async function waitForChatMessagesLoaded(canvasElement: HTMLElement): Promise { + await waitFor( + () => { + const messageWindow = canvasElement.querySelector('[data-testid="message-window"]'); + if (!messageWindow || messageWindow.getAttribute("data-loaded") !== "true") { + throw new Error("Messages not loaded yet"); + } + }, + { timeout: 5000 } + ); + + // One RAF to let any pending coalesced scroll complete + await new Promise((r) => requestAnimationFrame(r)); +} + export async function waitForChatInputAutofocusDone(canvasElement: HTMLElement): Promise { await waitFor( () => {