From 1db6b010c8b58e794edeed053c3ea2b117dd0b55 Mon Sep 17 00:00:00 2001 From: Wes Date: Sat, 25 Jul 2026 09:10:34 -0600 Subject: [PATCH] fix(desktop): preserve virtualized bottom intent Replace the timeout-based bottom chase with explicit bottom intent driven by Virtua content and viewport geometry. Reader navigation, targeted messages, and history prepends still take ownership immediately, while late row reflow and composer typing no longer strand the timeline above its newest message. Co-authored-by: Carl Signed-off-by: Wes --- .../messages/ui/TimelineMessageList.tsx | 14 +- .../ui/useVirtualizedBottomSettle.test.mjs | 227 ++++++++++++++++++ .../messages/ui/useVirtualizedBottomSettle.ts | 112 ++++++--- desktop/tests/e2e/scroll-history.spec.ts | 15 ++ 4 files changed, 329 insertions(+), 39 deletions(-) create mode 100644 desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index 55477e1253..1a0524e1ae 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -623,11 +623,8 @@ function VirtualizedTimelineRows({ if (!onVirtualizerApiChange) return; const api: TimelineVirtualizerApi = { scrollToBottom() { - retireTimelineSettle(); - const lastIndex = itemsLengthRef.current - 1; - if (lastIndex >= 0) { - listRef.current?.scrollToIndex(lastIndex, { align: "end" }); - } + retirePrependAnchor(); + settleAtBottom(); }, settleAtBottom, scrollToMessage(messageId) { @@ -640,7 +637,12 @@ function VirtualizedTimelineRows({ }; onVirtualizerApiChange(api); return () => onVirtualizerApiChange(null); - }, [onVirtualizerApiChange, retireTimelineSettle, settleAtBottom]); + }, [ + onVirtualizerApiChange, + retirePrependAnchor, + retireTimelineSettle, + settleAtBottom, + ]); React.useLayoutEffect(() => { const host = hostRef.current; diff --git a/desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs new file mode 100644 index 0000000000..1e0019bbb3 --- /dev/null +++ b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.test.mjs @@ -0,0 +1,227 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +class EventTargetShim { + listeners = new Map(); + addEventListener(type, listener) { + this.listeners.set(type, [...(this.listeners.get(type) ?? []), listener]); + } + removeEventListener(type, listener) { + this.listeners.set( + type, + (this.listeners.get(type) ?? []).filter( + (current) => current !== listener, + ), + ); + } + dispatchEvent(event) { + for (const listener of this.listeners.get(event.type) ?? []) + listener(event); + return true; + } +} + +class ElementShim extends EventTargetShim { + constructor(firstElementChild = null) { + super(); + this.firstElementChild = firstElementChild; + this.children = []; + this.childNodes = []; + this.isContentEditable = false; + this.nodeName = "DIV"; + this.tagName = "DIV"; + this.nodeType = 1; + this.namespaceURI = "http://www.w3.org/1999/xhtml"; + } + get ownerDocument() { + return globalThis.document; + } + appendChild(child) { + this.children.push(child); + this.childNodes.push(child); + return child; + } + removeChild(child) { + this.children = this.children.filter((current) => current !== child); + this.childNodes = this.childNodes.filter((current) => current !== child); + return child; + } + insertBefore(child, reference) { + const index = this.children.indexOf(reference); + if (index < 0) return this.appendChild(child); + this.children.splice(index, 0, child); + this.childNodes.splice(index, 0, child); + return child; + } + closest() { + return null; + } +} + +globalThis.document = { + addEventListener() {}, + createElement: () => new ElementShim(), + get defaultView() { + return globalThis.window; + }, + nodeType: 9, + removeEventListener() {}, +}; + +const animationFrames = new Map(); +let nextAnimationFrameId = 1; +globalThis.requestAnimationFrame = (callback) => { + const id = nextAnimationFrameId++; + animationFrames.set(id, callback); + return id; +}; +globalThis.cancelAnimationFrame = (id) => animationFrames.delete(id); +globalThis.HTMLElement = ElementShim; +globalThis.HTMLDivElement = ElementShim; +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +process.env.IS_REACT_ACT_ENVIRONMENT = "true"; +Object.defineProperty(globalThis, "window", { + configurable: true, + value: new EventTargetShim(), +}); +globalThis.window.HTMLIFrameElement = ElementShim; + +const resizeObservers = []; +globalThis.ResizeObserver = class { + constructor(callback) { + this.callback = callback; + resizeObservers.push(this); + } + disconnect() {} + observe(target) { + this.targets = [...(this.targets ?? []), target]; + } +}; + +import React from "react"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { useVirtualizedBottomSettle } from "./useVirtualizedBottomSettle.ts"; + +function flushAnimationFrames() { + const callbacks = [...animationFrames.values()]; + animationFrames.clear(); + for (const callback of callbacks) callback(performance.now()); +} + +function Harness({ apiRef, hostRef, itemsLengthRef, listRef }) { + apiRef.current = useVirtualizedBottomSettle(hostRef, listRef, itemsLengthRef); + return null; +} + +async function mountHarness() { + animationFrames.clear(); + resizeObservers.length = 0; + const content = new ElementShim(); + const scroller = new ElementShim(content); + const host = new ElementShim(scroller); + const writes = []; + const refs = { + api: { current: null }, + host: { current: host }, + itemsLength: { current: 5 }, + list: { + current: { + scrollToIndex(index, options) { + writes.push({ index, options }); + }, + }, + }, + }; + const root = createRoot(new ElementShim()); + await act(async () => { + root.render( + React.createElement(Harness, { + apiRef: refs.api, + hostRef: refs.host, + itemsLengthRef: refs.itemsLength, + listRef: refs.list, + }), + ); + }); + return { content, refs, root, scroller, writes }; +} + +test("bottom intent follows arbitrarily late virtual geometry changes", async () => { + const { content, refs, root, scroller, writes } = await mountHarness(); + refs.api.current.settle(); + assert.deepEqual(writes, [{ index: 4, options: { align: "end" } }]); + + const geometryObserver = resizeObservers.find((observer) => + observer.targets?.includes(content), + ); + assert.ok(geometryObserver); + geometryObserver.callback(); + flushAnimationFrames(); + assert.equal(writes.length, 2); + + // There is no deadline: another geometry change much later still re-pins. + geometryObserver.callback(); + flushAnimationFrames(); + assert.equal(writes.length, 3); + + // Viewport changes also move the physical floor without resizing content. + assert.ok(geometryObserver.targets.includes(scroller)); + geometryObserver.callback(); + flushAnimationFrames(); + assert.equal(writes.length, 4); + await act(async () => root.unmount()); +}); + +for (const eventType of ["pointerdown", "touchstart", "wheel", "keydown"]) { + test(`${eventType} transfers ownership away from bottom intent`, async () => { + const { content, refs, root, scroller, writes } = await mountHarness(); + refs.api.current.settle(); + const target = eventType === "keydown" ? window : scroller; + target.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: eventType === "keydown" ? "PageUp" : undefined, + metaKey: false, + type: eventType, + }); + + resizeObservers + .find((observer) => observer.targets?.includes(content)) + .callback(); + flushAnimationFrames(); + assert.equal(writes.length, 1); + await act(async () => root.unmount()); + }); +} + +test("typing and editable navigation keys preserve bottom intent", async () => { + const { content, refs, root, writes } = await mountHarness(); + refs.api.current.settle(); + + window.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: "a", + metaKey: false, + target: null, + type: "keydown", + }); + const editable = new ElementShim(); + editable.isContentEditable = true; + window.dispatchEvent({ + altKey: false, + ctrlKey: false, + key: "ArrowUp", + metaKey: false, + target: editable, + type: "keydown", + }); + + resizeObservers + .find((observer) => observer.targets?.includes(content)) + .callback(); + flushAnimationFrames(); + assert.equal(writes.length, 2); + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts index 2ff91de99f..dcb04d905c 100644 --- a/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts +++ b/desktop/src/features/messages/ui/useVirtualizedBottomSettle.ts @@ -1,67 +1,113 @@ import * as React from "react"; import type { VListHandle } from "virtua"; -const BOTTOM_EPSILON_PX = 1; -const SETTLE_DEADLINE_MS = 250; +const SCROLL_INTENT_KEYS = new Set([ + "ArrowDown", + "ArrowUp", + "End", + "Home", + "PageDown", + "PageUp", + " ", +]); + +function isEditableKeyboardTarget(target: EventTarget | null) { + if (!(target instanceof HTMLElement)) return false; + if (target.isContentEditable) return true; + return ( + target.closest("input, textarea, select, [contenteditable='true']") !== null + ); +} export function useVirtualizedBottomSettle( hostRef: React.RefObject, listRef: React.RefObject, itemsLengthRef: React.RefObject, ) { + const bottomIntentRef = React.useRef(false); const frameRef = React.useRef(null); - const cancel = React.useCallback(() => { + + const cancelFrame = React.useCallback(() => { if (frameRef.current !== null) { cancelAnimationFrame(frameRef.current); frameRef.current = null; } }, []); + const cancel = React.useCallback(() => { + bottomIntentRef.current = false; + cancelFrame(); + }, [cancelFrame]); + + const pinToBottom = React.useCallback(() => { + if (!bottomIntentRef.current) return; + const scroller = hostRef.current?.firstElementChild; + const lastIndex = itemsLengthRef.current - 1; + if (!(scroller instanceof HTMLDivElement) || lastIndex < 0) return; + listRef.current?.scrollToIndex(lastIndex, { align: "end" }); + }, [hostRef, itemsLengthRef, listRef]); + + const schedulePinToBottom = React.useCallback(() => { + if (!bottomIntentRef.current || frameRef.current !== null) return; + frameRef.current = requestAnimationFrame(() => { + frameRef.current = null; + pinToBottom(); + }); + }, [pinToBottom]); + React.useLayoutEffect(() => { const scroller = hostRef.current?.firstElementChild; if (!(scroller instanceof HTMLDivElement)) return; const retire = () => cancel(); + const retireForScrollKey = (event: KeyboardEvent) => { + if ( + !event.altKey && + !event.ctrlKey && + !event.metaKey && + !isEditableKeyboardTarget(event.target) && + SCROLL_INTENT_KEYS.has(event.key) + ) { + cancel(); + } + }; scroller.addEventListener("pointerdown", retire, { passive: true }); scroller.addEventListener("touchstart", retire, { passive: true }); scroller.addEventListener("wheel", retire, { passive: true }); - window.addEventListener("keydown", retire, true); + window.addEventListener("keydown", retireForScrollKey, true); return () => { scroller.removeEventListener("pointerdown", retire); scroller.removeEventListener("touchstart", retire); scroller.removeEventListener("wheel", retire); - window.removeEventListener("keydown", retire, true); + window.removeEventListener("keydown", retireForScrollKey, true); }; }, [cancel, hostRef]); + React.useLayoutEffect(() => { + const scroller = hostRef.current?.firstElementChild; + if (!(scroller instanceof HTMLDivElement)) return; + const content = scroller.firstElementChild; + if ( + !(content instanceof HTMLElement) || + typeof ResizeObserver === "undefined" + ) { + return; + } + // Virtua updates this inner element's extent whenever measured row geometry + // changes. Bottom intent therefore follows the physical floor for as long + // as it remains active, rather than guessing that layout is done after an + // arbitrary timeout. Reader input and explicit target/prepend navigation + // retire the intent through `cancel`. + const observer = new ResizeObserver(schedulePinToBottom); + observer.observe(content); + observer.observe(scroller); + return () => observer.disconnect(); + }, [hostRef, schedulePinToBottom]); + const settle = React.useCallback(() => { - cancel(); - const deadline = performance.now() + SETTLE_DEADLINE_MS; - let settledFrames = 0; - let previousHeight = -1; - const next = () => { - const scroller = hostRef.current?.firstElementChild; - const lastIndex = itemsLengthRef.current - 1; - if (!(scroller instanceof HTMLDivElement) || lastIndex < 0) { - cancel(); - return; - } - listRef.current?.scrollToIndex(lastIndex, { align: "end" }); - const atBottom = - scroller.scrollHeight - scroller.clientHeight - scroller.scrollTop <= - BOTTOM_EPSILON_PX; - settledFrames = - atBottom && scroller.scrollHeight === previousHeight - ? settledFrames + 1 - : 0; - previousHeight = scroller.scrollHeight; - if (settledFrames >= 2 || performance.now() >= deadline) { - frameRef.current = null; - return; - } - frameRef.current = requestAnimationFrame(next); - }; - next(); - }, [cancel, hostRef, itemsLengthRef, listRef]); + bottomIntentRef.current = true; + cancelFrame(); + pinToBottom(); + }, [cancelFrame, pinToBottom]); React.useEffect(() => cancel, [cancel]); return { cancel, settle }; diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index d92b6c46e7..b8d06fd7d6 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -99,6 +99,21 @@ test("channel switch settles at the newest message after virtualized rows measur const timeline = page.getByTestId("message-timeline"); await expect(timeline).toContainText("switch-bottom 79"); + // Reflow a rendered row well after the removed 250ms settle deadline. The + // geometry-driven bottom intent must still chase the new physical floor. + await timeline.evaluate((element) => { + window.setTimeout(() => { + const row = element.querySelector("[data-message-id]"); + if (row) { + row.style.minHeight = `${row.getBoundingClientRect().height + 240}px`; + } + element.dataset.delayedBottomReflow = "complete"; + }, 600); + }); + await expect(timeline).toHaveAttribute( + "data-delayed-bottom-reflow", + "complete", + ); await expect .poll(async () => { const metrics = await getTimelineMetrics(page);