diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 9b384e1a0f..e67ed16105 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -63,6 +63,7 @@ import { useChannelIntro } from "@/features/channels/ui/useChannelIntro"; import type { ChannelPaneProps } from "@/features/channels/ui/ChannelPane.types"; import * as agentSessionSelection from "@/features/channels/ui/agentSessionSelection"; import { usePrepareDmSendChannel } from "@/features/channels/ui/usePrepareDmSendChannel"; +import { useOwnSendArm } from "@/features/channels/ui/useOwnSendArm"; import { useChannelPaneMessages } from "@/features/channels/ui/useChannelPaneMessages"; import { useRoutedMessageEdit } from "@/features/channels/ui/useRoutedMessageEdit"; import { Button } from "@/shared/ui/button"; @@ -280,6 +281,11 @@ export const ChannelPane = React.memo(function ChannelPane({ } return pubkeys; }, [activityAgents, agentPubkeys, agentSessionAgents]); + const getTimelineChannelId = React.useCallback( + () => activeChannelIdRef.current, + [], + ); + const armOwnSend = useOwnSendArm(messageTimelineRef, getTimelineChannelId); const handleSendMessage = React.useCallback( async ( content: string, @@ -296,15 +302,24 @@ export const ChannelPane = React.memo(function ChannelPane({ isActiveWelcomeChannel && (containsWelcomePersonaMention(content) || mentionsKnownAgent(mentionPubkeys, knownAgentPubkeys)); - messageTimelineRef.current?.scrollToBottomOnNextUpdate(); - await onSendMessage( - content, - mentionPubkeys, - mediaTags, - channelId, - threadContext, - forceRest, - ); + // A send prepared into another channel (a DM message that mentions an + // agent outside the DM) lands its row there, so this timeline never sees + // it, and the arm skips it. The pane navigates there once the send + // succeeds. + const disarmOwnSend = armOwnSend(channelId ?? activeChannelId); + try { + await onSendMessage( + content, + mentionPubkeys, + mediaTags, + channelId, + threadContext, + forceRest, + ); + } catch (error) { + disarmOwnSend(); + throw error; + } if ( channelId && channelId !== activeChannelId && @@ -319,6 +334,7 @@ export const ChannelPane = React.memo(function ChannelPane({ }, [ activeChannelId, + armOwnSend, completeWelcomeComposerBanner, goChannel, isActiveWelcomeChannel, @@ -357,11 +373,8 @@ export const ChannelPane = React.memo(function ChannelPane({ [activeChannel, currentPubkey, profiles], ); const handleWelcomeAddAgent = React.useCallback(() => { - onAddAgent?.({ - beforeSend: () => - messageTimelineRef.current?.scrollToBottomOnNextUpdate(), - }); - }, [onAddAgent]); + onAddAgent?.({ beforeSend: armOwnSend }); + }, [armOwnSend, onAddAgent]); const standardChannelIntro = useChannelIntro({ activeChannel, onAddAgent, diff --git a/desktop/src/features/channels/ui/ChannelPane.types.ts b/desktop/src/features/channels/ui/ChannelPane.types.ts index 1fe5bf751b..2141a60131 100644 --- a/desktop/src/features/channels/ui/ChannelPane.types.ts +++ b/desktop/src/features/channels/ui/ChannelPane.types.ts @@ -96,7 +96,9 @@ export type ChannelPaneProps = { onChannelManagementDeleted?: () => void; onCloseIdleAuxiliaryPanel?: () => void; onCloseProfilePanel: () => void; - onAddAgent?: (options?: { beforeSend?: () => void }) => void; + onAddAgent?: (options?: { + beforeSend?: (channelId: string) => () => void; + }) => void; onAddFiles?: () => void; onBrowseChannels?: () => void; onCreateChannel?: () => void; diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index c6a8907896..640a50678d 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -559,7 +559,7 @@ export function ChannelScreen({ ? handleSendVideoReviewComment : undefined; const handleOpenAddBot = React.useCallback( - (options?: { beforeSend?: () => void }) => + (options?: { beforeSend?: (channelId: string) => () => void }) => welcomeAgentCreate.openAddAgent(() => setIsAddBotOpen(true), options), [welcomeAgentCreate], ); diff --git a/desktop/src/features/channels/ui/useChannelIntro.tsx b/desktop/src/features/channels/ui/useChannelIntro.tsx index 3374fb7a65..ac279b1f7a 100644 --- a/desktop/src/features/channels/ui/useChannelIntro.tsx +++ b/desktop/src/features/channels/ui/useChannelIntro.tsx @@ -38,7 +38,9 @@ export function useChannelIntro({ onWelcomeAddAgent, }: { activeChannel: Channel | null; - onAddAgent?: (options?: { beforeSend?: () => void }) => void; + onAddAgent?: (options?: { + beforeSend?: (channelId: string) => () => void; + }) => void; onAddFiles?: () => void; onBrowseChannels?: () => void; onCreateChannel?: () => void; diff --git a/desktop/src/features/channels/ui/useOwnSendArm.test.mjs b/desktop/src/features/channels/ui/useOwnSendArm.test.mjs new file mode 100644 index 0000000000..30e7f968bc --- /dev/null +++ b/desktop/src/features/channels/ui/useOwnSendArm.test.mjs @@ -0,0 +1,85 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { installDOMShim } from "../observedUnreadTestHarness.mjs"; + +installDOMShim(); + +import React from "react"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; + +import { useOwnSendArm } from "./useOwnSendArm.ts"; + +// Mounts the hook over a timeline that records what it was asked to do. The +// pane's channel is read live, as ChannelPane reads activeChannelIdRef, so a +// test can switch it under an arm the composer already holds. +async function mountPane(channelId) { + const calls = []; + const timelineRef = { + current: { + cancelScrollToBottomOnNextUpdate: () => calls.push("disarm"), + scrollToBottomOnNextUpdate: (target) => calls.push(`arm ${target}`), + }, + }; + const pane = { arm: null, calls, channelId }; + const getTimelineChannelId = () => pane.channelId; + function Harness() { + pane.arm = useOwnSendArm(timelineRef, getTimelineChannelId); + return null; + } + const root = createRoot(document.createElement("div")); + await act(async () => root.render(React.createElement(Harness))); + pane.unmount = () => act(async () => root.unmount()); + return pane; +} + +test("a send to the channel the timeline shows arms it", async () => { + const pane = await mountPane("general"); + + pane.arm("general"); + pane.arm(null); + + assert.deepEqual(pane.calls, ["arm general"]); + await pane.unmount(); +}); + +// A send whose upload spans a channel switch settles with the channel it was +// written in. Its arm must not pin or hold the channel shown now, and must not +// take the generation from the send that is pending there. +test("a send that settles after the pane switched channels is not armed", async () => { + const pane = await mountPane("design"); + pane.channelId = "general"; + const disarmPending = pane.arm("general"); + + const disarmLate = pane.arm("design"); + disarmLate(); + assert.deepEqual(pane.calls, ["arm general"]); + + disarmPending(); + assert.deepEqual(pane.calls, ["arm general", "disarm"]); + await pane.unmount(); +}); + +test("a failed send disarms the timeline", async () => { + const pane = await mountPane("general"); + + const disarm = pane.arm("general"); + disarm(); + + assert.deepEqual(pane.calls, ["arm general", "disarm"]); + await pane.unmount(); +}); + +test("a later send's arm outlives an earlier send's failure", async () => { + const pane = await mountPane("general"); + + const disarmEarlier = pane.arm("general"); + const disarmLater = pane.arm("general"); + disarmEarlier(); + assert.deepEqual(pane.calls, ["arm general", "arm general"]); + + disarmLater(); + assert.deepEqual(pane.calls, ["arm general", "arm general", "disarm"]); + await pane.unmount(); +}); diff --git a/desktop/src/features/channels/ui/useOwnSendArm.ts b/desktop/src/features/channels/ui/useOwnSendArm.ts new file mode 100644 index 0000000000..8850d56a4c --- /dev/null +++ b/desktop/src/features/channels/ui/useOwnSendArm.ts @@ -0,0 +1,41 @@ +import * as React from "react"; + +import type { MessageTimelineHandle } from "@/features/messages/ui/MessageTimeline"; + +type OwnSendTimeline = Pick< + MessageTimelineHandle, + "cancelScrollToBottomOnNextUpdate" | "scrollToBottomOnNextUpdate" +>; + +const noDisarm = () => {}; + +/** + * Arms the timeline's own-send pin for a send to `channelId` and returns its + * disarm for a failed send. + * + * A send can settle after the pane moved to another channel (a slow upload + * spans the switch), so the arm is skipped unless `channelId` is the channel + * the timeline shows now, read live from `getTimelineChannelId`. A skipped arm + * leaves the generation alone, so it can't strand an earlier send's disarm. + * The generation keeps a late failure from disarming a later send's pin. + */ +export function useOwnSendArm( + timelineRef: React.RefObject, + getTimelineChannelId: () => string | null, +) { + const generationRef = React.useRef(0); + return React.useCallback( + (channelId: string | null) => { + if (!channelId || channelId !== getTimelineChannelId()) { + return noDisarm; + } + const generation = ++generationRef.current; + timelineRef.current?.scrollToBottomOnNextUpdate(channelId); + return () => { + if (generationRef.current !== generation) return; + timelineRef.current?.cancelScrollToBottomOnNextUpdate(); + }; + }, + [getTimelineChannelId, timelineRef], + ); +} diff --git a/desktop/src/features/channels/useWelcomeAgentCreate.ts b/desktop/src/features/channels/useWelcomeAgentCreate.ts index cf102b9fbc..97682ed9aa 100644 --- a/desktop/src/features/channels/useWelcomeAgentCreate.ts +++ b/desktop/src/features/channels/useWelcomeAgentCreate.ts @@ -26,10 +26,17 @@ export function useWelcomeAgentCreate({ ); const [isOpen, setIsOpen] = React.useState(false); const [error, setError] = React.useState(null); - const beforeSendRef = React.useRef<(() => void) | null>(null); + // Runs right before the chat-first send, with the channel it sends to, and + // returns how to undo it if the send fails. + const beforeSendRef = React.useRef< + ((channelId: string) => () => void) | null + >(null); const openAddAgent = React.useCallback( - (openRegularPicker: () => void, options?: { beforeSend?: () => void }) => { + ( + openRegularPicker: () => void, + options?: { beforeSend?: (channelId: string) => () => void }, + ) => { setError(null); if (isWelcomeExperienceChannel(activeChannel)) { beforeSendRef.current = options?.beforeSend ?? null; @@ -59,8 +66,9 @@ export function useWelcomeAgentCreate({ return; } setError(null); + let undoBeforeSend: (() => void) | undefined; try { - beforeSendRef.current?.(); + undoBeforeSend = beforeSendRef.current?.(activeChannel.id); await sendMessageMutation.mutateAsync({ channelId: activeChannel.id, content: `@${welcomeGuideAgent.name}, help me create a new agent.`, @@ -68,6 +76,7 @@ export function useWelcomeAgentCreate({ }); setIsOpen(false); } catch (cause) { + undoBeforeSend?.(); setError( cause instanceof Error ? cause.message 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..ef1b435e0a --- /dev/null +++ b/desktop/src/features/messages/ui/MessageTimeline.ownSend.jsdom-test.mjs @@ -0,0 +1,387 @@ +/** + * 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(initial = history) { + 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(initial); + 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("channel")); + } 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); + }); +} + +// 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("channel")); + 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); +}); + +// A failed send can leave no row here, as when its optimistic row never +// resolved a channel. Its cancel drops the pin and the hold, and freezes the +// tail the hold left open if the reader is off the floor, so the next arrival +// waits behind the pill instead of firing the pin under the reader. +for (const order of [ + ["send", "fail", "gap"], + ["send", "gap", "fail"], + ["gap", "send", "fail"], +]) { + test(`a send that fails without a row leaves the reader in place (${order.join(", ")})`, async () => { + const { render, scroller, timeline } = await mountAtFloor(); + + for (const step of order) { + if (step === "send") { + act(() => timeline.current.scrollToBottomOnNextUpdate("channel")); + } else if (step === "fail") { + act(() => timeline.current.cancelScrollToBottomOnNextUpdate()); + } else { + readerScrollsTo(scroller, scroller.scrollTop - 200); + } + } + const readingOffset = scroller.scrollTop; + flushFrames(); + await render([...history, message(22, "from the other person")]); + flushFrames(); + + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]') + ?.textContent, + "1 new message", + ); + assert.equal(scroller.scrollTop, readingOffset); + }); +} + +// With the reader still at the floor, the hold skipped nothing, so the cancel +// leaves the tail open and the next arrival renders at once. A freeze here +// would only be released a frame later, holding that arrival behind the pill +// until then. +test("a send that fails at the floor keeps the timeline following", async () => { + const { render, timeline } = await mountAtFloor(); + const arrival = message(22, "from the other person"); + + act(() => timeline.current.scrollToBottomOnNextUpdate("channel")); + act(() => timeline.current.cancelScrollToBottomOnNextUpdate()); + flushFrames(); + await render([...history, arrival]); + + assert.ok(container.querySelector(`[data-message-id="${arrival.id}"]`)); + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]'), + null, + ); +}); + +// A send to this channel lands its optimistic row at once, which is an append +// that fires the pin and ends the hold. When the publish then fails, the row +// is reverted and the cancel finds nothing pending: it must not re-arm the +// freeze's bottom-report suppression, or returning to the floor would leave +// the arrival stuck behind the pill. +test("a reader who scrolls up before a same-channel send fails keeps their place", async () => { + const { render, scroller, timeline } = await mountAtFloor(); + + act(() => timeline.current.scrollToBottomOnNextUpdate("channel")); + await render([...history, sent]); + flushFrames(); + assert.equal(scroller.scrollTop, scroller.scrollHeight - VIEWPORT); + + readerScrollsTo(scroller, scroller.scrollTop - 200); + const readingOffset = scroller.scrollTop; + flushFrames(); + act(() => timeline.current.cancelScrollToBottomOnNextUpdate()); + await render(history); + flushFrames(); + const arrival = message(22, "from the other person"); + await render([...history, arrival]); + flushFrames(); + + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]') + ?.textContent, + "1 new message", + ); + assert.equal(scroller.scrollTop, readingOffset); + + readerScrollsTo(scroller, scroller.scrollHeight); + flushFrames(); + assert.ok(container.querySelector(`[data-message-id="${arrival.id}"]`)); + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]'), + null, + ); +}); + +// A send whose upload spans a channel switch settles with the channel it was +// written in. Its arm must not release this timeline's frozen tail or pin its +// next arrival under a reader who scrolled up. +test("an own send for another channel leaves this timeline's reader in place", async () => { + const { render, scroller, timeline } = await mountAtFloor(); + readerScrollsTo(scroller, scroller.scrollTop - 200); + const readingOffset = scroller.scrollTop; + flushFrames(); + await render([...history, message(22, "from the other person")]); + flushFrames(); + + act(() => timeline.current.scrollToBottomOnNextUpdate("design")); + flushFrames(); + await render([ + ...history, + message(22, "from the other person"), + message(23, "and again"), + ]); + flushFrames(); + + assert.equal( + container.querySelector('[data-testid="message-scroll-to-latest"]') + ?.textContent, + "2 new messages", + ); + assert.equal(scroller.scrollTop, readingOffset); +}); diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 3ceeff8f97..1eed795c5d 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -34,7 +34,11 @@ import { import { useSettleGatedPrependMessages } from "./useSettleGatedPrependMessages"; export type MessageTimelineHandle = { - scrollToBottomOnNextUpdate: () => void; + /** Release the frozen tail and pin the next append for a send to + * `sendChannelId`. Ignored when this timeline shows another channel. */ + scrollToBottomOnNextUpdate: (sendChannelId: string) => void; + /** Undo `scrollToBottomOnNextUpdate` for a send that failed. */ + cancelScrollToBottomOnNextUpdate: () => void; settleAtBottom: () => boolean; }; @@ -358,6 +362,9 @@ const MessageTimelineBase = React.forwardRef< onScroll, scrollToBottom, scrollToBottomOnNextUpdate, + isOwnSendPending, + isVirtualizerAtBottom, + cancelScrollToBottomOnNextUpdate, scrollToMessage, onVirtualizerAtBottomStateChange, } = useAnchoredScroll({ @@ -424,13 +431,17 @@ const MessageTimelineBase = React.forwardRef< } } else if (hasConfirmedVirtualizerBottomRef.current) { onVirtualizerAtBottomStateChange(false); - if (semanticAtBottomRef.current) { + // 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); } } }, - [onVirtualizerAtBottomStateChange, queueSemanticBottom], + [isOwnSendPending, onVirtualizerAtBottomStateChange, queueSemanticBottom], ); const hasPendingLiveRows = bufferedTimeline.pendingCount > 0; React.useEffect(() => { @@ -477,25 +488,62 @@ const MessageTimelineBase = React.forwardRef< const showPinnedIntroOnly = activePinnedIntro !== null && !showMessageList; const omitHistoryLeadIn = showChannelIntroOnly || showPinnedIntroOnly; - 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. - setIsSemanticallyAtBottom(true); - scrollToBottomOnNextUpdate(); - }, [scrollToBottomOnNextUpdate]); + const prepareForOwnMessage = React.useCallback( + (sendChannelId: string) => { + // A send that settles after a channel switch belongs to the channel it + // left, so it must neither release nor pin the one shown now. + if (!scrollToBottomOnNextUpdate(sendChannelId)) return; + // The user's own send is the deliberate Zulip exception: release buffered + // output along with 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, and the bottom + // report suppression that freeze armed. + if (semanticBottomRafRef.current !== null) { + window.cancelAnimationFrame(semanticBottomRafRef.current); + semanticBottomRafRef.current = null; + } + suppressNextSemanticBottomRef.current = false; + setIsSemanticallyAtBottom(true); + }, + [scrollToBottomOnNextUpdate], + ); + const cancelOwnMessage = React.useCallback(() => { + // While the send was pending, its hold skipped any freeze, including one + // queued before the send. If the reader is off the floor now, freeze as + // that report would have, so later arrivals wait behind the pill. + const skippedFreeze = + isOwnSendPending() && + hasConfirmedVirtualizerBottomRef.current && + !isVirtualizerAtBottom(); + cancelScrollToBottomOnNextUpdate(); + if (skippedFreeze) { + suppressNextSemanticBottomRef.current = true; + queueSemanticBottom(false); + } + }, [ + cancelScrollToBottomOnNextUpdate, + isOwnSendPending, + isVirtualizerAtBottom, + queueSemanticBottom, + ]); React.useImperativeHandle( ref, () => ({ scrollToBottomOnNextUpdate: prepareForOwnMessage, + cancelScrollToBottomOnNextUpdate: cancelOwnMessage, settleAtBottom: () => { if (!timelineVirtualizerApi) return false; scrollToBottom("auto"); return true; }, }), - [prepareForOwnMessage, scrollToBottom, timelineVirtualizerApi], + [ + cancelOwnMessage, + prepareForOwnMessage, + scrollToBottom, + timelineVirtualizerApi, + ], ); const jumpToMessage = React.useCallback( diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index dd0f4160d7..d829717709 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -918,7 +918,7 @@ const RELEASES = [ [ "own send", async ({ fake, render, state }) => { - state().scrollToBottomOnNextUpdate(); + state().scrollToBottomOnNextUpdate("conversation"); fake.messages = [...fake.messages, { createdAt: 600, id: "sent" }]; await render({}); }, @@ -1085,3 +1085,200 @@ test("reader input at the floor resumes following live messages", async () => { assert.deepEqual(held.settles, ["settle"]); await act(async () => held.root.unmount()); }); + +// A failed send may add no row, so its cancel must drop the one-shot and the +// pending send. The next tail is then someone else's arrival and must not pull +// a reader who left the floor, while a later send still pins. +test("a cancelled own send leaves the next tail with the reader", async () => { + globalThis.ResizeObserver = class { + disconnect() {} + observe() {} + }; + const scroller = document.createElement("div"); + scroller.querySelector = () => null; + const settles = []; + const fake = { + content: { current: null }, + messages: [{ createdAt: 500, id: "latest" }], + scrollToBottom: () => settles.push("jump"), + scrollToMessage: () => true, + scroller: { current: scroller }, + settleAtBottom: () => settles.push("settle"), + }; + let state = null; + const root = createRoot(document.createElement("div")); + const render = () => + act(async () => + root.render( + React.createElement(TargetHoldHarness, { + fake, + onState: (next) => { + state = next; + }, + }), + ), + ); + await render(); + // Let the mount's settling frame run, or its pin lands after the clear. + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + await act(async () => state.onVirtualizerAtBottomStateChange(true)); + settles.length = 0; // the mount's own bottom pins + + state.scrollToBottomOnNextUpdate("conversation"); + assert.equal(state.isOwnSendPending(), true); + state.cancelScrollToBottomOnNextUpdate(); + assert.equal(state.isOwnSendPending(), false); + + await act(async () => state.onVirtualizerAtBottomStateChange(false)); + fake.messages = [...fake.messages, { createdAt: 600, id: "live" }]; + await render(); + assert.deepEqual(settles, [], "the arrival does not move the reader"); + assert.equal(state.isAtBottom, false); + + state.scrollToBottomOnNextUpdate("conversation"); + fake.messages = [...fake.messages, { createdAt: 700, id: "sent" }]; + await render(); + assert.deepEqual(settles, ["jump"], "a later send still pins"); + assert.equal(state.isOwnSendPending(), false); + await act(async () => root.unmount()); +}); + +// MessageTimeline's cancel of a failed send freezes the tail only when both +// gates read through: the send is still pending (its row never landed, so the +// hold may have skipped a freeze), and the virtualizer last reported the reader +// off the floor. An arm for a channel this hook doesn't show is refused, as a +// send that settles after a channel switch would make one. +const OWN_SEND_STEPS = { + "send here": ({ armed, state }) => + armed.push(state().scrollToBottomOnNextUpdate("conversation")), + "send to other": ({ armed, state }) => + armed.push(state().scrollToBottomOnNextUpdate("other")), + "row as tail": ({ fake, render }) => { + fake.messages = [...fake.messages, { createdAt: 600, id: "sent" }]; + return render(); + }, + "row above tail": ({ fake, render }) => { + const [first, ...rest] = fake.messages; + fake.messages = [first, { createdAt: 400, id: "sent" }, ...rest]; + return render(); + }, + "channel change": ({ render }) => render({ channelId: "other" }), + floor: ({ state }) => + act(async () => state().onVirtualizerAtBottomStateChange(true)), + "off floor": ({ state }) => + act(async () => state().onVirtualizerAtBottomStateChange(false)), +}; + +const OWN_SEND_GATES = [ + ["nothing sent", [], { armed: [], atBottom: true, pending: false }], + [ + "a send whose row has not landed", + ["send here"], + { armed: [true], atBottom: true, pending: true }, + ], + [ + "a send whose row landed as the tail", + ["send here", "row as tail"], + { armed: [true], atBottom: true, pending: false }, + ], + [ + "a send whose row landed above the tail", + ["send here", "row above tail"], + { armed: [true], atBottom: true, pending: false }, + ], + [ + "a send to another channel", + ["send to other"], + { armed: [false], atBottom: true, pending: false }, + ], + [ + "a send, then a channel change", + ["send here", "channel change"], + { armed: [true], atBottom: true, pending: false }, + ], + [ + "a channel change, then a send there", + ["channel change", "send to other"], + { armed: [true], atBottom: true, pending: true }, + ], + [ + "the reader off the floor", + ["floor", "off floor"], + { armed: [], atBottom: false, pending: false }, + ], + [ + "the reader back at the floor", + ["floor", "off floor", "floor"], + { armed: [], atBottom: true, pending: false }, + ], + [ + "a channel change off the floor", + ["floor", "off floor", "channel change"], + { armed: [], atBottom: true, pending: false }, + ], + [ + "a pending send with the reader off the floor", + ["floor", "send here", "off floor"], + { armed: [true], atBottom: false, pending: true }, + ], +]; + +for (const [name, steps, expected] of OWN_SEND_GATES) { + test(`own-send gates: ${name}`, async () => { + globalThis.ResizeObserver = class { + disconnect() {} + observe() {} + }; + const scroller = document.createElement("div"); + scroller.querySelector = () => null; + const fake = { + content: { current: null }, + messages: [ + { createdAt: 100, id: "first" }, + { createdAt: 500, id: "latest" }, + ], + scrollToBottom: () => {}, + scrollToMessage: () => true, + scroller: { current: scroller }, + settleAtBottom: () => {}, + }; + let state = null; + let props = {}; + const root = createRoot(document.createElement("div")); + const render = (next = {}) => { + props = { ...props, ...next }; + return act(async () => + root.render( + React.createElement(TargetHoldHarness, { + fake, + onState: (current) => { + state = current; + }, + ...props, + }), + ), + ); + }; + await render(); + + const results = []; + for (const step of steps) { + await OWN_SEND_STEPS[step]({ + armed: results, + fake, + render, + state: () => state, + }); + } + + assert.deepEqual( + { + armed: results, + atBottom: state.isVirtualizerAtBottom(), + pending: state.isOwnSendPending(), + }, + expected, + ); + await act(async () => root.unmount()); + }); +} diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 63e67ce763..8b68472be0 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -86,8 +86,18 @@ type UseAnchoredScrollResult = { * the hook handled the settlement, including a preserved pinned target. */ settleAtBottomAfterLayout: () => boolean; /** Arm a one-shot scroll-to-bottom that fires on the next appended message - * (used by the composer's send flow). */ - scrollToBottomOnNextUpdate: () => void; + * (used by the composer's send flow), for a send to `sendChannelId`. + * Returns false without arming when this timeline shows another channel. */ + scrollToBottomOnNextUpdate: (sendChannelId: string) => 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; + /** The virtualizer's last bottom report, cleared early by a target jump or + * a live row under a held target, and reset by a channel change. */ + isVirtualizerAtBottom: () => boolean; + /** Disarm that one-shot and the pending send without moving the view, for + * a send that failed. */ + cancelScrollToBottomOnNextUpdate: () => void; /** Imperative: scroll a specific message into view; optionally pulse it. * Returns true if the row was found and scrolled, false otherwise. */ scrollToMessage: ( @@ -250,6 +260,15 @@ 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); + // The channel the state above belongs to, updated when the reset below + // runs. A send can settle after a channel switch, and its arm must not land + // on the channel shown now. + const channelIdRef = React.useRef(channelId); // 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 @@ -271,8 +290,8 @@ export function useAnchoredScroll({ // Reset everything when the channel changes — the layout effect that runs // immediately after this reset is responsible for either jumping to bottom // or to the target message for the new channel. - // biome-ignore lint/correctness/useExhaustiveDependencies: channelId is intentionally the sole trigger — we want this effect to fire exactly when the channel changes (and on mount). React.useLayoutEffect(() => { + channelIdRef.current = channelId; anchorRef.current = { kind: "at-bottom" }; virtualizerAtBottomRef.current = true; targetHoldRef.current = false; @@ -286,6 +305,7 @@ export function useAnchoredScroll({ prevMessagesRef.current = []; handledTargetIdRef.current = null; forceBottomOnNextAppendRef.current = false; + ownSendPendingRef.current = false; settlingRef.current = false; programmaticScrollTopRef.current = null; isWritingScrollRef.current = false; @@ -442,8 +462,28 @@ export function useAnchoredScroll({ // Arm a one-shot: the next append snaps to bottom regardless of where the // user is. The consumer calls this right before sending so their own // outbound message pulls the view down even if they'd scrolled up. - const scrollToBottomOnNextUpdate = React.useCallback(() => { - forceBottomOnNextAppendRef.current = true; + const scrollToBottomOnNextUpdate = React.useCallback( + (sendChannelId: string) => { + if (sendChannelId !== channelIdRef.current) return false; + forceBottomOnNextAppendRef.current = true; + ownSendPendingRef.current = true; + return true; + }, + [], + ); + const isOwnSendPending = React.useCallback( + () => ownSendPendingRef.current, + [], + ); + const isVirtualizerAtBottom = React.useCallback( + () => virtualizerAtBottomRef.current, + [], + ); + // A send that fails may never add a row here, so nothing else would clear + // these. + const cancelScrollToBottomOnNextUpdate = React.useCallback(() => { + forceBottomOnNextAppendRef.current = false; + ownSendPendingRef.current = false; }, []); const settleAtBottomAfterLayout = React.useCallback(() => { @@ -733,6 +773,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, @@ -1059,6 +1103,9 @@ export function useAnchoredScroll({ scrollToBottom: scrollToBottomImperative, settleAtBottomAfterLayout, scrollToBottomOnNextUpdate, + isOwnSendPending, + isVirtualizerAtBottom, + cancelScrollToBottomOnNextUpdate, scrollToMessage: scrollToMessageImperative, onVirtualizerAtBottomStateChange, };