From be5e080de697bf7555ec0deb90db168d15d9683c Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:55:41 +0100 Subject: [PATCH] fix(site/src/pages/AgentsPage): preserve chat scroll position when away from bottom (#23451) ## Summary Stabilizes the /agents chat viewport so users can read older messages without being yanked to the bottom when new content arrives. ## Architecture Replaces the implicit scroll-follow behavior with **ResizeObserver-driven scroll anchoring**: - **`autoScrollRef`** is the single source of truth. User scrolling away from bottom turns it off; scrolling back near bottom or clicking the button turns it back on. - A **content ResizeObserver** on an inner wrapper detects transcript growth. When auto-scroll is on, it re-pins to bottom via double-RAF (waiting for React commit + layout to settle). When off, it compensates `scrollTop` by the height delta to preserve the reading position. Sign-aware for both Chrome-style negative and Firefox-style positive `flex-col-reverse` scrollTop conventions. - Compensation is **skipped during pagination** (older messages prepend into the overflow direction; the browser preserves scrollTop) and **during reflow** from width changes. - A **container ResizeObserver** re-pins to bottom after viewport resizes (composer growth, panel changes) when auto-scroll is on. - **`isRestoringScrollRef`** guards against feedback loops from programmatic scroll writes. The smooth-scroll guard stays active until the scroll handler detects arrival at bottom. ## Files changed - **AgentDetailView.tsx**: Rewrote `ScrollAnchoredContainer` with the new approach. - **AgentDetailView.stories.tsx**: Refactored `ScrollToBottomButton` story scroll helpers into shared utilities. ## Behavior - **At bottom + new content**: stays pinned, button hidden. - **Scrolled up + new content**: reading position preserved, no jump. - **Viewport resize while pinned**: re-pins to bottom. - Scroll-to-bottom button and smooth scroll still work. --- .../components/AgentDetailView.stories.tsx | 187 ++++++++++++-- .../AgentsPage/components/AgentDetailView.tsx | 238 ++++++++++++++++-- 2 files changed, 384 insertions(+), 41 deletions(-) diff --git a/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx b/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx index f8f56afddc..28b86d8680 100644 --- a/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx +++ b/site/src/pages/AgentsPage/components/AgentDetailView.stories.tsx @@ -1,6 +1,6 @@ import { MockUserOwner } from "testHelpers/entities"; import { withAuthProvider, withDashboardProvider } from "testHelpers/storybook"; -import type { Meta, StoryObj } from "@storybook/react-vite"; +import type { Decorator, Meta, StoryObj } from "@storybook/react-vite"; import { API } from "api/api"; import type * as TypesGen from "api/typesGenerated"; import type { ChatDiffStatus, ChatMessagePart } from "api/typesGenerated"; @@ -453,25 +453,44 @@ const buildLongConversation = (count: number): TypesGen.ChatMessage[] => { return messages; }; +const scrollStoryDecorators: Decorator[] = [ + (Story) => ( +
+ +
+ ), +]; + +const waitForScrollOverflow = async (scrollContainer: HTMLElement) => { + await waitFor(() => { + expect(scrollContainer.scrollHeight).toBeGreaterThan( + scrollContainer.clientHeight, + ); + }); +}; + +const scrollAwayFromBottom = (scrollContainer: HTMLElement) => { + const maxScroll = scrollContainer.scrollHeight - scrollContainer.clientHeight; + scrollContainer.scrollTop = -maxScroll; + if (Math.abs(scrollContainer.scrollTop) < 100) { + scrollContainer.scrollTop = maxScroll; + } + scrollContainer.dispatchEvent(new Event("scroll")); +}; + /** Scroll-to-bottom button appears after scrolling up in a long * conversation, and clicking it returns to the bottom. */ export const ScrollToBottomButton: Story = { args: { store: buildStoreWithMessages(buildLongConversation(40)), }, - decorators: [ - (Story) => ( -
- -
- ), - ], + decorators: scrollStoryDecorators, play: async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -485,23 +504,13 @@ export const ScrollToBottomButton: Story = { const scrollContainer = canvas.getByTestId("scroll-container"); // Wait for content to render and create overflow. - await waitFor(() => { - expect(scrollContainer.scrollHeight).toBeGreaterThan( - scrollContainer.clientHeight, - ); - }); + await waitForScrollOverflow(scrollContainer); // Scroll up. In flex-col-reverse containers, Chrome uses // negative scrollTop values when scrolled away from the // bottom. Try negative first, fall back to positive for // other engines. - const maxScroll = - scrollContainer.scrollHeight - scrollContainer.clientHeight; - scrollContainer.scrollTop = -maxScroll; - if (Math.abs(scrollContainer.scrollTop) < 100) { - scrollContainer.scrollTop = maxScroll; - } - scrollContainer.dispatchEvent(new Event("scroll")); + scrollAwayFromBottom(scrollContainer); // Button should become visible (enters the accessibility tree). const button = await waitFor(() => { @@ -522,3 +531,129 @@ export const ScrollToBottomButton: Story = { }); }, }; + +/** When scrolled away from bottom, new content preserves scroll position. */ +export const ScrollPositionPreservedOnNewContent: Story = { + args: { + store: buildStoreWithMessages(buildLongConversation(30)), + }, + decorators: scrollStoryDecorators, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const scrollContainer = canvas.getByTestId("scroll-container"); + + await waitForScrollOverflow(scrollContainer); + + // Scroll away from bottom. + scrollAwayFromBottom(scrollContainer); + + // Wait for the button to confirm we are away from the bottom. + await waitFor( + () => { + expect( + canvas.getByRole("button", { name: "Scroll to bottom" }), + ).toBeVisible(); + }, + { timeout: 2000 }, + ); + + // Record position while clearly away from the bottom. + const scrollTopBefore = scrollContainer.scrollTop; + expect(Math.abs(scrollTopBefore)).toBeGreaterThan(50); + + const store = args?.store; + if (!store) { + throw new Error("Expected store in args"); + } + + const snapshot = store.getSnapshot(); + const existing: TypesGen.ChatMessage[] = []; + for (const id of snapshot.orderedMessageIDs) { + const message = snapshot.messagesByID.get(id); + if (message) { + existing.push(message); + } + } + + const newMessages = [ + buildMessage(31, "user", "Follow-up question about the implementation."), + buildMessage( + 32, + "assistant", + "Here is a detailed response about the implementation details you asked about.", + ), + ]; + store.replaceMessages(existing.concat(newMessages)); + + // Wait for ResizeObserver + RAF compensation to settle. + // We should remain significantly away from the bottom. + await waitFor( + () => { + expect(Math.abs(scrollContainer.scrollTop)).toBeGreaterThan(50); + }, + { timeout: 2000 }, + ); + + expect( + canvas.getByRole("button", { name: "Scroll to bottom" }), + ).toBeVisible(); + }, +}; + +/** When at bottom, new content keeps the user pinned to bottom. */ +export const ScrollPinnedToBottomOnNewContent: Story = { + args: { + store: buildStoreWithMessages(buildLongConversation(30)), + }, + decorators: scrollStoryDecorators, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const scrollContainer = canvas.getByTestId("scroll-container"); + + await waitForScrollOverflow(scrollContainer); + + // Verify the starting position is pinned to the bottom. + expect(Math.abs(scrollContainer.scrollTop)).toBeLessThan(5); + expect( + canvas.queryByRole("button", { name: "Scroll to bottom" }), + ).toBeNull(); + + const store = args?.store; + if (!store) { + throw new Error("Expected store in args"); + } + + const snapshot = store.getSnapshot(); + const existing: TypesGen.ChatMessage[] = []; + for (const id of snapshot.orderedMessageIDs) { + const message = snapshot.messagesByID.get(id); + if (message) { + existing.push(message); + } + } + + const newMessages = [ + buildMessage(31, "user", "Another question."), + buildMessage(32, "assistant", "Here is the answer with full details."), + buildMessage(33, "user", "Thanks, one more thing."), + buildMessage( + 34, + "assistant", + "Sure, here is the additional information you requested.", + ), + ]; + store.replaceMessages(existing.concat(newMessages)); + + // Wait for the double-RAF pin to complete. + await waitFor( + () => { + expect(Math.abs(scrollContainer.scrollTop)).toBeLessThan(5); + }, + { timeout: 2000 }, + ); + + expect( + canvas.queryByRole("button", { name: "Scroll to bottom" }), + ).toBeNull(); + }, +}; diff --git a/site/src/pages/AgentsPage/components/AgentDetailView.tsx b/site/src/pages/AgentsPage/components/AgentDetailView.tsx index 00a088c357..bafb5fb1b8 100644 --- a/site/src/pages/AgentsPage/components/AgentDetailView.tsx +++ b/site/src/pages/AgentsPage/components/AgentDetailView.tsx @@ -503,13 +503,29 @@ export const AgentDetailNotFoundView: FC = ({ /** * Scroll container that uses flex-col-reverse for bottom-anchored chat - * layout. Handles loading older message pages via an IntersectionObserver - * sentinel and manually restores scroll position after new content - * renders — CSS scroll anchoring is unreliable in flex-col-reverse - * containers. + * layout. In this layout scrollTop = 0 means the user is at the + * bottom (most recent content); scrolling up moves scrollTop away from + * 0 (negative in Chrome, positive in Firefox). + * + * Handles: + * - Loading older message pages via an IntersectionObserver sentinel. + * - ResizeObserver-driven scroll anchoring for transcript and viewport + * size changes. + * - A floating "Scroll to bottom" button when the user is scrolled + * away from the bottom. + * + * CSS scroll anchoring is unreliable in flex-col-reverse containers, + * so all position restoration is done manually. */ const SCROLL_THRESHOLD = 100; +// In flex-col-reverse, scrollTop is 0 at the bottom. Its sign +// when scrolled up varies by engine (negative in Chrome, positive +// in Firefox). The user is "near bottom" when close to 0. +function isNearBottom(container: HTMLElement): boolean { + return Math.abs(container.scrollTop) < SCROLL_THRESHOLD; +} + const ScrollAnchoredContainer: FC<{ scrollContainerRef: RefObject; isFetchingMoreMessages: boolean; @@ -527,6 +543,13 @@ const ScrollAnchoredContainer: FC<{ const observerRef = useRef(null); const isFetchingRef = useRef(isFetchingMoreMessages); const onFetchRef = useRef(onFetchMoreMessages); + const autoScrollRef = useRef(true); + const contentRef = useRef(null); + // Guard flag: true while a programmatic scroll adjustment is in-flight. + // The scroll handler skips autoScrollRef updates and re-render triggers + // when this is set, preventing user-visible jitter. Cleared when the + // scroll reaches its destination or the user actively interrupts. + const isRestoringScrollRef = useRef(false); useEffect(() => { isFetchingRef.current = isFetchingMoreMessages; onFetchRef.current = onFetchMoreMessages; @@ -576,13 +599,164 @@ const ScrollAnchoredContainer: FC<{ observer.observe(sentinel); }, [isFetchingMoreMessages]); + useEffect(() => { + const container = scrollContainerRef.current; + const content = contentRef.current; + if (!container || !content) return; + + const initialContentRect = content.getBoundingClientRect(); + let prevContentHeight = initialContentRect.height; + let prevContentWidth = initialContentRect.width; + let pinOuterRafId: number | null = null; + let pinInnerRafId: number | null = null; + let restoreGuardRafId: number | null = null; + + const cancelPendingPins = () => { + if (pinOuterRafId !== null) { + cancelAnimationFrame(pinOuterRafId); + } + if (pinInnerRafId !== null) { + cancelAnimationFrame(pinInnerRafId); + } + pinOuterRafId = null; + pinInnerRafId = null; + }; + + const scheduleBottomPin = () => { + cancelPendingPins(); + isRestoringScrollRef.current = true; + // Double-RAF lets React's commit phase and the browser's + // layout pass both complete before we pin to bottom. + pinOuterRafId = requestAnimationFrame(() => { + pinOuterRafId = null; + pinInnerRafId = requestAnimationFrame(() => { + pinInnerRafId = null; + if (!autoScrollRef.current) { + isRestoringScrollRef.current = false; + return; + } + if (restoreGuardRafId !== null) { + cancelAnimationFrame(restoreGuardRafId); + } + container.scrollTop = 0; + restoreGuardRafId = requestAnimationFrame(() => { + isRestoringScrollRef.current = false; + restoreGuardRafId = null; + }); + }); + }); + }; + + const compensateScroll = (delta: number) => { + if (restoreGuardRafId !== null) { + cancelAnimationFrame(restoreGuardRafId); + } + isRestoringScrollRef.current = true; + // In flex-col-reverse, "away from bottom" can be either + // negative (Chrome) or positive (Firefox). Detect which + // convention applies and compensate accordingly. + if (container.scrollTop < 0) { + // Negative convention: subtract to move away from 0 (bottom). + container.scrollTop -= delta; + } else { + // Positive convention: add to move away from 0 (bottom). + container.scrollTop += delta; + } + restoreGuardRafId = requestAnimationFrame(() => { + isRestoringScrollRef.current = false; + restoreGuardRafId = null; + }); + }; + + const observer = new ResizeObserver((entries) => { + const entry = entries[0]; + const nextHeight = + entry?.contentRect.height ?? content.getBoundingClientRect().height; + const nextWidth = + entry?.contentRect.width ?? content.getBoundingClientRect().width; + const delta = nextHeight - prevContentHeight; + const widthChanged = Math.abs(nextWidth - prevContentWidth) > 1; + prevContentHeight = nextHeight; + prevContentWidth = nextWidth; + if (Math.abs(delta) < 1) { + return; + } + + // Skip compensation during pagination. Older messages are + // prepended in flex-col-reverse which grows content into the + // overflow direction; the browser preserves scrollTop for us. + if (isFetchingRef.current) { + return; + } + + // Skip compensation during reflow. Width changes indicate the + // height delta is distributed through the transcript rather than + // appended at the bottom, so applying the full delta would + // overcompensate and jump the user. + if (widthChanged) { + return; + } + + if (autoScrollRef.current) { + scheduleBottomPin(); + return; + } + + compensateScroll(delta); + }); + observer.observe(content); + + return () => { + observer.disconnect(); + cancelPendingPins(); + if (restoreGuardRafId !== null) { + cancelAnimationFrame(restoreGuardRafId); + } + isRestoringScrollRef.current = false; + }; + }, [scrollContainerRef]); + + useEffect(() => { + const container = scrollContainerRef.current; + if (!container) return; + + let prevContainerHeight = container.clientHeight; + let restoreGuardRafId: number | null = null; + + const observer = new ResizeObserver((entries) => { + const nextHeight = + entries[0]?.contentRect.height ?? container.clientHeight; + const delta = nextHeight - prevContainerHeight; + prevContainerHeight = nextHeight; + if (Math.abs(delta) < 1 || !autoScrollRef.current) { + return; + } + + if (restoreGuardRafId !== null) { + cancelAnimationFrame(restoreGuardRafId); + } + isRestoringScrollRef.current = true; + container.scrollTop = 0; + restoreGuardRafId = requestAnimationFrame(() => { + isRestoringScrollRef.current = false; + restoreGuardRafId = null; + }); + }); + observer.observe(container); + + return () => { + observer.disconnect(); + if (restoreGuardRafId !== null) { + cancelAnimationFrame(restoreGuardRafId); + } + isRestoringScrollRef.current = false; + }; + }, [scrollContainerRef]); + // Track scroll position to show/hide the scroll-to-bottom button. // In a flex-col-reverse container, scrollTop = 0 means the user - // is at the bottom (most recent content). Scrolling up to see - // older messages makes scrollTop negative. - // - // Throttled to once per animation frame so we avoid calling - // setState on every high-frequency scroll event. + // is at the bottom (most recent content). Scrolling up moves + // scrollTop away from 0, with the sign varying by engine. useEffect(() => { const container = scrollContainerRef.current; if (!container) return; @@ -590,19 +764,51 @@ const ScrollAnchoredContainer: FC<{ let rafId: number | null = null; const handleScroll = () => { + // While a programmatic scroll is in progress (e.g. smooth + // scroll-to-bottom), suppress normal handling. Clear the + // guard once the scroll reaches the bottom so normal + // tracking resumes. User-input interruptions are handled + // separately via wheel/touchstart listeners. + if (isRestoringScrollRef.current) { + if (isNearBottom(container)) { + isRestoringScrollRef.current = false; + autoScrollRef.current = true; + } + return; + } + + const nearBottom = isNearBottom(container); + autoScrollRef.current = nearBottom; + + // Throttle the button visibility state update to once per + // frame. This is the only part that triggers a re-render. if (rafId !== null) return; rafId = requestAnimationFrame(() => { - const shouldShow = Math.abs(container.scrollTop) >= SCROLL_THRESHOLD; - setShowScrollToBottom((prev) => - prev === shouldShow ? prev : shouldShow, - ); + setShowScrollToBottom((prev) => { + const shouldShow = !isNearBottom(container); + return prev === shouldShow ? prev : shouldShow; + }); rafId = null; }); }; + const handleUserInterrupt = () => { + if (isRestoringScrollRef.current) { + isRestoringScrollRef.current = false; + } + }; + container.addEventListener("scroll", handleScroll, { passive: true }); + container.addEventListener("wheel", handleUserInterrupt, { + passive: true, + }); + container.addEventListener("touchstart", handleUserInterrupt, { + passive: true, + }); return () => { container.removeEventListener("scroll", handleScroll); + container.removeEventListener("wheel", handleUserInterrupt); + container.removeEventListener("touchstart", handleUserInterrupt); if (rafId !== null) { cancelAnimationFrame(rafId); } @@ -612,9 +818,11 @@ const ScrollAnchoredContainer: FC<{ const handleScrollToBottom = () => { const container = scrollContainerRef.current; if (!container) return; + autoScrollRef.current = true; + isRestoringScrollRef.current = true; container.scrollTo({ top: 0, behavior: "smooth" }); // Hide immediately so the button doesn't linger while the - // smooth scroll animates. If the user interrupts the scroll + // smooth scroll animates. If the user interrupts the scroll // before it reaches the bottom, the scroll handler will // re-show the button. setShowScrollToBottom(false); @@ -627,7 +835,7 @@ const ScrollAnchoredContainer: FC<{ data-testid="scroll-container" className="flex min-h-0 flex-1 flex-col-reverse overflow-y-auto [overflow-anchor:none] [scrollbar-gutter:stable] [scrollbar-width:thin] [scrollbar-color:hsl(var(--surface-quaternary))_transparent]" > - {children} +
{children}
{hasMoreMessages &&
}