diff --git a/src/features/chat/chat-message-renderer.ts b/src/features/chat/chat-message-renderer.ts index 56ca5098..54008a51 100644 --- a/src/features/chat/chat-message-renderer.ts +++ b/src/features/chat/chat-message-renderer.ts @@ -31,6 +31,7 @@ export interface ChatMessageRendererOptions { export class ChatMessageRenderer { private messagesEl: HTMLElement | null = null; + private bottomPinFrame: number | null = null; private readonly scrollController: MessageScrollController; private readonly markdownRenderer: MarkdownMessageRenderer; @@ -114,6 +115,7 @@ export class ChatMessageRenderer { } dispose(): void { + this.cancelBottomPinFrame(); if (this.messagesEl) { unmountReactRoot(this.messagesEl); } @@ -129,6 +131,15 @@ export class ChatMessageRenderer { } } + forceMessagesToBottom(): void { + this.scrollController.pinToBottom(this.messagesEl); + this.scheduleBottomPinAfterLayout(); + } + + correctMessagesAfterLayoutChange(): void { + this.scrollController.correctAfterLayoutChange(); + } + private async copyMessageText(text: string): Promise { await copyTextWithNotice(text, "Copied message.", "Could not copy message."); } @@ -147,6 +158,24 @@ export class ChatMessageRenderer { } this.dispatch({ type: "ui/detail-open-set", key, open }); } + + private scheduleBottomPinAfterLayout(): void { + const messagesEl = this.messagesEl; + if (!messagesEl || this.bottomPinFrame !== null) return; + + this.bottomPinFrame = messagesEl.win.requestAnimationFrame(() => { + this.bottomPinFrame = null; + if (!this.state.messagesPinnedToBottom) return; + this.scrollController.pinToBottom(this.messagesEl); + }); + } + + private cancelBottomPinFrame(): void { + const messagesEl = this.messagesEl; + if (!messagesEl || this.bottomPinFrame === null) return; + messagesEl.win.cancelAnimationFrame(this.bottomPinFrame); + this.bottomPinFrame = null; + } } export { implementPlanCandidateFromState }; diff --git a/src/features/chat/chat-view-controller-assembly.ts b/src/features/chat/chat-view-controller-assembly.ts index 53e7cd93..8a4d13bb 100644 --- a/src/features/chat/chat-view-controller-assembly.ts +++ b/src/features/chat/chat-view-controller-assembly.ts @@ -233,7 +233,7 @@ export function createChatViewControllerAssembly(host: ChatViewControllerAssembl renderIfDetached: host.effects.render.now, onDraftChange: host.effects.liveState.refresh, onComposerResize: () => { - if (host.getState().messagesPinnedToBottom) host.effects.scroll.forceBottom(); + host.effects.scroll.correctAfterLayoutChange(); }, onSubmit: () => void composerSubmission.submit(), onNewThread: () => void host.startNewThread(), diff --git a/src/features/chat/ui/composer.tsx b/src/features/chat/ui/composer.tsx index f044e212..b4009aca 100644 --- a/src/features/chat/ui/composer.tsx +++ b/src/features/chat/ui/composer.tsx @@ -85,8 +85,8 @@ function ComposerShell({ const composer = composerRef.current; if (!composer) return; onComposer(composer); - syncComposerHeight(composer); - }, [onComposer]); + if (syncComposerHeight(composer)) callbacks.onComposerResize(); + }, [callbacks, onComposer]); useLayoutEffect(() => { const container = suggestionsRef.current; const selected = selectedSuggestionRef.current; diff --git a/src/features/chat/ui/scroll.ts b/src/features/chat/ui/scroll.ts index 76a3d442..b293dfc0 100644 --- a/src/features/chat/ui/scroll.ts +++ b/src/features/chat/ui/scroll.ts @@ -79,6 +79,8 @@ export class MessageScrollController { private currentAnchor: ScrollAnchor | null = null; private lastScrollTop: number | null = null; private lastScrollHeight: number | null = null; + private lastClientHeight: number | null = null; + private userScrolledAwayFromBottom = false; constructor(private readonly options: MessageScrollControllerOptions) {} @@ -128,10 +130,15 @@ export class MessageScrollController { pinToBottom(container = this.container): void { if (!container) return; this.setScrollTop(container, bottomScrollTop(container)); + this.userScrolledAwayFromBottom = false; this.updatePinnedState(container); this.rememberCurrentAnchor(container); } + correctAfterLayoutChange(): void { + this.scheduleSizeChangeCorrection(); + } + scrollByTextLines(direction: MessageScrollDirection, container = this.container): void { if (!container) return; const maxScrollTop = Math.max(0, container.scrollHeight - container.clientHeight); @@ -163,6 +170,8 @@ export class MessageScrollController { this.currentAnchor = null; this.lastScrollTop = null; this.lastScrollHeight = null; + this.lastClientHeight = null; + this.userScrolledAwayFromBottom = false; } private attach(container: HTMLElement): void { @@ -172,6 +181,7 @@ export class MessageScrollController { this.container = container; this.lastScrollTop = container.scrollTop; this.lastScrollHeight = container.scrollHeight; + this.lastClientHeight = container.clientHeight; container.onscroll = this.handleScroll; const ResizeObserverConstructor = (container.win as Window & { ResizeObserver?: typeof ResizeObserver }).ResizeObserver; @@ -179,6 +189,7 @@ export class MessageScrollController { this.resizeObserver = new ResizeObserverConstructor(() => { this.scheduleSizeChangeCorrection(); }); + this.resizeObserver.observe(container); } } @@ -187,23 +198,31 @@ export class MessageScrollController { if (!container) return; const previousScrollTop = this.lastScrollTop ?? container.scrollTop; const previousScrollHeight = this.lastScrollHeight ?? container.scrollHeight; - const wasPinnedBeforeGrowth = + const previousClientHeight = this.lastClientHeight ?? container.clientHeight; + const wasPinnedBeforeLayoutChange = previousScrollHeight > 0 && isNearScrollBottom({ scrollHeight: previousScrollHeight, scrollTop: previousScrollTop, - clientHeight: container.clientHeight, + clientHeight: previousClientHeight, }); const grewSinceLastScroll = container.scrollHeight > previousScrollHeight; + const viewportHeightChanged = container.clientHeight !== previousClientHeight; this.lastScrollTop = container.scrollTop; this.lastScrollHeight = container.scrollHeight; - if (container.scrollTop < previousScrollTop) { + this.lastClientHeight = container.clientHeight; + if (viewportHeightChanged && wasPinnedBeforeLayoutChange) { + this.userScrolledAwayFromBottom = false; + this.options.setMessagesPinnedToBottom(true); + } else if (container.scrollTop < previousScrollTop) { + this.userScrolledAwayFromBottom = true; this.options.setMessagesPinnedToBottom(false); } else if ( !this.options.messagesPinnedToBottom() || - (container.scrollTop > previousScrollTop && (!grewSinceLastScroll || !wasPinnedBeforeGrowth)) + (container.scrollTop > previousScrollTop && (!grewSinceLastScroll || !wasPinnedBeforeLayoutChange)) ) { this.updatePinnedState(container); + if (this.options.messagesPinnedToBottom()) this.userScrolledAwayFromBottom = false; } this.rememberCurrentAnchor(container); }; @@ -217,7 +236,7 @@ export class MessageScrollController { const activeContainer = this.container; if (!activeContainer) return; - if (this.options.messagesPinnedToBottom()) { + if (this.options.messagesPinnedToBottom() || (!this.userScrolledAwayFromBottom && this.wasPinnedAtLastMeasurement(activeContainer))) { this.pinToBottom(activeContainer); } else { this.restoreAnchor(activeContainer, this.currentAnchor); @@ -268,12 +287,25 @@ export class MessageScrollController { container.scrollTop = scrollTop; this.lastScrollTop = container.scrollTop; this.lastScrollHeight = container.scrollHeight; + this.lastClientHeight = container.clientHeight; } private restoreAnchor(container: HTMLElement, anchor: ScrollAnchor | null): void { const scrollTop = restoredAnchorScrollTop(container, anchor); if (scrollTop !== null) this.setScrollTop(container, scrollTop); } + + private wasPinnedAtLastMeasurement(container: HTMLElement): boolean { + const scrollHeight = this.lastScrollHeight ?? container.scrollHeight; + return ( + scrollHeight > 0 && + isNearScrollBottom({ + scrollHeight, + scrollTop: this.lastScrollTop ?? container.scrollTop, + clientHeight: this.lastClientHeight ?? container.clientHeight, + }) + ); + } } function textLineHeight(element: HTMLElement): number { diff --git a/src/features/chat/view-effects.ts b/src/features/chat/view-effects.ts index 53250749..77d8a5cf 100644 --- a/src/features/chat/view-effects.ts +++ b/src/features/chat/view-effects.ts @@ -9,6 +9,7 @@ export interface ChatViewEffectHost { refreshLiveState: () => void; deferRefreshLiveState: () => void; forceMessagesToBottom: () => void; + correctMessagesAfterLayoutChange: () => void; preserveMessageScrollPosition: () => void; scrollMessagesToBottomOnFocus: () => void; setStatus: (status: string) => void; @@ -45,6 +46,7 @@ export interface ChatViewEffects { }; scroll: { forceBottom: () => void; + correctAfterLayoutChange: () => void; preservePosition: () => void; bottomOnFocus: () => void; }; @@ -95,6 +97,7 @@ export function createChatViewEffects(host: ChatViewEffectHost): ChatViewEffects }, scroll: { forceBottom: host.forceMessagesToBottom, + correctAfterLayoutChange: host.correctMessagesAfterLayoutChange, preservePosition: host.preserveMessageScrollPosition, bottomOnFocus: host.scrollMessagesToBottomOnFocus, }, diff --git a/src/features/chat/view-snapshot.ts b/src/features/chat/view-snapshot.ts index 492daf6c..b67f56bd 100644 --- a/src/features/chat/view-snapshot.ts +++ b/src/features/chat/view-snapshot.ts @@ -59,7 +59,7 @@ export function messagesSlotSnapshot(state: ChatState, pendingRequestsSignature: state.loadingHistory, chatTurnBusy(state), state.messagesPinnedToBottom, - state.composerDraft, + state.composerDraft.trim().length > 0, state.selectedCollaborationMode, displayItemsSignature(state.displayItems), turnDiffsSignature(state.turnDiffs), diff --git a/src/features/chat/view.ts b/src/features/chat/view.ts index cb30fc32..54384817 100644 --- a/src/features/chat/view.ts +++ b/src/features/chat/view.ts @@ -211,6 +211,10 @@ export class CodexChatView extends ItemView { }, forceMessagesToBottom: () => { this.messageScroll.forceBottom(); + this.messageRenderer.forceMessagesToBottom(); + }, + correctMessagesAfterLayoutChange: () => { + this.messageRenderer.correctMessagesAfterLayoutChange(); }, preserveMessageScrollPosition: () => { this.messageScroll.preservePosition(); diff --git a/tests/features/chat/chat-message-renderer.test.ts b/tests/features/chat/chat-message-renderer.test.ts index f4922345..adab1cb6 100644 --- a/tests/features/chat/chat-message-renderer.test.ts +++ b/tests/features/chat/chat-message-renderer.test.ts @@ -135,6 +135,68 @@ describe("ChatMessageRenderer scroll pinning", () => { expect(state.messagesPinnedToBottom).toBe(true); }); + it("can repin the current scroll container after composer growth shrinks the viewport", async () => { + const state = createChatState(); + state.activeThreadId = "thread"; + state.displayItems = [{ id: "message", kind: "message", role: "assistant", text: "Streaming message", turnId: "turn" }]; + const parent = document.createElement("div"); + const renderer = chatMessageRenderer(state); + + const messages = parent.createDiv({ cls: "codex-panel__messages" }); + Object.defineProperty(messages, "scrollHeight", { value: 1000, configurable: true }); + Object.defineProperty(messages, "clientHeight", { value: 160, configurable: true }); + messages.scrollTop = 1000; + renderer.render(messages); + await settleMessageRender(messages); + + Object.defineProperty(messages, "clientHeight", { value: 100, configurable: true }); + messages.scrollTop = 940; + + renderer.forceMessagesToBottom(); + + expect(messages.scrollTop).toBe(1000); + expect(state.messagesPinnedToBottom).toBe(true); + }); + + it("repins after composer growth has changed the scroll viewport height", async () => { + const state = createChatState(); + state.activeThreadId = "thread"; + state.displayItems = [{ id: "message", kind: "message", role: "assistant", text: "Streaming message", turnId: "turn" }]; + const parent = document.createElement("div"); + const renderer = chatMessageRenderer(state); + + const messages = parent.createDiv({ cls: "codex-panel__messages" }); + let scrollTop = 0; + let layoutSettled = false; + Object.defineProperties(messages, { + scrollHeight: { value: 1000, configurable: true }, + clientHeight: { + get: () => (layoutSettled ? 100 : 160), + configurable: true, + }, + scrollTop: { + get: () => scrollTop, + set: (value: number) => { + scrollTop = Math.min(value, 1000 - messages.clientHeight); + }, + configurable: true, + }, + }); + messages.scrollTop = 1000; + renderer.render(messages); + await settleMessageRender(messages); + expect(messages.scrollTop).toBe(840); + + renderer.forceMessagesToBottom(); + expect(messages.scrollTop).toBe(840); + + layoutSettled = true; + await settleMessageRender(messages); + + expect(messages.scrollTop).toBe(900); + expect(state.messagesPinnedToBottom).toBe(true); + }); + it("does not force the bottom into view when the user is reading older messages", async () => { const state = createChatState(); state.activeThreadId = "thread"; diff --git a/tests/features/chat/ui/view-scroll.test.ts b/tests/features/chat/ui/view-scroll.test.ts index 1d1c0dda..0f9b7331 100644 --- a/tests/features/chat/ui/view-scroll.test.ts +++ b/tests/features/chat/ui/view-scroll.test.ts @@ -136,6 +136,89 @@ describe("message scroll helpers", () => { resizeObserver.restore(); }); + it("keeps the scroll container pinned after the viewport height shrinks", async () => { + const resizeObserver = installResizeObserver(); + const container = clampedMessageContainer({ scrollTop: 0, scrollHeight: 1000, clientHeight: 160 }); + container.append(messageBlock("message", 0, 1000)); + let pinned = true; + const controller = new MessageScrollController({ + messagesPinnedToBottom: () => pinned, + setMessagesPinnedToBottom: (value) => { + pinned = value; + }, + }); + + controller.completeRender(controller.prepareRender(container, "auto")); + await animationFrame(container); + expect(container.scrollTop).toBe(840); + + setClientHeight(container, 100); + resizeObserver.trigger(); + await animationFrame(container); + + expect(container.scrollTop).toBe(900); + expect(pinned).toBe(true); + controller.dispose(); + resizeObserver.restore(); + }); + + it("repins viewport shrink from the last measured bottom even when state was stale", async () => { + const resizeObserver = installResizeObserver(); + const container = clampedMessageContainer({ scrollTop: 0, scrollHeight: 1000, clientHeight: 160 }); + container.append(messageBlock("message", 0, 1000)); + let pinned = true; + const controller = new MessageScrollController({ + messagesPinnedToBottom: () => pinned, + setMessagesPinnedToBottom: (value) => { + pinned = value; + }, + }); + + controller.completeRender(controller.prepareRender(container, "auto")); + await animationFrame(container); + expect(container.scrollTop).toBe(840); + + pinned = false; + setClientHeight(container, 100); + resizeObserver.trigger(); + await animationFrame(container); + + expect(container.scrollTop).toBe(900); + expect(pinned).toBe(true); + controller.dispose(); + resizeObserver.restore(); + }); + + it("keeps pinned state when viewport growth clamps scroll top downward", async () => { + const resizeObserver = installResizeObserver(); + const container = clampedMessageContainer({ scrollTop: 0, scrollHeight: 1000, clientHeight: 100 }); + container.append(messageBlock("message", 0, 1000)); + let pinned = true; + const controller = new MessageScrollController({ + messagesPinnedToBottom: () => pinned, + setMessagesPinnedToBottom: (value) => { + pinned = value; + }, + }); + + controller.completeRender(controller.prepareRender(container, "auto")); + await animationFrame(container); + expect(container.scrollTop).toBe(900); + + setClientHeight(container, 160); + container.dispatchEvent(new Event("scroll")); + expect(container.scrollTop).toBe(840); + expect(pinned).toBe(true); + + resizeObserver.trigger(); + await animationFrame(container); + + expect(container.scrollTop).toBe(840); + expect(pinned).toBe(true); + controller.dispose(); + resizeObserver.restore(); + }); + it("restores the remembered message block after an observed size change while reading history", async () => { const resizeObserver = installResizeObserver(); const container = messageContainer({ scrollTop: 150, scrollHeight: 1000, clientHeight: 300 }); @@ -350,6 +433,37 @@ function messageContainer(metrics: { scrollTop: number; scrollHeight: number; cl return container; } +function clampedMessageContainer(metrics: { scrollTop: number; scrollHeight: number; clientHeight: number }): HTMLElement { + const container = document.createElement("div"); + let scrollTop = metrics.scrollTop; + const scrollHeight = metrics.scrollHeight; + let clientHeight = metrics.clientHeight; + Object.defineProperties(container, { + scrollTop: { + get: () => scrollTop, + set: (value: number) => { + scrollTop = Math.min(value, Math.max(0, scrollHeight - clientHeight)); + }, + configurable: true, + }, + scrollHeight: { + get: () => scrollHeight, + configurable: true, + }, + clientHeight: { + get: () => clientHeight, + configurable: true, + }, + }); + Object.defineProperty(container, "setTestClientHeight", { + value: (value: number) => { + clientHeight = value; + scrollTop = Math.min(scrollTop, Math.max(0, scrollHeight - clientHeight)); + }, + }); + return container; +} + function messageBlock(key: string, offsetTop: number, offsetHeight: number): HTMLElement { const element = document.createElement("div"); element.setAttribute("data-codex-panel-block-key", key); @@ -368,6 +482,10 @@ function setScrollHeight(container: HTMLElement, value: number): void { (container as HTMLElement & { setTestScrollHeight: (scrollHeight: number) => void }).setTestScrollHeight(value); } +function setClientHeight(container: HTMLElement, value: number): void { + (container as HTMLElement & { setTestClientHeight: (clientHeight: number) => void }).setTestClientHeight(value); +} + function animationFrame(element: HTMLElement): Promise { return new Promise((resolve) => { element.win.requestAnimationFrame(() => { diff --git a/tests/features/chat/view-effects.test.ts b/tests/features/chat/view-effects.test.ts index 2b6a7015..0038bf0d 100644 --- a/tests/features/chat/view-effects.test.ts +++ b/tests/features/chat/view-effects.test.ts @@ -15,6 +15,7 @@ function createHost(): ChatViewEffectHost { refreshLiveState: vi.fn(), deferRefreshLiveState: vi.fn(), forceMessagesToBottom: vi.fn(), + correctMessagesAfterLayoutChange: vi.fn(), preserveMessageScrollPosition: vi.fn(), scrollMessagesToBottomOnFocus: vi.fn(), setStatus: vi.fn(), @@ -54,6 +55,7 @@ describe("createChatViewEffects", () => { effects.liveState.refresh(); effects.liveState.deferRefresh(); effects.scroll.forceBottom(); + effects.scroll.correctAfterLayoutChange(); effects.scroll.preservePosition(); effects.scroll.bottomOnFocus(); effects.status.set("ready"); @@ -83,6 +85,7 @@ describe("createChatViewEffects", () => { expect(host.refreshLiveState).toHaveBeenCalledOnce(); expect(host.deferRefreshLiveState).toHaveBeenCalledOnce(); expect(host.forceMessagesToBottom).toHaveBeenCalledOnce(); + expect(host.correctMessagesAfterLayoutChange).toHaveBeenCalledOnce(); expect(host.preserveMessageScrollPosition).toHaveBeenCalledOnce(); expect(host.scrollMessagesToBottomOnFocus).toHaveBeenCalledOnce(); expect(host.setStatus).toHaveBeenCalledWith("ready"); diff --git a/tests/features/chat/view-snapshot.test.ts b/tests/features/chat/view-snapshot.test.ts index f9f2905c..60a79247 100644 --- a/tests/features/chat/view-snapshot.test.ts +++ b/tests/features/chat/view-snapshot.test.ts @@ -40,6 +40,11 @@ describe("chat view snapshots", () => { expect(messagesSlotSnapshot(changedDraft, "")).not.toBe(messages); expect(composerSlotSnapshot(changedDraft, null)).not.toBe(composer); + const changedNonEmptyDraftText = { ...state, composerDraft: "hello again" }; + expect(messagesSlotSnapshot(changedNonEmptyDraftText, "")).not.toBe(messages); + expect(messagesSlotSnapshot(changedNonEmptyDraftText, "")).toBe(messagesSlotSnapshot(changedDraft, "")); + expect(composerSlotSnapshot(changedNonEmptyDraftText, null)).not.toBe(composer); + const changedStatus = { ...state, status: "Connected." }; expect(toolbarSlotSnapshot(changedStatus, false)).not.toBe(toolbar); expect(messagesSlotSnapshot(changedStatus, "")).toBe(messages);