diff --git a/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs new file mode 100644 index 0000000000..7784bea4b8 --- /dev/null +++ b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs @@ -0,0 +1,233 @@ +/** + * The user's own send reaches the bottom of the timeline even when Virtua + * reports a gap from the floor around it. + * + * MessageTimeline freezes its live tail when Virtua reports that the reader + * has left the bottom, and a send arms a one-shot pin for the next append. If + * the freeze lands before the sent row reaches the deferred snapshot, the row + * waits behind a "1 new message" pill, and the pin never sees its append. + * Virtua can report that gap while the reader is at the floor: it counts rows + * in its store during render, so a deferred render that React discards leaves + * the sent row counted. The next scroll event, from the composer shrinking + * after the send, then reads that row's height as distance from the bottom. + */ +import assert from "node:assert/strict"; +import { afterEach, test } from "node:test"; + +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + RouterContextProvider, + createMemoryHistory, + createRootRoute, + createRouter, +} from "@tanstack/react-router"; +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; + +import { MessageTimeline } from "./MessageTimeline.tsx"; + +const ROW = 60; +const VIEWPORT = 400; + +// jsdom has no layout. Model what the timeline reads: the scroller's offset +// is clamped to the height Virtua gives its inner container, and the list +// host is as tall as the scroller so Virtua keeps an offscreen buffer. +const offsets = new WeakMap(); +const isScroller = (element) => element?.style?.overflowY === "auto"; +const maxOffset = (element) => + Math.max( + (Number.parseFloat(element.firstElementChild?.style.height) || 0) - + VIEWPORT, + 0, + ); +const clampOffset = (element, value) => + Math.min(Math.max(value, 0), maxOffset(element)); +Object.defineProperties(window.HTMLElement.prototype, { + clientHeight: { + configurable: true, + get() { + return isScroller(this) || isScroller(this.firstElementChild) + ? VIEWPORT + : 0; + }, + }, + scrollHeight: { + configurable: true, + get() { + return isScroller(this) ? maxOffset(this) + VIEWPORT : 0; + }, + }, + scrollTop: { + configurable: true, + get() { + const offset = clampOffset(this, offsets.get(this) ?? 0); + offsets.set(this, offset); + return offset; + }, + set(value) { + offsets.set(this, clampOffset(this, value)); + }, + }, + offsetParent: { + configurable: true, + get() { + return this.parentElement; + }, + }, +}); + +const resizeObservers = new Set(); +globalThis.ResizeObserver = window.ResizeObserver = class { + constructor(callback) { + this.callback = callback; + this.targets = new Set(); + resizeObservers.add(this); + } + observe(target) { + this.targets.add(target); + } + unobserve(target) { + this.targets.delete(target); + } + disconnect() { + resizeObservers.delete(this); + } +}; + +// Frames run only when the test flushes them, so it decides whether a queued +// freeze lands before or after the send. +let frames = new Map(); +let nextFrameId = 1; +globalThis.requestAnimationFrame = window.requestAnimationFrame = ( + callback, +) => { + const id = nextFrameId++; + frames.set(id, callback); + return id; +}; +globalThis.cancelAnimationFrame = window.cancelAnimationFrame = (id) => { + frames.delete(id); +}; +function flushFrames() { + act(() => { + const pending = frames; + frames = new Map(); + for (const callback of pending.values()) callback(performance.now()); + }); +} + +function message(index, body = `message ${index}`) { + return { + id: index.toString(16).padStart(64, "0"), + createdAt: 1_700_000_000 + index, + pubkey: "a".repeat(64), + author: "alice", + time: "1:00 AM", + body, + depth: 0, + kind: 9, + tags: [], + }; +} + +const history = Array.from({ length: 20 }, (_, index) => message(index + 1)); +const sent = message(21, "my own send"); + +let root; +let container; +afterEach(() => { + act(() => root?.unmount()); + container?.remove(); +}); + +async function mountAtFloor() { + const router = createRouter({ + routeTree: createRootRoute(), + history: createMemoryHistory({ initialEntries: ["/"] }), + }); + const queryClient = new QueryClient({ + defaultOptions: { queries: { enabled: false } }, + }); + const timeline = React.createRef(); + const render = (messages) => + act(async () => + root.render( + React.createElement( + RouterContextProvider, + { router }, + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(MessageTimeline, { + channelId: "channel", + messages, + ref: timeline, + }), + ), + ), + ), + ); + + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + await render(history); + const scroller = container.querySelector('[data-testid="message-timeline"]'); + act(() => { + for (const observer of resizeObservers) { + observer.callback( + [...observer.targets].map((target) => ({ + target, + contentRect: { + width: 600, + height: target === scroller ? VIEWPORT : ROW, + }, + })), + ); + } + }); + flushFrames(); + readerScrollsTo(scroller, scroller.scrollHeight); + flushFrames(); + return { render, scroller, timeline }; +} + +function readerScrollsTo(scroller, offset) { + act(() => { + scroller.scrollTop = offset; + scroller.dispatchEvent(new window.Event("scroll")); + }); +} + +// The guard cannot tell Virtua's stale gap from a real one, so a real 200px +// gap stands in for it. Either way, the send pulls the view to its row. +for (const { name, order } of [ + { name: "after the send", order: ["send", "gap"] }, + { name: "with its freeze still queued at the send", order: ["gap", "send"] }, +]) { + test(`an own send reaches the floor when Virtua reports a gap ${name}`, async () => { + const { render, scroller, timeline } = await mountAtFloor(); + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]'), + null, + ); + + for (const step of order) { + if (step === "send") { + act(() => timeline.current.scrollToBottomOnNextUpdate()); + } else { + readerScrollsTo(scroller, scroller.scrollTop - 200); + } + } + flushFrames(); + await render([...history, sent]); + flushFrames(); + + assert.ok(container.querySelector(`[data-message-id="${sent.id}"]`)); + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]'), + null, + ); + assert.equal(scroller.scrollTop, scroller.scrollHeight - VIEWPORT); + }); +} diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 3ceeff8f97..2bb13b01d8 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -358,6 +358,7 @@ const MessageTimelineBase = React.forwardRef< onScroll, scrollToBottom, scrollToBottomOnNextUpdate, + isBottomPinArmed, scrollToMessage, onVirtualizerAtBottomStateChange, } = useAnchoredScroll({ @@ -424,13 +425,17 @@ const MessageTimelineBase = React.forwardRef< } } else if (hasConfirmedVirtualizerBottomRef.current) { onVirtualizerAtBottomStateChange(false); - if (semanticAtBottomRef.current) { + // An armed own-send pin owns the next append. Freezing the tail first + // would buffer the sent row, and the pin would never see it arrive. + // Virtua reports a stale gap here when a discarded render already + // counted the sent row in its store. + if (semanticAtBottomRef.current && !isBottomPinArmed()) { suppressNextSemanticBottomRef.current = true; queueSemanticBottom(false); } } }, - [onVirtualizerAtBottomStateChange, queueSemanticBottom], + [isBottomPinArmed, onVirtualizerAtBottomStateChange, queueSemanticBottom], ); const hasPendingLiveRows = bufferedTimeline.pendingCount > 0; React.useEffect(() => { @@ -480,7 +485,13 @@ const MessageTimelineBase = React.forwardRef< const prepareForOwnMessage = React.useCallback(() => { // The user's own send is the deliberate Zulip exception: release buffered // output before arming the next-append bottom pin so the sent row can enter - // Virtua's model and become the new physical floor. + // Virtua's model and become the new physical floor. Drop a freeze queued + // before the send, or it would land after this release. + if (semanticBottomRafRef.current !== null) { + window.cancelAnimationFrame(semanticBottomRafRef.current); + semanticBottomRafRef.current = null; + } + semanticAtBottomRef.current = true; setIsSemanticallyAtBottom(true); scrollToBottomOnNextUpdate(); }, [scrollToBottomOnNextUpdate]); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 63e67ce763..8310ac5c9d 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -88,6 +88,8 @@ type UseAnchoredScrollResult = { /** Arm a one-shot scroll-to-bottom that fires on the next appended message * (used by the composer's send flow). */ scrollToBottomOnNextUpdate: () => void; + /** True while that one-shot is armed and its append has not landed. */ + isBottomPinArmed: () => boolean; /** Imperative: scroll a specific message into view; optionally pulse it. * Returns true if the row was found and scrolled, false otherwise. */ scrollToMessage: ( @@ -445,6 +447,10 @@ export function useAnchoredScroll({ const scrollToBottomOnNextUpdate = React.useCallback(() => { forceBottomOnNextAppendRef.current = true; }, []); + const isBottomPinArmed = React.useCallback( + () => forceBottomOnNextAppendRef.current, + [], + ); const settleAtBottomAfterLayout = React.useCallback(() => { const container = scrollContainerRef.current; @@ -1059,6 +1065,7 @@ export function useAnchoredScroll({ scrollToBottom: scrollToBottomImperative, settleAtBottomAfterLayout, scrollToBottomOnNextUpdate, + isBottomPinArmed, scrollToMessage: scrollToMessageImperative, onVirtualizerAtBottomStateChange, };