From d4cc1a8b726da309eabf033b178dd969c05a1dcc Mon Sep 17 00:00:00 2001 From: Shakker Date: Tue, 14 Jul 2026 20:16:15 +0100 Subject: [PATCH] fix: consolidate chat scroll ownership --- ui/src/pages/chat/chat-pane.ts | 5 +-- ui/src/pages/chat/chat-send.test.ts | 1 - ui/src/pages/chat/chat-state.ts | 2 -- ui/src/pages/chat/scroll.test.ts | 50 ++--------------------------- ui/src/pages/chat/scroll.ts | 49 +--------------------------- 5 files changed, 7 insertions(+), 100 deletions(-) diff --git a/ui/src/pages/chat/chat-pane.ts b/ui/src/pages/chat/chat-pane.ts index 4731e9913869..871727809061 100644 --- a/ui/src/pages/chat/chat-pane.ts +++ b/ui/src/pages/chat/chat-pane.ts @@ -139,7 +139,7 @@ import { hasAbortableSessionRun, reconcileStaleChatRunAfterSessionStatePublication, } from "./run-lifecycle.ts"; -import { scheduleChatScroll } from "./scroll.ts"; +import { scheduleChatScroll, scheduleCommittedChatScroll } from "./scroll.ts"; import { clearChatMessagesFromCache } from "./session-message-cache.ts"; import { configureToolTitleFetcher } from "./tool-titles.ts"; @@ -2195,7 +2195,8 @@ class ChatPane extends OpenClawLightDomElement { allowExternalEmbedUrls: state.allowExternalEmbedUrls, chatMessageMaxWidth: state.chatMessageMaxWidth, assistantAttachmentAuthToken: resolveAssistantAttachmentAuthToken(state as never), - onAssistantAttachmentLoaded: () => state.scrollToBottom(), + onAssistantAttachmentLoaded: () => + scheduleCommittedChatScroll(state, false, false, { source: "resize" }), basePath: state.basePath, }; if (!this.showPaneHeader) { diff --git a/ui/src/pages/chat/chat-send.test.ts b/ui/src/pages/chat/chat-send.test.ts index e7f0b7206e9a..90705dc18b96 100644 --- a/ui/src/pages/chat/chat-send.test.ts +++ b/ui/src/pages/chat/chat-send.test.ts @@ -328,7 +328,6 @@ function makeHost(overrides?: Partial): TestChatHost { chatScrollCommitCleanup: null, chatScrollFrame: null, chatScrollGuardFrame: null, - chatScrollTimeout: null, chatScrollGeneration: 0, chatLastScrollTop: 0, chatLastScrollHeight: 0, diff --git a/ui/src/pages/chat/chat-state.ts b/ui/src/pages/chat/chat-state.ts index 1a7846479ed8..2d062a5fc751 100644 --- a/ui/src/pages/chat/chat-state.ts +++ b/ui/src/pages/chat/chat-state.ts @@ -248,7 +248,6 @@ export type ChatPageHost = ChatHost & chatStreamRenderFrame: number | null; chatScrollFrame: number | null; chatScrollGuardFrame: number | null; - chatScrollTimeout: number | null; chatScrollGeneration: number; chatLastScrollTop: number; chatLastScrollHeight: number; @@ -1291,7 +1290,6 @@ export function createPageState( chatStreamRenderFrame: null, chatScrollFrame: null, chatScrollGuardFrame: null, - chatScrollTimeout: null, chatScrollGeneration: 0, chatLastScrollTop: 0, chatLastScrollHeight: 0, diff --git a/ui/src/pages/chat/scroll.test.ts b/ui/src/pages/chat/scroll.test.ts index e0e28e960537..339e22e13e22 100644 --- a/ui/src/pages/chat/scroll.test.ts +++ b/ui/src/pages/chat/scroll.test.ts @@ -18,28 +18,16 @@ function createScrollHost( scrollHeight?: number; scrollTop?: number; clientHeight?: number; - overflowY?: string; } = {}, ) { - const { - scrollHeight = 2000, - scrollTop = 1500, - clientHeight = 500, - overflowY = "auto", - } = overrides; + const { scrollHeight = 2000, scrollTop = 1500, clientHeight = 500 } = overrides; const container = { scrollHeight, scrollTop, clientHeight, - style: { overflowY } as unknown as CSSStyleDeclaration, }; - // Make getComputedStyle return the overflowY value - vi.spyOn(window, "getComputedStyle").mockReturnValue({ - overflowY, - } as unknown as CSSStyleDeclaration); - const renderLifecycle: RenderLifecycle = { invalidate: vi.fn(), afterCommit: vi.fn((effect) => { @@ -56,7 +44,6 @@ function createScrollHost( chatScrollCommitCleanup: null as (() => void) | null, chatScrollFrame: null as number | null, chatScrollGuardFrame: null as number | null, - chatScrollTimeout: null as number | null, chatScrollGeneration: 0, chatLastScrollTop: 0, chatLastScrollHeight: 0, @@ -506,22 +493,18 @@ describe("resetChatScroll", () => { expect(host.chatProgrammaticScrollTarget).toBe(0); }); - it("cancels frame id zero and the late-size retry", () => { + it("cancels frame id zero and the programmatic guard", () => { const { host } = createScrollHost({}); const cancelFrame = vi.spyOn(window, "cancelAnimationFrame"); - const clearTimer = vi.spyOn(window, "clearTimeout"); host.chatScrollFrame = 0; host.chatScrollGuardFrame = 7; - host.chatScrollTimeout = 9; cancelChatScroll(host); expect(cancelFrame).toHaveBeenCalledWith(0); expect(cancelFrame).toHaveBeenCalledWith(7); - expect(clearTimer).toHaveBeenCalledWith(9); expect(host.chatScrollFrame).toBeNull(); expect(host.chatScrollGuardFrame).toBeNull(); - expect(host.chatScrollTimeout).toBeNull(); }); }); @@ -656,7 +639,7 @@ describe("programmatic scroll guard", () => { expect(host.chatNewMessagesBelow).toBe(false); }); - it("does not retry a smooth manual scroll after the user scrolls up", async () => { + it("stops a smooth manual scroll after the user scrolls up", async () => { const frameCallbacks: FrameRequestCallback[] = []; vi.spyOn(window, "requestAnimationFrame").mockImplementation((callback) => { frameCallbacks.push(callback); @@ -678,9 +661,7 @@ describe("programmatic scroll guard", () => { container.scrollTop = 400; handleChatScroll(host, createScrollEvent(2000, 400, 400)); - vi.advanceTimersByTime(200); - expect(host.chatScrollTimeout).toBeNull(); expect(host.chatIsProgrammaticScroll).toBe(false); expect(container.scrollTop).toBe(400); }); @@ -715,29 +696,4 @@ describe("programmatic scroll guard", () => { expect(host.chatUserNearBottom).toBe(false); }); - - it("retry timeout sets and clears chatIsProgrammaticScroll", async () => { - const { host, container } = createScrollHost({ - scrollHeight: 2000, - scrollTop: 1600, - clientHeight: 400, - }); - host.chatUserNearBottom = true; - host.chatHasAutoScrolled = true; - - scheduleChatScroll(host); - await host.updateComplete; - - // After the initial rAF the flag must already be cleared. - expect(host.chatIsProgrammaticScroll).toBe(false); - - // Advance past the retry delay (120ms) — retry scrollTop assignment fires. - vi.advanceTimersByTime(150); - - // After the retry's synchronous scrollTop assignment, the flag is set true. - // A subsequent rAF clears it — but our mock runs rAF synchronously. - expect(host.chatIsProgrammaticScroll).toBe(false); - // Retry must have updated the programmatic target and scrolled. - expect(host.chatProgrammaticScrollTarget).toBe(container.scrollHeight); - }); }); diff --git a/ui/src/pages/chat/scroll.ts b/ui/src/pages/chat/scroll.ts index 103b2e8a0c9f..00ac59c91fe9 100644 --- a/ui/src/pages/chat/scroll.ts +++ b/ui/src/pages/chat/scroll.ts @@ -12,7 +12,6 @@ type ChatScrollHost = { chatScrollCommitCleanup: (() => void) | null; chatScrollFrame: number | null; chatScrollGuardFrame: number | null; - chatScrollTimeout: number | null; chatScrollGeneration: number; chatLastScrollTop: number; chatLastScrollHeight?: number; @@ -42,10 +41,6 @@ function cancelCommittedChatScroll(host: ChatScrollHost): void { cancelAnimationFrame(host.chatScrollGuardFrame); host.chatScrollGuardFrame = null; } - if (host.chatScrollTimeout != null) { - clearTimeout(host.chatScrollTimeout); - host.chatScrollTimeout = null; - } host.chatIsProgrammaticScroll = false; } @@ -91,18 +86,7 @@ function scheduleProgrammaticScrollGuardClear( } function pickScrollTarget(host: ChatScrollHost): HTMLElement | null { - const container = queryHost(host, ".chat-thread") as HTMLElement | null; - if (container) { - const overflowY = getComputedStyle(container).overflowY; - const canScroll = - overflowY === "auto" || - overflowY === "scroll" || - container.scrollHeight - container.clientHeight > 1; - if (canScroll) { - return container; - } - } - return (document.scrollingElement ?? document.documentElement) as HTMLElement | null; + return queryHost(host, ".chat-thread") as HTMLElement | null; } /** Schedule layout work when the caller already runs after the DOM commit. */ @@ -164,33 +148,6 @@ export function scheduleCommittedChatScroll( scheduleProgrammaticScrollGuardClear(host, generation, target, smoothEnabled); host.chatUserNearBottom = true; setNewMessagesBelow(host, false); - - // Markdown, images, and mobile controls can grow after the first layout. - const retryDelay = effectiveForce ? 150 : 120; - host.chatScrollTimeout = window.setTimeout(() => { - host.chatScrollTimeout = null; - if (generation !== host.chatScrollGeneration) { - return; - } - const latest = pickScrollTarget(host); - if (!latest) { - return; - } - const latestDistanceFromBottom = latest.scrollHeight - latest.scrollTop - latest.clientHeight; - const shouldStickRetry = - manualScroll || - effectiveForce || - (!host.chatFollowLocked && - (host.chatUserNearBottom || latestDistanceFromBottom < NEAR_BOTTOM_THRESHOLD)); - if (!shouldStickRetry) { - return; - } - host.chatProgrammaticScrollTarget = latest.scrollHeight; - host.chatIsProgrammaticScroll = true; - latest.scrollTop = latest.scrollHeight; - scheduleProgrammaticScrollGuardClear(host, generation, latest, false); - host.chatUserNearBottom = true; - }, retryDelay); }); } @@ -237,10 +194,6 @@ export function handleChatScroll(host: ChatScrollHost, event: Event): void { cancelAnimationFrame(host.chatScrollGuardFrame); host.chatScrollGuardFrame = null; } - if (host.chatScrollTimeout != null) { - clearTimeout(host.chatScrollTimeout); - host.chatScrollTimeout = null; - } host.chatIsProgrammaticScroll = false; } const distanceFromBottom = container.scrollHeight - container.scrollTop - container.clientHeight;