diff --git a/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs index 7784bea4b8..82b06cf34b 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs +++ b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs @@ -140,7 +140,7 @@ afterEach(() => { container?.remove(); }); -async function mountAtFloor() { +async function mountAtFloor(initial = history) { const router = createRouter({ routeTree: createRootRoute(), history: createMemoryHistory({ initialEntries: ["/"] }), @@ -171,7 +171,7 @@ async function mountAtFloor() { container = document.createElement("div"); document.body.appendChild(container); root = createRoot(container); - await render(history); + await render(initial); const scroller = container.querySelector('[data-testid="message-timeline"]'); act(() => { for (const observer of resizeObservers) { @@ -231,3 +231,35 @@ for (const { name, order } of [ assert.equal(scroller.scrollTop, scroller.scrollHeight - VIEWPORT); }); } + +// Rows arrive sorted by created_at, so a tail from a sender whose clock runs +// ahead stays the tail when the send lands above it, and the pin stays armed. +// The send's row still ends its hold on freezing: a reader who then scrolls up +// gets the next arrival behind the pill, even one that becomes the tail. +test("a reader who scrolls up after a send that lands above the tail keeps their place", async () => { + const earlier = history.slice(0, -1); + const aheadTail = { ...history.at(-1), createdAt: sent.createdAt + 3600 }; + const aheadNext = { ...message(22), createdAt: aheadTail.createdAt + 1 }; + const { render, scroller, timeline } = await mountAtFloor([ + ...earlier, + aheadTail, + ]); + + act(() => timeline.current.scrollToBottomOnNextUpdate()); + await render([...earlier, sent, aheadTail]); + flushFrames(); + assert.ok(container.querySelector(`[data-message-id="${sent.id}"]`)); + + readerScrollsTo(scroller, scroller.scrollTop - 200); + const readingOffset = scroller.scrollTop; + flushFrames(); + await render([...earlier, sent, aheadTail, aheadNext]); + flushFrames(); + + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]') + ?.textContent, + "1 new message", + ); + assert.equal(scroller.scrollTop, readingOffset); +}); diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 2bb13b01d8..28cdcd63ad 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -358,7 +358,7 @@ const MessageTimelineBase = React.forwardRef< onScroll, scrollToBottom, scrollToBottomOnNextUpdate, - isBottomPinArmed, + isOwnSendPending, scrollToMessage, onVirtualizerAtBottomStateChange, } = useAnchoredScroll({ @@ -425,17 +425,17 @@ const MessageTimelineBase = React.forwardRef< } } else if (hasConfirmedVirtualizerBottomRef.current) { onVirtualizerAtBottomStateChange(false); - // 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()) { + // Until the sent row lands, the 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 && !isOwnSendPending()) { suppressNextSemanticBottomRef.current = true; queueSemanticBottom(false); } } }, - [isBottomPinArmed, onVirtualizerAtBottomStateChange, queueSemanticBottom], + [isOwnSendPending, onVirtualizerAtBottomStateChange, queueSemanticBottom], ); const hasPendingLiveRows = bufferedTimeline.pendingCount > 0; React.useEffect(() => { @@ -486,12 +486,13 @@ const MessageTimelineBase = React.forwardRef< // 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. Drop a freeze queued - // before the send, or it would land after this release. + // before the send, or it would land after this release, and the bottom + // report suppression that freeze armed. if (semanticBottomRafRef.current !== null) { window.cancelAnimationFrame(semanticBottomRafRef.current); semanticBottomRafRef.current = null; } - semanticAtBottomRef.current = true; + suppressNextSemanticBottomRef.current = false; setIsSemanticallyAtBottom(true); scrollToBottomOnNextUpdate(); }, [scrollToBottomOnNextUpdate]); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 8310ac5c9d..e6df41134f 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -88,8 +88,9 @@ 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; + /** True from that arming until the first append lands, whether or not the + * append moves the tail and fires the one-shot. */ + isOwnSendPending: () => boolean; /** Imperative: scroll a specific message into view; optionally pulse it. * Returns true if the row was found and scrolled, false otherwise. */ scrollToMessage: ( @@ -252,6 +253,11 @@ export function useAnchoredScroll({ // appends, we snap to bottom even if they had scrolled up to read history. // Consumed (and cleared) by the next append in the restoration effect. const forceBottomOnNextAppendRef = React.useRef(false); + // Set with the one-shot, but any append clears it, including one that sorts + // above the current tail (a sender whose clock runs ahead, a same-second + // row) and so leaves the one-shot armed for a later tail. MessageTimeline + // holds its tail open only while this is set. + const ownSendPendingRef = React.useRef(false); // True from a programmatic bottom pin until the list's row measurement settles // and the view reaches a true physical bottom. During this window `onScroll` // ignores transient gaps and keeps chasing the floor. A `ref`, not state — the @@ -288,6 +294,7 @@ export function useAnchoredScroll({ prevMessagesRef.current = []; handledTargetIdRef.current = null; forceBottomOnNextAppendRef.current = false; + ownSendPendingRef.current = false; settlingRef.current = false; programmaticScrollTopRef.current = null; isWritingScrollRef.current = false; @@ -446,9 +453,10 @@ export function useAnchoredScroll({ // outbound message pulls the view down even if they'd scrolled up. const scrollToBottomOnNextUpdate = React.useCallback(() => { forceBottomOnNextAppendRef.current = true; + ownSendPendingRef.current = true; }, []); - const isBottomPinArmed = React.useCallback( - () => forceBottomOnNextAppendRef.current, + const isOwnSendPending = React.useCallback( + () => ownSendPendingRef.current, [], ); @@ -739,6 +747,10 @@ export function useAnchoredScroll({ previous: prevMessages, }); const isPrepend = messageDelta === "prepend"; + // The first append ends a pending send, wherever its row sorts. + if (newLatestArrived || messageDelta === "append") { + ownSendPendingRef.current = false; + } // One-shot: an outbound send armed `scrollToBottomOnNextUpdate`. When the // resulting append lands, snap to bottom regardless of the current anchor, @@ -1065,7 +1077,7 @@ export function useAnchoredScroll({ scrollToBottom: scrollToBottomImperative, settleAtBottomAfterLayout, scrollToBottomOnNextUpdate, - isBottomPinArmed, + isOwnSendPending, scrollToMessage: scrollToMessageImperative, onVirtualizerAtBottomStateChange, };