From 1507ac5064a78f2dbfeddd76ecde787c7c7cfafd Mon Sep 17 00:00:00 2001 From: Ian Oberst Date: Wed, 22 Jul 2026 09:29:33 -0700 Subject: [PATCH 1/2] fix(desktop): scroll threads to first unread Co-authored-by: Ian Oberst Signed-off-by: Ian Oberst Co-authored-by: Codex Ai-assisted: true --- .../features/channels/ui/ChannelScreen.tsx | 20 +++--- .../channels/ui/useThreadOpenScrollTarget.ts | 45 +++++++++++++ desktop/tests/e2e/thread-unread.spec.ts | 65 +++++++++++++++---- 3 files changed, 109 insertions(+), 21 deletions(-) create mode 100644 desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 840d0b5d2b..2d1f8343f1 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -78,6 +78,7 @@ import { useMessageProfiles } from "./useMessageProfiles"; import { useChannelPanelHistoryState } from "./useChannelPanelHistoryState"; import { useChannelProfilePanel } from "./useChannelProfilePanel"; import { useChannelRouteTarget } from "./useChannelRouteTarget"; +import { useThreadOpenScrollTarget } from "./useThreadOpenScrollTarget"; import { useChannelUnreadState } from "./useChannelUnreadState"; import type { ChannelScreenProps } from "./ChannelScreen.types"; const HEADER_ACTIONS_COMPACT_BREAKPOINT_PX = 760, @@ -143,9 +144,6 @@ export function ChannelScreen({ const [expandedThreadReplyIds, setExpandedThreadReplyIds] = React.useState( () => new Set(), ); - const [threadScrollTargetId, setThreadScrollTargetId] = React.useState< - string | null - >(null); const [threadReplyTargetId, setThreadReplyTargetId] = React.useState< string | null >(null); @@ -476,6 +474,12 @@ export function ChannelScreen({ isThreadMuted, readStateVersion, }); + const [threadScrollTargetId, setThreadScrollTargetId, clearThreadTarget] = + useThreadOpenScrollTarget( + effectiveOpenThreadHeadId, + threadFirstUnreadReplyId, + threadRepliesQuery.isFetching, + ); const editTargetMessage = React.useMemo( () => timelineMessages.find((message) => message.id === editTargetId) ?? null, @@ -642,11 +646,8 @@ export function ChannelScreen({ setThreadReplyTargetId(null); setEditTargetId(null); }, - [], + [setThreadScrollTargetId], ); - const handleThreadScrollTargetResolved = React.useCallback(() => { - setThreadScrollTargetId(null); - }, []); const handleTargetReached = React.useCallback(() => { clearMessageRouteTarget({ replace: true }); }, [clearMessageRouteTarget]); @@ -741,6 +742,7 @@ export function ChannelScreen({ setOpenThreadHeadId, handleCloseAgentSession, setProfilePanelPubkey, + setThreadScrollTargetId, ]); const handleToggleMembers = React.useCallback( () => setIsMembersSidebarOpen((prev) => !prev), @@ -922,9 +924,7 @@ export function ChannelScreen({ onSendMessage={handleSendMessage} onSendVideoReviewComment={effectiveSendVideoReviewComment} onSendThreadReply={handleSendThreadReply} - onThreadScrollTargetResolved={ - handleThreadScrollTargetResolved - } + onThreadScrollTargetResolved={clearThreadTarget} onThreadPanelResizeStart={handleThreadPanelResizeStart} onTargetReached={handleTargetReached} onToggleReaction={effectiveToggleReaction} diff --git a/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts b/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts new file mode 100644 index 0000000000..7066f5ba43 --- /dev/null +++ b/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts @@ -0,0 +1,45 @@ +import * as React from "react"; + +export function useThreadOpenScrollTarget( + threadHeadId: string | null, + firstUnreadReplyId: string | null, + isFetchingReplies: boolean, +) { + const [scrollTargetId, setScrollTargetId] = React.useState( + null, + ); + const latchedForHeadRef = React.useRef(null); + + React.useEffect(() => { + if (!threadHeadId) { + latchedForHeadRef.current = null; + return; + } + if (latchedForHeadRef.current === threadHeadId) { + return; + } + + // Explicit route/deep-link targets take precedence over the unread anchor. + if (scrollTargetId !== null) { + latchedForHeadRef.current = threadHeadId; + return; + } + + // A cached thread has data while its fresh subtree is still loading. Wait + // for that refresh so a stale all-read snapshot cannot latch too early. + if (isFetchingReplies) { + return; + } + + latchedForHeadRef.current = threadHeadId; + if (firstUnreadReplyId) { + setScrollTargetId(firstUnreadReplyId); + } + }, [firstUnreadReplyId, isFetchingReplies, scrollTargetId, threadHeadId]); + + const clearScrollTarget = React.useCallback(() => { + setScrollTargetId(null); + }, []); + + return [scrollTargetId, setScrollTargetId, clearScrollTarget] as const; +} diff --git a/desktop/tests/e2e/thread-unread.spec.ts b/desktop/tests/e2e/thread-unread.spec.ts index 586ca3dd12..ec3c53b1f2 100644 --- a/desktop/tests/e2e/thread-unread.spec.ts +++ b/desktop/tests/e2e/thread-unread.spec.ts @@ -109,6 +109,39 @@ async function expandReply( await expect.poll(() => replies.count()).toBeGreaterThan(before); } +async function expectAtBottom( + scrollRegion: import("@playwright/test").Locator, +) { + await expect + .poll(() => + scrollRegion.evaluate( + (element) => + element.scrollHeight - element.clientHeight - element.scrollTop, + ), + ) + .toBeLessThanOrEqual(1); +} + +async function expectInsideScrollRegion( + target: import("@playwright/test").Locator, + scrollRegion: import("@playwright/test").Locator, +) { + await expect + .poll(async () => { + const [targetBox, regionBox] = await Promise.all([ + target.boundingBox(), + scrollRegion.boundingBox(), + ]); + return Boolean( + targetBox && + regionBox && + targetBox.y >= regionBox.y && + targetBox.y + targetBox.height <= regionBox.y + regionBox.height, + ); + }) + .toBe(true); +} + test.describe("thread unread indicator", () => { test("01-thread-unread-badge", async ({ page }) => { await installMockBridge(page); @@ -175,12 +208,16 @@ test.describe("thread unread indicator", () => { await expect(page.getByTestId("chat-title")).toHaveText("general"); await waitForMockLiveSubscription(page, "general"); - // Emit an initial reply so the thread summary appears - await emitMockMessage(page, "general", "Earlier reply", { - parentEventId: "mock-general-welcome", - pubkey: TEST_IDENTITIES.alice.pubkey, - createdAt: Math.floor(Date.now() / 1000) - 10, - }); + // Fill more than one panel so first-unread and bottom are observably + // different positions. + const earlierBase = Math.floor(Date.now() / 1000) - 30; + for (let i = 0; i < 12; i++) { + await emitMockMessage(page, "general", `Earlier reply ${i + 1}`, { + parentEventId: "mock-general-welcome", + pubkey: TEST_IDENTITIES.alice.pubkey, + createdAt: earlierBase + i, + }); + } // Open thread to establish frontier, then close const threadSummary = page.getByTestId("message-thread-summary").first(); @@ -190,13 +227,20 @@ test.describe("thread unread indicator", () => { await page.getByTestId("auxiliary-panel-close").click(); await expect(page.getByTestId("message-thread-panel")).not.toBeVisible(); + // Re-opening with every reply read keeps the existing bottom behavior. + await threadSummary.click(); + const threadBody = page.getByTestId("message-thread-body"); + await expectAtBottom(threadBody); + await page.getByTestId("auxiliary-panel-close").click(); + await expect(page.getByTestId("message-thread-panel")).not.toBeVisible(); + // Switch away await page.getByTestId("channel-random").click(); await expect(page.getByTestId("chat-title")).toHaveText("random"); // Emit new unread replies const base = unreadTimestamp(); - for (let i = 0; i < 2; i++) { + for (let i = 0; i < 12; i++) { await emitMockMessage(page, "general", `New reply ${i + 1}`, { parentEventId: "mock-general-welcome", pubkey: TEST_IDENTITIES.alice.pubkey, @@ -210,12 +254,11 @@ test.describe("thread unread indicator", () => { await page.getByTestId("message-thread-summary").first().click(); await expect(page.getByTestId("message-thread-panel")).toBeVisible(); - // The unread divider should appear above the first unread reply - // (not at index 0 since there's a read reply before the unread ones) + // The unread divider should already be visible above the first unread + // reply. Do not scroll it into view: that would hide a bottom-pin failure. const divider = page.getByTestId("message-unread-divider"); await expect(divider).toBeVisible(); - await divider.scrollIntoViewIfNeeded(); - await page.waitForTimeout(300); + await expectInsideScrollRegion(divider, threadBody); }); test("03-thread-badge-casual-browse", async ({ page }) => { From aacb91f5768b066314653062005da226a79baa04 Mon Sep 17 00:00:00 2001 From: Ian Oberst Date: Wed, 22 Jul 2026 13:10:22 -0700 Subject: [PATCH 2/2] fix(desktop): stabilize inbox thread opening Capture the Inbox read frontier before selection advances it, then keep the resulting first-unread or bottom target stable across conversation swaps and delayed layout. Co-authored-by: Ian Oberst Signed-off-by: Ian Oberst Co-authored-by: Codex Ai-assisted: true --- .../src/features/channels/ui/ChannelPane.tsx | 18 +- .../features/channels/ui/ChannelPane.types.ts | 6 +- .../features/channels/ui/ChannelScreen.tsx | 5 +- .../channels/ui/useThreadOpenScrollTarget.ts | 71 ++- .../channels/ui/useThreadViewModeSwitch.ts | 11 +- .../home/lib/inboxOpenScroll.test.mjs | 65 +++ .../src/features/home/lib/inboxOpenScroll.ts | 45 ++ desktop/src/features/home/ui/HomeView.tsx | 33 +- .../src/features/home/ui/InboxDetailPane.tsx | 92 +++- .../src/features/home/useInboxRowSelection.ts | 87 ++++ .../features/home/useInboxThreadContext.ts | 21 +- .../messages/ui/MessageThreadPanel.tsx | 4 + .../messages/ui/anchoredScrollTarget.ts | 85 ++++ .../ui/useAnchoredScroll.lifecycle.test.mjs | 433 +++++++++++++++++- .../features/messages/ui/useAnchoredScroll.ts | 257 +++++++---- desktop/tests/e2e/inbox-live-update.spec.ts | 391 ++++++++++++++-- desktop/tests/e2e/thread-unread.spec.ts | 42 +- 17 files changed, 1461 insertions(+), 205 deletions(-) create mode 100644 desktop/src/features/home/lib/inboxOpenScroll.test.mjs create mode 100644 desktop/src/features/home/lib/inboxOpenScroll.ts create mode 100644 desktop/src/features/home/useInboxRowSelection.ts create mode 100644 desktop/src/features/messages/ui/anchoredScrollTarget.ts diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 121e211f47..bf7382a3ec 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -153,7 +153,7 @@ export const ChannelPane = React.memo(function ChannelPane({ threadMessages, threadMessagesPending = false, threadPanelWidthPx, - threadScrollTargetId, + threadScrollTarget, threadTypingPubkeys, threadReplyTargetMessage, threadUnreadCounts, @@ -514,19 +514,18 @@ export const ChannelPane = React.memo(function ChannelPane({ const isOverlay = useIsThreadPanelOverlay(); const useSplitAuxiliaryPane = !isSinglePanelView && !isOverlay; const threadViewMode = useThreadViewMode(); - // Focus mode is a wide-viewport-only alternative to the split thread pane: - // narrow viewports keep their existing single-panel / floating-overlay - // behavior untouched. It applies to the thread panel only — channel - // management, agent session and profile panels always use the split pane. + // Focus is a wide-only thread alternative; narrow and non-thread panels keep + // their existing single-panel, overlay, or split-pane behavior. const useFocusThreadDrawer = threadViewMode === "focus" && useSplitAuxiliaryPane && (Boolean(threadHeadMessage) || shouldShowThreadSkeleton); const { channelIsCovered, markExitComplete } = useFocusDrawerPresence(useFocusThreadDrawer); - const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } = + const { changeThreadViewMode, resolveScrollTarget, scrollTarget } = useThreadViewModeSwitch({ - externalScrollTargetId: threadScrollTargetId, + externalScrollTargetAlignment: threadScrollTarget.alignment, + externalScrollTargetId: threadScrollTarget.id, onExternalTargetResolved: onThreadScrollTargetResolved, onModeChange: markExitComplete, }); @@ -886,8 +885,9 @@ export const ChannelPane = React.memo(function ChannelPane({ onUnfollowThread={onUnfollowThread} profiles={profiles} replyTargetMessage={threadReplyTargetMessage} - scrollTargetHighlights={!layoutScrollTargetId} - scrollTargetId={layoutScrollTargetId ?? threadScrollTargetId} + scrollTargetHighlights={!scrollTarget.isLayout} + scrollTargetAlignment={scrollTarget.alignment} + scrollTargetId={scrollTarget.id} threadHead={threadHeadMessage} threadHeadVideoReviewContext={threadHeadVideoReviewContext} widthPx={threadPanelWidthPx} diff --git a/desktop/src/features/channels/ui/ChannelPane.types.ts b/desktop/src/features/channels/ui/ChannelPane.types.ts index 5a27d85c4d..cf954a7a5e 100644 --- a/desktop/src/features/channels/ui/ChannelPane.types.ts +++ b/desktop/src/features/channels/ui/ChannelPane.types.ts @@ -5,6 +5,7 @@ import type { ImetaMedia } from "@/features/messages/lib/imetaMediaMarkdown"; import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; import type { ChannelWindowThreadSummary } from "@/features/messages/lib/channelWindowStore"; import type { TimelineMessage } from "@/features/messages/types"; +import type { ScrollTargetAlignment } from "@/features/messages/ui/anchoredScrollTarget"; import type { TypingIndicatorEntry } from "@/features/messages/useChannelTyping"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import type { useChannelFind } from "@/features/search/useChannelFind"; @@ -156,7 +157,10 @@ export type ChannelPaneProps = { threadPanelWidthPx: number; threadTypingPubkeys: string[]; threadReplyTargetMessage: TimelineMessage | null; - threadScrollTargetId: string | null; + threadScrollTarget: { + alignment: ScrollTargetAlignment; + id: string | null; + }; threadUnreadCounts?: ReadonlyMap; threadReplyUnreadCounts?: ReadonlyMap; threadFirstUnreadReplyId?: string | null; diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 2d1f8343f1..388a054d7d 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -474,10 +474,11 @@ export function ChannelScreen({ isThreadMuted, readStateVersion, }); - const [threadScrollTargetId, setThreadScrollTargetId, clearThreadTarget] = + const [threadScrollTarget, setThreadScrollTargetId, clearThreadTarget] = useThreadOpenScrollTarget( effectiveOpenThreadHeadId, threadFirstUnreadReplyId, + threadPanelData.visibleReplies, threadRepliesQuery.isFetching, ); const editTargetMessage = React.useMemo( @@ -949,7 +950,7 @@ export function ChannelScreen({ threadPanelWidthPx={threadPanelWidthPx} threadTypingPubkeys={threadTypingPubkeys} threadReplyTargetMessage={displayedThreadReplyTargetMessage} - threadScrollTargetId={threadScrollTargetId} + threadScrollTarget={threadScrollTarget} threadUnreadCounts={threadUnreadCounts} threadReplyUnreadCounts={threadReplyUnreadCounts} threadFirstUnreadReplyId={displayedThreadFirstUnreadReplyId} diff --git a/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts b/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts index 7066f5ba43..d9a1ed92f9 100644 --- a/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts +++ b/desktop/src/features/channels/ui/useThreadOpenScrollTarget.ts @@ -1,14 +1,46 @@ import * as React from "react"; +import type { ScrollTargetAlignment } from "@/features/messages/ui/anchoredScrollTarget"; + +type ThreadScrollTargetState = { + alignment: ScrollTargetAlignment; + id: string | null; +}; + +type LoadedThreadReply = { + message: { id: string }; +}; export function useThreadOpenScrollTarget( threadHeadId: string | null, firstUnreadReplyId: string | null, + loadedReplies: readonly LoadedThreadReply[], isFetchingReplies: boolean, ) { - const [scrollTargetId, setScrollTargetId] = React.useState( - null, - ); + const [scrollTarget, setScrollTarget] = + React.useState({ alignment: "center", id: null }); const latchedForHeadRef = React.useRef(null); + const lastReplyId = loadedReplies.at(-1)?.message.id ?? null; + const isReadyToLatch = + !isFetchingReplies && + lastReplyId !== null && + (firstUnreadReplyId === null || + loadedReplies.some((entry) => entry.message.id === firstUnreadReplyId)); + + // Every caller-provided target is an explicit navigation (route target, + // branch expansion, sent reply, or layout-mode restore), so it remains + // centered even when its id happens to equal the unread anchor. + const setScrollTargetId = React.useCallback< + React.Dispatch> + >((nextTarget) => { + setScrollTarget((current) => { + const nextId = + typeof nextTarget === "function" ? nextTarget(current.id) : nextTarget; + if (current.id === nextId && current.alignment === "center") { + return current; + } + return { alignment: "center", id: nextId }; + }); + }, []); React.useEffect(() => { if (!threadHeadId) { @@ -20,26 +52,41 @@ export function useThreadOpenScrollTarget( } // Explicit route/deep-link targets take precedence over the unread anchor. - if (scrollTargetId !== null) { + if (scrollTarget.id !== null) { latchedForHeadRef.current = threadHeadId; return; } - // A cached thread has data while its fresh subtree is still loading. Wait - // for that refresh so a stale all-read snapshot cannot latch too early. - if (isFetchingReplies) { + // Wait for the refreshed reply set and divider snapshot to agree on a real + // rendered row. This prevents a transient unread id from being frozen as + // an invisible boundary. + if (!isReadyToLatch) { return; } latchedForHeadRef.current = threadHeadId; - if (firstUnreadReplyId) { - setScrollTargetId(firstUnreadReplyId); + const nextId = firstUnreadReplyId ?? lastReplyId; + if (nextId) { + setScrollTarget({ + alignment: firstUnreadReplyId ? "top-with-divider" : "bottom", + id: nextId, + }); } - }, [firstUnreadReplyId, isFetchingReplies, scrollTargetId, threadHeadId]); + }, [ + firstUnreadReplyId, + isReadyToLatch, + lastReplyId, + scrollTarget.id, + threadHeadId, + ]); const clearScrollTarget = React.useCallback(() => { - setScrollTargetId(null); + setScrollTarget((current) => + current.id === null && current.alignment === "center" + ? current + : { alignment: "center", id: null }, + ); }, []); - return [scrollTargetId, setScrollTargetId, clearScrollTarget] as const; + return [scrollTarget, setScrollTargetId, clearScrollTarget] as const; } diff --git a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts index 1cc0b36a4b..ce68ad2c1b 100644 --- a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts +++ b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts @@ -4,6 +4,7 @@ import { setThreadViewMode, type ThreadViewMode, } from "@/features/channels/lib/threadViewModePreference"; +import type { ScrollTargetAlignment } from "@/features/messages/ui/anchoredScrollTarget"; export function findTopVisibleThreadMessageId( body: HTMLElement | null, @@ -32,6 +33,7 @@ export function getResolvedThreadTargets({ } type ThreadViewModeSwitchOptions = { + externalScrollTargetAlignment: ScrollTargetAlignment; externalScrollTargetId: string | null; onExternalTargetResolved: () => void; onModeChange?: (mode: ThreadViewMode) => void; @@ -39,6 +41,7 @@ type ThreadViewModeSwitchOptions = { /** Preserves the reply being read while the thread changes presentation. */ export function useThreadViewModeSwitch({ + externalScrollTargetAlignment, externalScrollTargetId, onExternalTargetResolved, onModeChange, @@ -81,7 +84,13 @@ export function useThreadViewModeSwitch({ return { changeThreadViewMode, - layoutScrollTargetId, resolveScrollTarget, + scrollTarget: { + alignment: layoutScrollTargetId + ? "center" + : externalScrollTargetAlignment, + id: layoutScrollTargetId ?? externalScrollTargetId, + isLayout: layoutScrollTargetId !== null, + }, }; } diff --git a/desktop/src/features/home/lib/inboxOpenScroll.test.mjs b/desktop/src/features/home/lib/inboxOpenScroll.test.mjs new file mode 100644 index 0000000000..ceedb8bbdb --- /dev/null +++ b/desktop/src/features/home/lib/inboxOpenScroll.test.mjs @@ -0,0 +1,65 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveInboxOpenScrollTarget } from "./inboxOpenScroll.ts"; + +const messages = [ + { createdAt: 10, id: "root" }, + { createdAt: 20, id: "read-reply" }, + { createdAt: 30, id: "first-unread" }, + { createdAt: 40, id: "latest" }, +]; + +test("unread thread opens at the first reply beyond its captured frontier", () => { + assert.deepEqual( + resolveInboxOpenScrollTarget( + { + anchorEventId: "latest", + conversationId: "root", + excludedRootId: "root", + forcedUnreadMessageId: null, + openReadAt: 20, + requestId: 1, + wasUnread: true, + }, + messages, + ), + { alignment: "top-with-divider", id: "first-unread" }, + ); +}); + +test("already-read thread opens at its physical bottom", () => { + assert.deepEqual( + resolveInboxOpenScrollTarget( + { + anchorEventId: "latest", + conversationId: "root", + excludedRootId: "root", + forcedUnreadMessageId: null, + openReadAt: 40, + requestId: 2, + wasUnread: false, + }, + messages, + ), + { alignment: "bottom", id: "latest" }, + ); +}); + +test("local mark-unread targets the overridden representative event", () => { + assert.deepEqual( + resolveInboxOpenScrollTarget( + { + anchorEventId: "latest", + conversationId: "root", + excludedRootId: "root", + forcedUnreadMessageId: "latest", + openReadAt: 40, + requestId: 3, + wasUnread: true, + }, + messages, + ), + { alignment: "top-with-divider", id: "latest" }, + ); +}); diff --git a/desktop/src/features/home/lib/inboxOpenScroll.ts b/desktop/src/features/home/lib/inboxOpenScroll.ts new file mode 100644 index 0000000000..d161bb066b --- /dev/null +++ b/desktop/src/features/home/lib/inboxOpenScroll.ts @@ -0,0 +1,45 @@ +import type { ScrollTargetAlignment } from "@/features/messages/ui/anchoredScrollTarget"; + +export type InboxOpenScrollIntent = { + anchorEventId: string; + conversationId: string; + excludedRootId: string | null; + forcedUnreadMessageId: string | null; + openReadAt: number | null; + requestId: number; + wasUnread: boolean; +}; + +export type InboxOpenScrollTarget = { + alignment: ScrollTargetAlignment; + id: string; +}; + +/** + * Resolves the Inbox-owned layout target from the read state captured before + * opening the row advances its marker. The caller latches this once context + * loading finishes so live arrivals cannot move the open-time boundary. + */ +export function resolveInboxOpenScrollTarget( + intent: InboxOpenScrollIntent, + messages: readonly { createdAt: number; id: string }[], +): InboxOpenScrollTarget | null { + if (messages.length === 0) return null; + + if (intent.wasUnread) { + const firstUnread = intent.forcedUnreadMessageId + ? messages.find((message) => message.id === intent.forcedUnreadMessageId) + : messages.find( + (message) => + message.id !== intent.excludedRootId && + (intent.openReadAt === null || + message.createdAt > intent.openReadAt), + ); + if (firstUnread) { + return { alignment: "top-with-divider", id: firstUnread.id }; + } + } + + const lastMessage = messages.at(-1); + return lastMessage ? { alignment: "bottom", id: lastMessage.id } : null; +} diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx index 992702f431..4efbee9366 100644 --- a/desktop/src/features/home/ui/HomeView.tsx +++ b/desktop/src/features/home/ui/HomeView.tsx @@ -17,6 +17,7 @@ import { getInboxConversationId, } from "@/features/home/lib/inbox"; import { useInboxSelectionAnchor } from "@/features/home/useInboxSelectionAnchor"; +import { useInboxRowSelection } from "@/features/home/useInboxRowSelection"; import { getReactionTargetId, matchesInboxFilter, @@ -167,20 +168,8 @@ export function HomeView({ const profilePanelView = profilePanelViewFromSearch( inboxSearchValues.profileView, ); - // Selection state — two-tier design so explicit and automatic selections - // have distinct ownership: - // - // urlSelectedItemId — explicit/user anchor, URL-authoritative. Written - // only by handleUserSelectItem (via applyInboxSearchPatch) and by - // back/forward navigation. Never touched by background data loads. - // - // autoSelectedEventId — default desktop selection when the URL carries no - // explicit anchor. Written only by the auto-selection effect. Never - // triggers a history push. - // - // selectedEventId — the effective anchor used everywhere below: the URL - // anchor when present, otherwise the auto-selected fallback. Derived - // synchronously, no separate state — so there is no mirror-revert race. + // Explicit selections are URL-owned; automatic desktop selection stays + // local so background data never creates navigation entries. const [autoSelectedEventId, setAutoSelectedEventId] = React.useState< string | null >(null); @@ -396,6 +385,16 @@ export function HomeView({ undoDoneLocal: undoDone, undoUnreadLocal: undoUnread, }); + const { openScrollIntent, selectInboxRow } = useInboxRowSelection({ + doneSet: effectiveDoneSet, + getChannelReadAt, + getMessageReadAt, + getThreadReadAt, + items: inboxItems, + localUnreadSet: unreadSet, + markItemRead, + selectItem: handleUserSelectItem, + }); // Resolve the selected row and stable conversation ID from inboxItems // (unfiltered). We need conversationId before filtering so we can keep the // selected item visible when unreadOnly is on. The event anchor may point to @@ -757,10 +756,7 @@ export function HomeView({ preview: item.preview.slice(0, 100), }); }} - onSelect={(itemId) => { - handleUserSelectItem(itemId); - markItemRead(itemId); - }} + onSelect={selectInboxRow} onSelectDraft={setSelectedDraftKey} onUnreadOnlyChange={setUnreadOnly} reminderPubkey={currentPubkey} @@ -814,6 +810,7 @@ export function HomeView({ item={selectedItem} latchedDefaultParentId={latchedDefaultParentId} messages={contextMessages} + openScrollIntent={openScrollIntent} profiles={feedProfiles} selectedEventId={selectedEventId} onBack={ diff --git a/desktop/src/features/home/ui/InboxDetailPane.tsx b/desktop/src/features/home/ui/InboxDetailPane.tsx index 611adbf559..62ec7ddcb4 100644 --- a/desktop/src/features/home/ui/InboxDetailPane.tsx +++ b/desktop/src/features/home/ui/InboxDetailPane.tsx @@ -6,6 +6,11 @@ import type { InboxItem, InboxReply, } from "@/features/home/lib/inbox"; +import { + type InboxOpenScrollIntent, + type InboxOpenScrollTarget, + resolveInboxOpenScrollTarget, +} from "@/features/home/lib/inboxOpenScroll"; import { ChannelMembersBar } from "@/features/channels/ui/ChannelMembersBar"; import { formatInboxTypeLabel } from "@/features/home/lib/inbox"; import { @@ -22,6 +27,7 @@ import { orderMentionPubkeysByText } from "@/features/messages/lib/orderMentionP import { getThreadReference } from "@/features/messages/lib/threading"; import { normalizePubkey } from "@/shared/lib/pubkey"; import { MessageComposer } from "@/features/messages/ui/MessageComposer"; +import { UnreadDivider } from "@/features/messages/ui/UnreadDivider"; import { useAnchoredScroll } from "@/features/messages/ui/useAnchoredScroll"; import { useComposerHeightPadding } from "@/features/messages/ui/useComposerHeightPadding"; import { UpdateIndicator } from "@/features/settings/UpdateIndicator"; @@ -80,6 +86,8 @@ type InboxDetailPaneProps = { * (e.g. a very old anchor evicted by a newer representative). */ latchedDefaultParentId?: string | null; + /** Inbox-row layout intent captured before opening advances read state. */ + openScrollIntent?: InboxOpenScrollIntent | null; onBack?: () => void; onDelete: () => void; onManageChannel: (channelId: string) => void; @@ -120,6 +128,7 @@ export function InboxDetailPane({ currentPubkey, selectedEventId, latchedDefaultParentId = null, + openScrollIntent = null, onBack, onDelete, onManageChannel, @@ -207,14 +216,60 @@ export function InboxDetailPane({ ...pendingReplyMessages, ] : pendingReplyMessages; + const [latchedOpenTarget, setLatchedOpenTarget] = React.useState<{ + requestId: number; + target: InboxOpenScrollTarget; + } | null>(null); + const hasMatchingOpenIntent = + openScrollIntent?.conversationId === conversationId && + openScrollIntent.anchorEventId === selectedEventId; + React.useEffect(() => { + if (!hasMatchingOpenIntent || !openScrollIntent) { + setLatchedOpenTarget(null); + return; + } + if ( + isThreadContextLoading || + latchedOpenTarget?.requestId === openScrollIntent.requestId + ) { + return; + } + const target = resolveInboxOpenScrollTarget( + openScrollIntent, + displayMessages, + ); + if (target) { + setLatchedOpenTarget({ requestId: openScrollIntent.requestId, target }); + } + }, [ + displayMessages, + hasMatchingOpenIntent, + isThreadContextLoading, + latchedOpenTarget?.requestId, + openScrollIntent, + ]); + const openTarget = + hasMatchingOpenIntent && + latchedOpenTarget?.requestId === openScrollIntent?.requestId + ? latchedOpenTarget.target + : null; + const scrollTarget = hasMatchingOpenIntent + ? openTarget + : selectedEventId + ? { alignment: "center" as const, id: selectedEventId } + : null; const { onScroll } = useAnchoredScroll({ - channelId: conversationId, + channelId: hasMatchingOpenIntent + ? `${conversationId}:${openScrollIntent.requestId}` + : conversationId, contentRef, + highlightTargetMessage: !hasMatchingOpenIntent, isLoading: isThreadContextLoading, messages: displayMessages, - pinTargetCentered: true, + pinTargetCentered: !hasMatchingOpenIntent, scrollContainerRef, - targetMessageId: selectedEventId, + targetAlignment: scrollTarget?.alignment, + targetMessageId: scrollTarget?.id ?? null, }); const focusComposer = React.useCallback(() => { @@ -478,6 +533,7 @@ export function InboxDetailPane({ aria-busy={isThreadContextLoading} className="-mt-13 min-h-0 flex-1 overflow-y-auto overscroll-contain pb-32 pt-13 [overflow-anchor:none]" data-testid="home-inbox-detail-scroll" + key={conversationId} onScroll={onScroll} ref={scrollContainerRef} > @@ -497,18 +553,24 @@ export function InboxDetailPane({ ); return ( - + + {openTarget?.alignment === "top-with-divider" && + openTarget.id === message.id && + index > 0 ? ( + + ) : null} + + ); })} diff --git a/desktop/src/features/home/useInboxRowSelection.ts b/desktop/src/features/home/useInboxRowSelection.ts new file mode 100644 index 0000000000..f10ab872ff --- /dev/null +++ b/desktop/src/features/home/useInboxRowSelection.ts @@ -0,0 +1,87 @@ +import * as React from "react"; + +import type { InboxItem } from "@/features/home/lib/inbox"; +import type { InboxOpenScrollIntent } from "@/features/home/lib/inboxOpenScroll"; +import { + hasGroupedUnreadOverride, + resolveInboxItemReadAt, +} from "@/features/home/useHomeInboxReadState"; +import { + getThreadReference, + isThreadReply, +} from "@/features/messages/lib/threading"; + +type UseInboxRowSelectionOptions = { + doneSet: ReadonlySet; + getChannelReadAt: (channelId: string) => number | null; + getMessageReadAt: (messageId: string) => number | null; + getThreadReadAt: (rootId: string, channelId?: string | null) => number | null; + items: readonly InboxItem[]; + localUnreadSet: ReadonlySet; + markItemRead: (itemId: string) => void; + selectItem: (itemId: string | null) => void; +}; + +export function useInboxRowSelection({ + doneSet, + getChannelReadAt, + getMessageReadAt, + getThreadReadAt, + items, + localUnreadSet, + markItemRead, + selectItem, +}: UseInboxRowSelectionOptions) { + const requestIdRef = React.useRef(0); + const [openScrollIntent, setOpenScrollIntent] = + React.useState(null); + const itemById = React.useMemo( + () => new Map(items.map((item) => [item.id, item])), + [items], + ); + + const selectInboxRow = React.useCallback( + (itemId: string) => { + const item = itemById.get(itemId); + if (item) { + requestIdRef.current += 1; + const wasUnread = !doneSet.has(item.id); + const threadRootId = isThreadReply(item.item.tags) + ? getThreadReference(item.item.tags).rootId + : null; + setOpenScrollIntent({ + anchorEventId: item.id, + conversationId: item.conversationId, + excludedRootId: threadRootId, + forcedUnreadMessageId: + wasUnread && hasGroupedUnreadOverride(item, localUnreadSet) + ? item.id + : null, + openReadAt: wasUnread + ? resolveInboxItemReadAt(item, { + getChannelReadAt, + getMessageReadAt, + getThreadReadAt, + }) + : null, + requestId: requestIdRef.current, + wasUnread, + }); + } + selectItem(itemId); + markItemRead(itemId); + }, + [ + doneSet, + getChannelReadAt, + getMessageReadAt, + getThreadReadAt, + itemById, + localUnreadSet, + markItemRead, + selectItem, + ], + ); + + return { openScrollIntent, selectInboxRow }; +} diff --git a/desktop/src/features/home/useInboxThreadContext.ts b/desktop/src/features/home/useInboxThreadContext.ts index b3c1da9485..a6afbb4f4f 100644 --- a/desktop/src/features/home/useInboxThreadContext.ts +++ b/desktop/src/features/home/useInboxThreadContext.ts @@ -38,7 +38,9 @@ export function useInboxThreadContext( channelMessages: RelayEvent[] | undefined, ): InboxThreadContextResult { const [fetchedEvents, setFetchedEvents] = React.useState([]); - const [isLoading, setIsLoading] = React.useState(false); + const [loadedSelectionKey, setLoadedSelectionKey] = React.useState< + string | null + >(null); const selectedEvent = React.useMemo( () => (item ? relayEventFromFeedItem(item) : null), @@ -52,13 +54,21 @@ export function useInboxThreadContext( ? getThreadReference(selectedEvent.tags).parentId : null; const selectedChannelId = item?.channelId ?? null; + const selectionKey = selectedEvent + ? [ + selectedChannelId, + selectedEvent.id, + selectedParentId, + selectedThreadRootId, + ].join(":") + : null; React.useEffect(() => { let isCancelled = false; if (!selectedEvent || !selectedThreadRootId) { setFetchedEvents([]); - setIsLoading(false); + setLoadedSelectionKey(null); return () => { isCancelled = true; }; @@ -71,8 +81,6 @@ export function useInboxThreadContext( return; } - setIsLoading(true); - try { const selection = { selectedChannelId, @@ -127,7 +135,7 @@ export function useInboxThreadContext( ); } finally { if (!isCancelled) { - setIsLoading(false); + setLoadedSelectionKey(selectionKey); } } } @@ -142,6 +150,7 @@ export function useInboxThreadContext( selectedEvent, selectedParentId, selectedThreadRootId, + selectionKey, ]); const events = React.useMemo(() => { @@ -240,7 +249,7 @@ export function useInboxThreadContext( return { events, - isLoading, + isLoading: selectionKey !== null && loadedSelectionKey !== selectionKey, reactionEvents, refreshReactions, }; diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index c7624e43a5..06fa240d5d 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -45,6 +45,7 @@ import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; import { TypingIndicatorRow } from "./TypingIndicatorRow"; import { UnreadDivider } from "./UnreadDivider"; import { useComposerHeightPadding } from "./useComposerHeightPadding"; +import type { ScrollTargetAlignment } from "./anchoredScrollTarget"; import { useAnchoredScroll } from "./useAnchoredScroll"; import { selectDeferredListRenderState } from "@/features/messages/lib/timelineSnapshot"; @@ -98,6 +99,7 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & { ) => Promise; profiles?: UserProfileLookup; replyTargetMessage: TimelineMessage | null; + scrollTargetAlignment?: ScrollTargetAlignment; scrollTargetId: string | null; threadHead: TimelineMessage | null; threadReplies: MainTimelineEntry[]; @@ -213,6 +215,7 @@ export function MessageThreadPanel({ onUnfollowThread, profiles, replyTargetMessage, + scrollTargetAlignment = "center", scrollTargetId, scrollTargetHighlights = true, threadHead, @@ -489,6 +492,7 @@ export function MessageThreadPanel({ highlightTargetMessage: scrollTargetHighlights, onTargetReached: onScrollTargetResolved, scrollContainerRef: threadBodyRef, + targetAlignment: scrollTargetAlignment, targetMessageId: scrollTargetId, }); diff --git a/desktop/src/features/messages/ui/anchoredScrollTarget.ts b/desktop/src/features/messages/ui/anchoredScrollTarget.ts new file mode 100644 index 0000000000..c9eec53663 --- /dev/null +++ b/desktop/src/features/messages/ui/anchoredScrollTarget.ts @@ -0,0 +1,85 @@ +const TOP_ALIGNED_TARGET_INSET_PX = 12; + +export type ScrollTargetAlignment = "bottom" | "center" | "top-with-divider"; + +export function makeTargetKey( + id: string | null, + alignment: ScrollTargetAlignment, +) { + return id ? `${alignment}:${id}` : null; +} + +export function resolveTargetDivider({ + alignment, + container, + firstMessageId, + targetMessageId, +}: { + alignment: ScrollTargetAlignment; + container: HTMLElement; + firstMessageId: string | undefined; + targetMessageId: string; +}): HTMLElement | null | undefined { + if (alignment !== "top-with-divider") return null; + const divider = container.querySelector( + '[data-testid="message-unread-divider"]', + ); + // The row can commit one render before its unread divider. Keep a non-first + // target unresolved so a later DOM retry aligns the marker, not the row. + // The first reply intentionally has no divider and may use the row. + if (firstMessageId !== targetMessageId && !divider) return undefined; + return divider; +} + +export function getTargetScrollPlacement({ + alignment, + containerClientHeight, + containerScrollHeight, + containerScrollTop, + containerTop, + dividerTop, + targetHeight, + targetTop, +}: { + alignment: ScrollTargetAlignment; + containerClientHeight: number; + containerScrollHeight: number; + containerScrollTop: number; + containerTop: number; + dividerTop: number | null; + targetHeight: number; + targetTop: number; +}) { + const currentTopOffset = targetTop - containerTop; + const maxScrollTop = Math.max( + 0, + containerScrollHeight - containerClientHeight, + ); + let targetScrollTop: number; + if (alignment === "bottom") { + targetScrollTop = maxScrollTop; + } else if (alignment === "top-with-divider") { + targetScrollTop = Math.min( + maxScrollTop, + Math.max( + 0, + containerScrollTop + + ((dividerTop ?? targetTop) - containerTop) - + TOP_ALIGNED_TARGET_INSET_PX, + ), + ); + } else { + const centeredTopOffset = (containerClientHeight - targetHeight) / 2; + targetScrollTop = Math.min( + maxScrollTop, + Math.max(0, containerScrollTop + currentTopOffset - centeredTopOffset), + ); + } + + return { + contentTop: targetTop + containerScrollTop - containerTop, + maxScrollTop, + targetScrollTop, + targetTopOffset: currentTopOffset - (targetScrollTop - containerScrollTop), + }; +} diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index 44854c0d58..4b59eb59ae 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -143,6 +143,9 @@ import { createRoot } from "react-dom/client"; import { useAnchoredScroll } from "./useAnchoredScroll.ts"; +const PINNED_CENTER_MESSAGES = [{ id: "selected" }]; +const ALIGNED_TARGET_MESSAGES = [{ id: "target" }]; + function makePinnedCenterNodes() { const resizeObservers = []; const content = {}; @@ -220,15 +223,131 @@ function makePinnedCenterNodes() { } function Harness({ channelId, refs }) { - useAnchoredScroll({ + const anchoredScroll = useAnchoredScroll({ channelId, contentRef: refs.content, isLoading: false, - messages: [{ id: "selected" }], + messages: PINNED_CENTER_MESSAGES, pinTargetCentered: true, scrollContainerRef: refs.container, targetMessageId: "selected", }); + refs.onScroll = anchoredScroll.onScroll; + return null; +} + +function makeAlignedTargetNodes({ + dividerContentTop = 600, + dividerVisible = true, + scrollHeight = 1_200, +} = {}) { + const mutationObservers = []; + const resizeObservers = []; + const content = {}; + const container = { + clientHeight: 400, + scrollHeight, + scrollTop: 0, + getBoundingClientRect() { + return { top: 0 }; + }, + querySelector(selector) { + return selector === '[data-testid="message-unread-divider"]' + ? dividerVisible + ? divider + : null + : row; + }, + querySelectorAll() { + return [row]; + }, + scrollTo({ top }) { + this.scrollTop = Math.min( + top, + Math.max(0, this.scrollHeight - this.clientHeight), + ); + }, + }; + const divider = { + getBoundingClientRect() { + const top = dividerContentTop - container.scrollTop; + return { bottom: top + 20, height: 20, top }; + }, + }; + const wrapper = { + querySelector(selector) { + return selector === '[data-testid="message-unread-divider"]' + ? dividerVisible + ? divider + : null + : null; + }, + }; + const row = { + dataset: { messageId: "target" }, + parentElement: wrapper, + getBoundingClientRect() { + const top = dividerContentTop + 20 - container.scrollTop; + return { bottom: top + 80, height: 80, top }; + }, + }; + + globalThis.ResizeObserver = class { + constructor(callback) { + this.callback = callback; + resizeObservers.push(this); + } + + disconnect() {} + + observe(target) { + this.target = target; + } + }; + + globalThis.MutationObserver = class { + constructor(callback) { + this.callback = callback; + mutationObservers.push(this); + } + + disconnect() {} + + observe(target, options) { + this.options = options; + this.target = target; + } + }; + + return { + container, + content, + mutationObservers, + resizeObservers, + showDivider() { + dividerVisible = true; + }, + }; +} + +function AlignedTargetHarness({ + alignment, + messages = ALIGNED_TARGET_MESSAGES, + onTargetReached, + refs, + targetMessageId = "target", +}) { + const anchoredScroll = useAnchoredScroll({ + channelId: alignment, + contentRef: refs.content, + isLoading: false, + messages, + onTargetReached, + scrollContainerRef: refs.container, + targetAlignment: alignment, + targetMessageId, + }); + refs.onScroll = anchoredScroll.onScroll; return null; } @@ -261,7 +380,9 @@ test("channel change attaches pinned-center observers after refs mount", async ( nodes.container.dispatchEvent({ type: "wheel" }); }); nodes.moveSelectedRowBy(96); - nodes.resizeObservers[0].callback(); + await act(async () => { + nodes.resizeObservers[0].callback(); + }); assert.deepEqual( nodes.container.scrollWrites, [], @@ -272,3 +393,309 @@ test("channel change attaches pinned-center observers after refs mount", async ( root.unmount(); }); }); + +test("a stale scroll event from a retired conversation cannot release the new pinned target", async () => { + const firstNodes = makePinnedCenterNodes(); + const refs = { + container: { current: firstNodes.container }, + content: { current: firstNodes.content }, + }; + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render( + React.createElement(Harness, { channelId: "first-conversation", refs }), + ); + }); + + const secondNodes = makePinnedCenterNodes(); + refs.container.current = secondNodes.container; + refs.content.current = secondNodes.content; + await act(async () => { + root.render( + React.createElement(Harness, { channelId: "second-conversation", refs }), + ); + }); + + secondNodes.moveSelectedRowBy(24); + await act(async () => { + secondNodes.resizeObservers[0].callback(); + }); + assert.deepEqual(secondNodes.container.scrollWrites, [24]); + secondNodes.container.scrollWrites.length = 0; + + await act(async () => { + refs.onScroll({ currentTarget: firstNodes.container }); + }); + secondNodes.moveSelectedRowBy(96); + await act(async () => { + secondNodes.resizeObservers[0].callback(); + }); + assert.deepEqual( + secondNodes.container.scrollWrites, + [96], + "the new conversation must retain its pin after the old container's delayed scroll event", + ); + + await act(async () => { + root.unmount(); + }); +}); + +test("bottom-aligned id target reaches and follows the physical floor", async () => { + const nodes = makeAlignedTargetNodes(); + const refs = { + container: { current: nodes.container }, + content: { current: nodes.content }, + }; + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "bottom", + refs, + }), + ); + }); + assert.equal(nodes.container.scrollTop, 800); + + nodes.container.scrollHeight = 1_600; + await act(async () => { + nodes.resizeObservers[0].callback(); + }); + assert.equal( + nodes.container.scrollTop, + 1_200, + "late content growth must keep an all-read thread at the new floor", + ); + + await act(async () => { + root.unmount(); + }); +}); + +test("top-with-divider target leaves the unread marker inset from the top", async () => { + const nodes = makeAlignedTargetNodes(); + const refs = { + container: { current: nodes.container }, + content: { current: nodes.content }, + }; + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "top-with-divider", + refs, + }), + ); + }); + assert.equal( + nodes.container.scrollTop, + 588, + "the divider at content offset 600 should retain the 12px top inset", + ); + + await act(async () => { + root.unmount(); + }); +}); + +test("a non-first unread target does not resolve until its divider exists", async () => { + const nodes = makeAlignedTargetNodes({ dividerVisible: false }); + const messages = [{ id: "before" }, { id: "target" }]; + let resolvedTargets = 0; + const onTargetReached = () => { + resolvedTargets += 1; + }; + const refs = { + container: { current: nodes.container }, + content: { current: nodes.content }, + }; + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "top-with-divider", + messages, + onTargetReached, + refs, + targetMessageId: null, + }), + ); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + assert.equal( + nodes.container.scrollTop, + 800, + "the mount fallback may hold the floor while the expected divider is absent", + ); + + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "top-with-divider", + messages, + onTargetReached, + refs, + }), + ); + }); + assert.equal(resolvedTargets, 0); + assert.equal(nodes.container.scrollTop, 800); + + nodes.showDivider(); + await act(async () => { + nodes.mutationObservers.at(-1).callback(); + }); + assert.equal(resolvedTargets, 1); + assert.equal( + nodes.container.scrollTop, + 588, + "the same target must place the divider after it commits", + ); + + await act(async () => { + root.unmount(); + }); +}); + +test("a near-floor top target survives its programmatic scroll event", async () => { + const nodes = makeAlignedTargetNodes({ dividerContentTop: 790 }); + const refs = { + container: { current: nodes.container }, + content: { current: nodes.content }, + }; + const root = createRoot(document.createElement("div")); + const pendingFrames = new Map(); + let nextFrameId = 1; + const originalRequestAnimationFrame = globalThis.requestAnimationFrame; + const originalCancelAnimationFrame = globalThis.cancelAnimationFrame; + globalThis.requestAnimationFrame = (callback) => { + const id = nextFrameId++; + pendingFrames.set(id, callback); + return id; + }; + globalThis.cancelAnimationFrame = (id) => { + pendingFrames.delete(id); + }; + + try { + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "top-with-divider", + refs, + }), + ); + }); + assert.equal(nodes.container.scrollTop, 778); + assert.equal(pendingFrames.size, 1); + + await act(async () => { + refs.onScroll(); + refs.onScroll(); + }); + await act(async () => { + nodes.resizeObservers[0].callback(); + }); + assert.equal( + nodes.container.scrollTop, + 778, + "duplicate native events from the target's own near-floor scroll must not turn its message anchor into bottom glue", + ); + + nodes.container.scrollTop = 800; + await act(async () => { + refs.onScroll(); + }); + nodes.container.scrollHeight = 1_600; + await act(async () => { + nodes.resizeObservers[0].callback(); + }); + assert.equal( + nodes.container.scrollTop, + 1_200, + "a later user scroll to the floor must still restore bottom glue", + ); + } finally { + globalThis.requestAnimationFrame = originalRequestAnimationFrame; + globalThis.cancelAnimationFrame = originalCancelAnimationFrame; + + await act(async () => { + root.unmount(); + }); + } +}); + +test("a resolved target cancels the stale mount bottom-pin frame", async () => { + const nodes = makeAlignedTargetNodes(); + const refs = { + container: { current: nodes.container }, + content: { current: nodes.content }, + }; + const root = createRoot(document.createElement("div")); + const pendingFrames = new Map(); + let nextFrameId = 1; + const originalRequestAnimationFrame = globalThis.requestAnimationFrame; + const originalCancelAnimationFrame = globalThis.cancelAnimationFrame; + globalThis.requestAnimationFrame = (callback) => { + const id = nextFrameId++; + pendingFrames.set(id, callback); + return id; + }; + globalThis.cancelAnimationFrame = (id) => { + pendingFrames.delete(id); + }; + + try { + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "top-with-divider", + refs, + targetMessageId: null, + }), + ); + }); + assert.equal(nodes.container.scrollTop, 800); + assert.equal(pendingFrames.size, 1); + + await act(async () => { + root.render( + React.createElement(AlignedTargetHarness, { + alignment: "top-with-divider", + refs, + targetMessageId: "target", + }), + ); + }); + assert.equal(nodes.container.scrollTop, 588); + + await act(async () => { + refs.onScroll(); + }); + assert.equal( + nodes.container.scrollTop, + 588, + "the resolved target scroll must supersede the mount bottom settle guard", + ); + + for (const callback of pendingFrames.values()) callback(0); + assert.equal( + nodes.container.scrollTop, + 588, + "the mount fallback must not overwrite the resolved unread target", + ); + } finally { + globalThis.requestAnimationFrame = originalRequestAnimationFrame; + globalThis.cancelAnimationFrame = originalCancelAnimationFrame; + await act(async () => { + root.unmount(); + }); + } +}); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index b865260de0..55ea6b4476 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -4,6 +4,12 @@ import { classifyTimelineMessageDelta, type TimelineMessageDelta, } from "@/features/messages/lib/timelineSnapshot"; +import { + getTargetScrollPlacement, + makeTargetKey, + resolveTargetDivider, + type ScrollTargetAlignment, +} from "@/features/messages/ui/anchoredScrollTarget"; /** * Distance (in CSS pixels) below which we consider the scroll position @@ -12,13 +18,8 @@ import { * rounding from the layout engine. */ const AT_BOTTOM_THRESHOLD_PX = 32; -// Tests and user-visible "pinned" affordances need the view at the physical -// floor, not merely within the looser UI at-bottom threshold. The loose -// threshold decides whether the user is close enough to count as reading the -// latest message; this strict threshold decides when a programmatic bottom pin -// has actually finished settling. +// Pinned affordances need the physical floor, not the looser UI threshold. const TRUE_BOTTOM_THRESHOLD_PX = 1; - type AnchorState = | { kind: "at-bottom" } | { kind: "message"; messageId: string; topOffset: number } @@ -104,6 +105,8 @@ type UseAnchoredScrollOptions = { /** When set, scroll to this message on mount and on change. */ targetMessageId?: string | null; + /** Thread-open anchors use top/bottom; explicit targets default to center. */ + targetAlignment?: ScrollTargetAlignment; /** Whether a targeted message should pulse after scrolling to it. */ highlightTargetMessage?: boolean; /** Keeps a targeted message centered until the user deliberately scrolls. */ @@ -124,7 +127,9 @@ type UseAnchoredScrollOptions = { type UseAnchoredScrollResult = { /** Pass through to the scroll container's `onScroll`. */ - onScroll: () => void; + onScroll: ( + event?: Pick, "currentTarget">, + ) => void; /** True when the user is within `AT_BOTTOM_THRESHOLD_PX` of the bottom. */ isAtBottom: boolean; /** Number of new messages that have arrived while the user is not at the @@ -226,6 +231,7 @@ export function useAnchoredScroll({ splitPanelOpen = false, targetMessageId = null, + targetAlignment = "center", highlightTargetMessage = true, pinTargetCentered = false, onTargetReached, @@ -256,10 +262,10 @@ export function useAnchoredScroll({ const prevFirstMessageIdRef = React.useRef(undefined); const prevMessageCountRef = React.useRef(0); const prevMessagesRef = React.useRef>([]); - const handledTargetIdRef = React.useRef(null); + const handledTargetRequestRef = React.useRef(null); + const targetRequestKey = makeTargetKey(targetMessageId, targetAlignment); const highlightTimeoutRef = React.useRef(null); - // Tracks a pending rAF queued by pinToBottomOnMount so it can be cancelled - // on channel switch (the channelId reset effect clears it). + // Tracks a pending mount pin so channel switches can cancel it. const mountPinRafIdRef = React.useRef(null); // One-shot: the consumer calls `scrollToBottomOnNextUpdate()` right before // it sends a message (see ChannelPane). When the user's own message then @@ -271,8 +277,7 @@ export function useAnchoredScroll({ // ignores transient gaps and keeps chasing the floor. A `ref`, not state — the // guard runs on a native scroll event, outside React's render cycle. const settlingRef = React.useRef(false); - // Pinned-center corrections write scroll position themselves. Keep the next - // matching scroll event from being mistaken for a user releasing the pin. + // Preserve a deliberate anchor across its own programmatic scroll event. const programmaticScrollTopRef = React.useRef(null); const isWritingScrollRef = React.useRef(false); const programmaticScrollRafRef = React.useRef(null); @@ -292,7 +297,7 @@ export function useAnchoredScroll({ prevFirstMessageIdRef.current = undefined; prevMessageCountRef.current = 0; prevMessagesRef.current = []; - handledTargetIdRef.current = null; + handledTargetRequestRef.current = null; forceBottomOnNextAppendRef.current = false; settlingRef.current = false; programmaticScrollTopRef.current = null; @@ -319,9 +324,8 @@ export function useAnchoredScroll({ if (programmaticScrollRafRef.current !== null) { cancelAnimationFrame(programmaticScrollRafRef.current); } - // A programmatic scroll event is delivered before the next frame. If the - // browser does not emit one, expire the guard so a later user scroll is - // never swallowed. + // Programmatic events are delivered before the next frame. Expire the + // exact destination then so a later non-pinned scroll is never swallowed. programmaticScrollRafRef.current = requestAnimationFrame(() => { if (programmaticScrollTopRef.current === container.scrollTop) { programmaticScrollTopRef.current = null; @@ -472,27 +476,46 @@ export function useAnchoredScroll({ } anchorRef.current = { kind: "message", messageId, topOffset: 0 }; setIsAtBottom(false); - if (el && options.highlight) highlightMessage(messageId); + if (el && options.highlight && targetAlignment !== "bottom") { + highlightMessage(messageId); + } return el !== null; } if (!el) return false; + const targetDivider = resolveTargetDivider({ + alignment: targetAlignment, + container, + firstMessageId: messages[0]?.id, + targetMessageId: messageId, + }); + if (targetDivider === undefined) return false; + + // A thread-open target can arrive one render after the mount fallback + // queued its settling bottom pin. Once the target row is real, that + // stale frame and its scroll-event settle guard must not overwrite a + // non-bottom placement. A bottom target keeps the guard so it can chase + // late row measurements to the physical floor. + if (mountPinRafIdRef.current !== null) { + cancelAnimationFrame(mountPinRafIdRef.current); + mountPinRafIdRef.current = null; + } + settlingRef.current = targetAlignment === "bottom"; + const rect = el.getBoundingClientRect(); const containerRect = container.getBoundingClientRect(); - const currentTopOffset = rect.top - containerRect.top; - const centeredTopOffset = (container.clientHeight - rect.height) / 2; - const maxScrollTop = Math.max( - 0, - container.scrollHeight - container.clientHeight, - ); - const targetScrollTop = Math.min( - maxScrollTop, - Math.max(0, container.scrollTop + currentTopOffset - centeredTopOffset), - ); - const targetTopOffset = - currentTopOffset - (targetScrollTop - container.scrollTop); - const contentTop = rect.top + container.scrollTop - containerRect.top; + const { contentTop, maxScrollTop, targetScrollTop, targetTopOffset } = + getTargetScrollPlacement({ + alignment: targetAlignment, + containerClientHeight: container.clientHeight, + containerScrollHeight: container.scrollHeight, + containerScrollTop: container.scrollTop, + containerTop: containerRect.top, + dividerTop: targetDivider?.getBoundingClientRect().top ?? null, + targetHeight: rect.height, + targetTop: rect.top, + }); if (pinTargetCentered) { writePinnedCenterScroll(container, () => { @@ -508,89 +531,99 @@ export function useAnchoredScroll({ }; setIsAtBottom(isAtBottomNow(container)); } else { + const scrollTopBefore = container.scrollTop; container.scrollTo({ top: targetScrollTop, behavior: options.behavior ?? "auto", }); + noteProgrammaticScroll(container, scrollTopBefore); // Smooth scrolling starts an async animation, so measuring after the call can still return the pre-animation position. // Save the clamped destination offset instead; otherwise a concurrent // render/ResizeObserver restore can fight the smooth scroll back toward // where it started. - anchorRef.current = { - kind: "message", - messageId, - topOffset: targetTopOffset, - }; + anchorRef.current = + targetAlignment === "bottom" + ? { kind: "at-bottom" } + : { + kind: "message", + messageId, + topOffset: targetTopOffset, + }; } if (!pinTargetCentered) { setIsAtBottom(maxScrollTop - targetScrollTop <= AT_BOTTOM_THRESHOLD_PX); } - if (options.highlight) highlightMessage(messageId); + if (options.highlight && targetAlignment !== "bottom") { + highlightMessage(messageId); + } return true; }, [ highlightMessage, + messages, + noteProgrammaticScroll, pinTargetCentered, scrollContainerRef, + targetAlignment, virtualizerOwnsPrependAnchoring, writePinnedCenterScroll, virtualScrollToMessage, ], ); - // Scroll handler: recompute anchor + bottom state from the current - // scroll position. Cheap enough to run on every scroll event — a single - // `getBoundingClientRect` walk plus rect reads. - const onScroll = React.useCallback(() => { - const container = scrollContainerRef.current; - if (!container) return; - // Virtua owns anchoring and reports bottom state separately. Avoid the - // fallback's O(N) DOM walk on every compositor-driven scroll event. - if (virtualizerOwnsPrependAnchoring) return; - // Row measurement can grow `scrollHeight` after a bottom pin and emit scroll - // events while `scrollTop` holds at the old floor — opening a transient gap - // above the true bottom. `computeAnchor` would read that as a deliberate - // scroll-up and latch a message anchor, freezing the view short of bottom. - // While settling, keep the anchor at-bottom and chase the physical floor. - if (settlingRef.current) { - if (settleProgrammaticBottomPin(container)) { - settlingRef.current = false; - } else { - if (virtualizerOwnsPrependAnchoring) { + // Recompute anchor + bottom state from the current scroll position. + const onScroll = React.useCallback( + (event?: Pick, "currentTarget">) => { + const container = scrollContainerRef.current; + if (!container) return; + // Ignore a native scroll queued for a keyed-out conversation surface. + if (event && event.currentTarget !== container) return; + // Virtua owns anchoring and reports bottom state separately. Avoid the + // fallback's O(N) DOM walk on every compositor-driven scroll event. + if (virtualizerOwnsPrependAnchoring) return; + // Row measurement can grow `scrollHeight` after a bottom pin and emit scroll + // events while `scrollTop` holds at the old floor — opening a transient gap + // above the true bottom. `computeAnchor` would read that as a deliberate + // scroll-up and latch a message anchor, freezing the view short of bottom. + // While settling, keep the anchor at-bottom and chase the physical floor. + if (settlingRef.current) { + if (settleProgrammaticBottomPin(container)) { settlingRef.current = false; + } else { + if (virtualizerOwnsPrependAnchoring) { + settlingRef.current = false; + } + return; } - return; } - } - if (anchorRef.current.kind === "pinned-center") { if ( + anchorRef.current.kind !== "at-bottom" && shouldIgnorePinnedCenterScroll({ currentScrollTop: container.scrollTop, expectedScrollTop: programmaticScrollTopRef.current, isWritingScroll: isWritingScrollRef.current, }) ) { - if (programmaticScrollTopRef.current === container.scrollTop) { - programmaticScrollTopRef.current = null; - } + // Chromium can emit duplicate events for one programmatic write; keep + // its destination armed until noteProgrammaticScroll's rAF expires it. return; } - releasePinnedCenter(); - return; - } - anchorRef.current = computeAnchor(container); - const atBottom = anchorRef.current.kind === "at-bottom"; - setIsAtBottom((prev) => (prev === atBottom ? prev : atBottom)); - if (atBottom) { - setNewMessageCount(0); - } - }, [ - releasePinnedCenter, - scrollContainerRef, - virtualizerOwnsPrependAnchoring, - ]); + if (anchorRef.current.kind === "pinned-center") { + // Explicit input releases the pin; bare native scroll events are + // ambiguous because programmatic events can arrive after long reflow. + return; + } + anchorRef.current = computeAnchor(container); + const atBottom = anchorRef.current.kind === "at-bottom"; + setIsAtBottom((prev) => (prev === atBottom ? prev : atBottom)); + if (atBottom) { + setNewMessageCount(0); + } + }, + [scrollContainerRef, virtualizerOwnsPrependAnchoring], + ); // --------------------------------------------------------------------------- // Anchor restoration: after every render, stick to the bottom if the user is @@ -599,6 +632,7 @@ export function useAnchoredScroll({ // loaded row stays in the DOM, so there is no JS message-anchor restore. // --------------------------------------------------------------------------- + // biome-ignore lint/correctness/useExhaustiveDependencies: channelId must rerun initialization after the reset effect even when cached messages and target are referentially unchanged. React.useLayoutEffect(() => { const container = scrollContainerRef.current; if (!container) return; @@ -630,7 +664,7 @@ export function useAnchoredScroll({ highlight: highlightTargetMessage, }) ) { - handledTargetIdRef.current = targetMessageId; + handledTargetRequestRef.current = targetRequestKey; onTargetReached?.(targetMessageId); } else { pinToBottomOnMount(); @@ -736,6 +770,7 @@ export function useAnchoredScroll({ prevMessageCountRef.current = messages.length; prevMessagesRef.current = messages; }, [ + channelId, highlightTargetMessage, isLoading, messages, @@ -744,6 +779,7 @@ export function useAnchoredScroll({ scrollToBottomImperative, scrollToMessageImperative, targetMessageId, + targetRequestKey, repinPinnedCenter, virtualScrollToBottom, virtualSettleAtBottom, @@ -784,8 +820,7 @@ export function useAnchoredScroll({ virtualizerOwnsPrependAnchoring, ]); - // Pinned centers survive our own corrections but release as soon as the - // reader deliberately takes control of the scroll position. + // Release pinned centers only when the reader deliberately takes control. // biome-ignore lint/correctness/useExhaustiveDependencies: channelId deliberately re-subscribes after a keyed or conditional scroll-container mount replaces ref.current. React.useEffect(() => { if (!pinTargetCentered) return; @@ -793,16 +828,23 @@ export function useAnchoredScroll({ if (!container) return; const handleUserInteraction = () => releasePinnedCenter(); + const handlePointerDown = (event: PointerEvent) => { + if (event.target === container) releasePinnedCenter(); + }; container.addEventListener("wheel", handleUserInteraction, { passive: true, }); container.addEventListener("touchstart", handleUserInteraction, { passive: true, }); + container.addEventListener("pointerdown", handlePointerDown, { + passive: true, + }); container.addEventListener("keydown", handleUserInteraction); return () => { container.removeEventListener("wheel", handleUserInteraction); container.removeEventListener("touchstart", handleUserInteraction); + container.removeEventListener("pointerdown", handlePointerDown); container.removeEventListener("keydown", handleUserInteraction); }; }, [channelId, pinTargetCentered, releasePinnedCenter, scrollContainerRef]); @@ -822,7 +864,7 @@ export function useAnchoredScroll({ // biome-ignore lint/correctness/useExhaustiveDependencies: `messages` and `virtualizerRenderVersion` are intentional retry triggers, not values read by the effect body — the effect reads the DOM (querySelector), and we need it to re-run each time the message list or virtualized rendered range changes so a target spliced into older history gets centered once its row commits. React.useEffect(() => { if (!targetMessageId) { - handledTargetIdRef.current = null; + handledTargetRequestRef.current = null; releasePinnedCenter(); return; } @@ -832,7 +874,8 @@ export function useAnchoredScroll({ ) { releasePinnedCenter(); } - if (handledTargetIdRef.current === targetMessageId || isLoading) return; + if (handledTargetRequestRef.current === targetRequestKey || isLoading) + return; if (!hasInitializedRef.current) return; // initial-mount path will handle. void virtualizerRenderVersion; @@ -847,7 +890,7 @@ export function useAnchoredScroll({ highlight: highlightTargetMessage, }) ) { - handledTargetIdRef.current = targetMessageId; + handledTargetRequestRef.current = targetRequestKey; onTargetReached?.(targetMessageId); } return; @@ -858,11 +901,14 @@ export function useAnchoredScroll({ // each `messages` commit and retries until the row exists. return; } - handledTargetIdRef.current = targetMessageId; - scrollToMessageImperative(targetMessageId, { - highlight: highlightTargetMessage, - }); - onTargetReached?.(targetMessageId); + if ( + scrollToMessageImperative(targetMessageId, { + highlight: highlightTargetMessage, + }) + ) { + handledTargetRequestRef.current = targetRequestKey; + onTargetReached?.(targetMessageId); + } }, [ highlightTargetMessage, isLoading, @@ -872,10 +918,49 @@ export function useAnchoredScroll({ scrollContainerRef, scrollToMessageImperative, targetMessageId, + targetRequestKey, virtualizerOwnsPrependAnchoring, virtualizerRenderVersion, ]); + // Adjacent target chrome can commit after its row without changing messages + // or box size. Retry unresolved targets on child-list commits too. + React.useEffect(() => { + if ( + !targetMessageId || + handledTargetRequestRef.current === targetRequestKey || + isLoading || + typeof MutationObserver === "undefined" + ) { + return; + } + const content = contentRef.current; + if (!content) return; + + const observer = new MutationObserver(() => { + if (handledTargetRequestRef.current === targetRequestKey) return; + if ( + scrollToMessageImperative(targetMessageId, { + highlight: highlightTargetMessage, + }) + ) { + handledTargetRequestRef.current = targetRequestKey; + onTargetReached?.(targetMessageId); + observer.disconnect(); + } + }); + observer.observe(content, { childList: true, subtree: true }); + return () => observer.disconnect(); + }, [ + contentRef, + highlightTargetMessage, + isLoading, + onTargetReached, + scrollToMessageImperative, + targetMessageId, + targetRequestKey, + ]); + React.useEffect(() => { return () => { if (highlightTimeoutRef.current !== null) { diff --git a/desktop/tests/e2e/inbox-live-update.spec.ts b/desktop/tests/e2e/inbox-live-update.spec.ts index 94e0b7b54a..9bc548acdf 100644 --- a/desktop/tests/e2e/inbox-live-update.spec.ts +++ b/desktop/tests/e2e/inbox-live-update.spec.ts @@ -30,6 +30,7 @@ type MockWindow = Window & { parentEventId?: string | null; pubkey?: string; mentionPubkeys?: string[]; + createdAt?: number; id?: string; kind?: number; extraTags?: string[][]; @@ -70,6 +71,13 @@ type MockWindow = Window & { __BUZZ_E2E_RELEASE_GET_EVENT__?: () => number; /** Running count of get_event invocations since installMockBridge. */ __BUZZ_E2E_GET_EVENT_CALL_COUNT__?: number; + __BUZZ_E2E_EMIT_MOCK_READ_STATE__?: (input: { + clientId: string; + contexts: Record; + createdAt: number; + slotId: string; + }) => unknown; + __BUZZ_E2E_RETIRED_INBOX_SCROLLER__?: HTMLElement; }; // ─── helpers ──────────────────────────────────────────────────────────────── @@ -173,6 +181,16 @@ async function getItemParam(page: import("@playwright/test").Page) { return hashSearch.get("item"); } +async function navigateToInboxItem( + page: import("@playwright/test").Page, + itemId: string, +) { + await page.evaluate((id) => { + window.location.hash = `#/?item=${id}`; + }, itemId); + await expect.poll(() => getItemParam(page)).toBe(itemId); +} + // ─── seed helpers ───────────────────────────────────────────────────────── /** @@ -325,8 +343,12 @@ test.describe("inbox stable-conversation regressions", () => { ) as HTMLElement | null; if (!pane) return; pane.scrollTop = pane.scrollHeight; // go to bottom - const mid = Math.max(1, Math.floor(pane.scrollHeight / 2)); + const mid = Math.max( + 1, + Math.floor((pane.scrollHeight - pane.clientHeight) / 2), + ); pane.scrollTop = mid; // settle at mid + pane.dispatchEvent(new Event("scroll", { bubbles: true })); }); // State-based wait: scroll must be non-zero before proceeding. await page.waitForFunction(() => { @@ -337,6 +359,18 @@ test.describe("inbox stable-conversation regressions", () => { }); const scrollTopBefore = await getScrollTop(page); expect(scrollTopBefore).toBeGreaterThan(0); + const rootOffsetBefore = await page.evaluate((rootId) => { + const pane = document.querySelector( + '[data-testid="home-inbox-detail"] [aria-busy]', + ); + const rootRow = pane?.querySelector( + `[data-message-id="${rootId}"]`, + ); + return pane && rootRow + ? rootRow.getBoundingClientRect().top - pane.getBoundingClientRect().top + : null; + }, root.id); + expect(rootOffsetBefore).not.toBeNull(); // Type a draft and confirm focus before the live update. const composer = detail.getByTestId("message-input"); @@ -358,8 +392,21 @@ test.describe("inbox stable-conversation regressions", () => { ).toBeVisible(); // ── 1: scroll position preserved ────────────────────────────────── - const scrollTopAfter = await getScrollTop(page); - expect(Math.abs(scrollTopAfter - scrollTopBefore)).toBeLessThanOrEqual(2); + const rootOffsetAfter = await page.evaluate((rootId) => { + const pane = document.querySelector( + '[data-testid="home-inbox-detail"] [aria-busy]', + ); + const rootRow = pane?.querySelector( + `[data-message-id="${rootId}"]`, + ); + return pane && rootRow + ? rootRow.getBoundingClientRect().top - pane.getBoundingClientRect().top + : null; + }, root.id); + expect(rootOffsetAfter).not.toBeNull(); + expect( + Math.abs((rootOffsetAfter ?? 0) - (rootOffsetBefore ?? 0)), + ).toBeLessThanOrEqual(2); // ── 2a: draft text preserved ─────────────────────────────────────── await expect(composer).toHaveText( @@ -929,7 +976,7 @@ test.describe("inbox stable-conversation regressions", () => { expect(raceSend?.parentEventId).toBe(coldRoot.id); }); - test("clicking newest inbox item after fetch-path load snaps to that message, not mid-thread", async ({ + test("unread fetch-path thread waits for context, then opens at its first unread reply", async ({ page, }) => { // Regression: the selected message was visible in `displayMessages` at @@ -937,8 +984,8 @@ test.describe("inbox stable-conversation regressions", () => { // fetched older ancestors and prepended them above the viewport, shifting // scrollTop to mid-thread while the newest message slid below the fold. // - // Fix: the deliberate-selection center is deferred until - // isThreadContextLoading transitions true → false (fetch settled). + // Fix: the Inbox layout target is deferred until context loading settles, + // then resolves against the pre-click read frontier. // // Fixture: // - fetchRoot + 10 older replies are in mockMessages only (fetch path). @@ -946,9 +993,8 @@ test.describe("inbox stable-conversation regressions", () => { // representative. // - The thread is long enough that the detail pane requires scrolling. // - // This test must FAIL at 5536cede5 (center fires before fetch, lands - // mid-thread) and PASS with the fix (center fires after fetch, lands on - // the newest message). + // This test must not latch the visible representative before fetched older + // replies reveal the real first-unread boundary. await installMockBridge(page); await page.goto("/"); @@ -963,11 +1009,17 @@ test.describe("inbox stable-conversation regressions", () => { const win = window as MockWindow; const emit = win.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; const push = win.__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__; - if (!emit || !push) throw new Error("Bridge helpers not ready"); + const emitReadState = win.__BUZZ_E2E_EMIT_MOCK_READ_STATE__; + if (!emit || !push || !emitReadState) { + throw new Error("Bridge helpers not ready"); + } + + const base = Math.floor(Date.now() / 1_000) - 30; const fetchRoot = emit({ channelName: "general", content: "Fetch-path thread root.", + createdAt: base, pubkey: senderPubkey, id: "d0".repeat(32), }); @@ -978,6 +1030,7 @@ test.describe("inbox stable-conversation regressions", () => { emit({ channelName: "general", content: `Fetch-path older reply ${i} — long enough to occupy vertical space in the pane so that the total thread height requires scrolling to see the newest message.`, + createdAt: base + i, parentEventId: fetchRoot.id, pubkey: senderPubkey, id: `d${i.toString(16).padStart(1, "0")}`.repeat(32), @@ -989,6 +1042,7 @@ test.describe("inbox stable-conversation regressions", () => { channelName: "general", content: "Fetch-path newest reply — the one the user clicks in inbox.", + createdAt: base + 20, parentEventId: fetchRoot.id, pubkey: senderPubkey, mentionPubkeys: [currentPubkey], @@ -1006,6 +1060,12 @@ test.describe("inbox stable-conversation regressions", () => { tags: fetchNewest.tags, category: "mention", }); + emitReadState({ + clientId: "inbox-fetch-boundary-client", + contexts: { [`thread:${fetchRoot.id}`]: base }, + createdAt: base + 25, + slotId: "inboxfetchboundary00000000000000", + }); return { fetchRoot, fetchNewest }; }, @@ -1016,11 +1076,33 @@ test.describe("inbox stable-conversation regressions", () => { }, ); + await page.waitForFunction( + ({ contextId, expectedReadAt }) => { + for (let index = 0; index < localStorage.length; index += 1) { + const key = localStorage.key(index); + if (!key?.startsWith("buzz.channel-read-state.v2:")) continue; + const raw = localStorage.getItem(key); + if (!raw) continue; + if ( + Date.parse(JSON.parse(raw)[contextId]) / 1_000 === + expectedReadAt + ) { + return true; + } + } + return false; + }, + { + contextId: `thread:${fetchRoot.id}`, + expectedReadAt: fetchRoot.created_at, + }, + ); + // Confirm the feed representative row appeared in the inbox list. const newestRow = page.getByTestId(`home-inbox-item-${fetchNewest.id}`); await expect(newestRow).toBeVisible(); - // Reset the spy so only the click's center is counted. + // Reset the spy so this layout open cannot hide a center fallback. await resetScrollIntoViewCount(page); // ── Click the newest reply row ──────────────────────────────────── @@ -1041,27 +1123,26 @@ test.describe("inbox stable-conversation regressions", () => { // After settle, confirm the older replies are now rendered (fetch landed). await expect(detail).toContainText("Fetch-path older reply 1"); - // ── Assert: selected message is within the pane viewport ───────── - // The selected message (fetchNewest) must be visible inside the scroll - // container — i.e. its bounding rect overlaps with the pane's visible area. - const isSelectedMessageInViewport = await page.evaluate(() => { - const selectedMsg = document.querySelector( - '[data-testid="home-inbox-selected-message"]', + // The first fetched reply is the first unread message in this never-read + // thread, so its divider and row must be visible after context settles. + const firstUnreadIsInViewport = await page.evaluate(() => { + const firstUnread = document.querySelector( + `[data-message-id="${"d1".repeat(32)}"]`, ); const pane = document.querySelector( '[data-testid="home-inbox-detail"] [aria-busy]', ); - if (!selectedMsg || !pane) return false; + if (!firstUnread || !pane) return false; const paneRect = pane.getBoundingClientRect(); - const msgRect = selectedMsg.getBoundingClientRect(); - // At least the top half of the element must overlap the pane's visible area. + const msgRect = firstUnread.getBoundingClientRect(); const msgCenter = msgRect.top + msgRect.height / 2; return msgCenter >= paneRect.top && msgCenter <= paneRect.bottom; }); - expect(isSelectedMessageInViewport).toBe(true); + expect(firstUnreadIsInViewport).toBe(true); + await expect(page.getByTestId("message-unread-divider")).toBeVisible(); - // ── Assert: exactly one programmatic scroll fired ───────────────── - expect(await getScrollIntoViewCount(page)).toBe(1); + // Layout targets use container scroll placement, never center fallback. + expect(await getScrollIntoViewCount(page)).toBe(0); // ── Bonus: passive live arrivals after settle still trigger zero ── // Inject a sibling to confirm the scroll-stability invariant is intact. @@ -1102,8 +1183,242 @@ test.describe("inbox stable-conversation regressions", () => { await expect( page.getByTestId(`home-inbox-item-${"de".repeat(32)}`), ).toBeVisible(); - // Count must remain 1 — passive arrival must not trigger another center. - expect(await getScrollIntoViewCount(page)).toBe(1); + // Passive arrival must not trigger a center. + expect(await getScrollIntoViewCount(page)).toBe(0); + }); + + test("long inbox thread opens at its frozen unread boundary, then reopens at bottom", async ({ + page, + }) => { + await installMockBridge(page); + await page.goto("/"); + await expect(getListPane(page)).toBeVisible(); + await waitForBridgeReady(page); + + const { firstUnreadId, longNewest, otherReply } = await page.evaluate( + ({ channelId, currentPubkey, senderPubkey }) => { + const win = window as MockWindow; + const emit = win.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + const push = win.__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__; + const emitReadState = win.__BUZZ_E2E_EMIT_MOCK_READ_STATE__; + if (!emit || !push || !emitReadState) { + throw new Error("Bridge helpers not ready"); + } + + const base = Math.floor(Date.now() / 1_000) - 100; + const longRoot = emit({ + channelName: "general", + content: "Long reopen thread root.", + createdAt: base, + pubkey: senderPubkey, + id: "91".repeat(32), + }); + let firstUnreadId = ""; + for (let index = 0; index < 40; index += 1) { + const reply = emit({ + channelName: "general", + content: `Long reopen older reply ${index + 1} — enough body text to make repeated selection and late layout changes exercise a genuinely tall inbox conversation.`, + createdAt: base + index + 1, + parentEventId: longRoot.id, + pubkey: senderPubkey, + id: `${(0xa0 + index).toString(16).padStart(2, "0")}`.repeat(32), + }); + if (index === 30) firstUnreadId = reply.id; + } + const longNewest = emit({ + channelName: "general", + content: "Long reopen newest reply — this must remain visible.", + createdAt: base + 50, + parentEventId: longRoot.id, + pubkey: senderPubkey, + mentionPubkeys: [currentPubkey], + id: "d1".repeat(32), + }); + push({ + id: longNewest.id, + kind: longNewest.kind, + pubkey: longNewest.pubkey, + content: longNewest.content, + created_at: longNewest.created_at, + channel_id: channelId, + channel_name: "general", + tags: longNewest.tags, + category: "mention", + }); + emitReadState({ + clientId: "inbox-open-boundary-client", + contexts: { [`thread:${longRoot.id}`]: base + 30 }, + createdAt: base + 70, + slotId: "inboxopenboundary000000000000000", + }); + + const otherRoot = emit({ + channelName: "general", + content: "Other reopen thread root.", + createdAt: base + 60, + pubkey: senderPubkey, + id: "e1".repeat(32), + }); + const otherReply = emit({ + channelName: "general", + content: "Other reopen thread reply.", + createdAt: base + 61, + parentEventId: otherRoot.id, + pubkey: senderPubkey, + mentionPubkeys: [currentPubkey], + id: "e2".repeat(32), + }); + push({ + id: otherReply.id, + kind: otherReply.kind, + pubkey: otherReply.pubkey, + content: otherReply.content, + created_at: otherReply.created_at, + channel_id: channelId, + channel_name: "general", + tags: otherReply.tags, + category: "mention", + }); + + return { firstUnreadId, longNewest, otherReply }; + }, + { + channelId: GENERAL_CHANNEL_ID, + currentPubkey: TEST_IDENTITIES.tyler.pubkey, + senderPubkey: TEST_IDENTITIES.alice.pubkey, + }, + ); + + await page.waitForFunction( + ({ contextId, expectedReadAt }) => { + for (let index = 0; index < localStorage.length; index += 1) { + const key = localStorage.key(index); + if (!key?.startsWith("buzz.channel-read-state.v2:")) continue; + const raw = localStorage.getItem(key); + if (!raw) continue; + const value = JSON.parse(raw)[contextId]; + if (Date.parse(value) / 1_000 === expectedReadAt) return true; + } + return false; + }, + { + contextId: `thread:${"91".repeat(32)}`, + expectedReadAt: longNewest.created_at - 20, + }, + ); + + const longRow = page.getByTestId(`home-inbox-item-${longNewest.id}`); + const otherRow = page.getByTestId(`home-inbox-item-${otherReply.id}`); + await expect(longRow).toBeVisible(); + await expect(otherRow).toBeVisible(); + + await longRow.click(); + await expect(getDetailPane(page)).toContainText( + "Long reopen older reply 1", + ); + await page.waitForFunction(() => { + const scroller = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + return scroller?.getAttribute("aria-busy") !== "true"; + }); + await expect(page.getByTestId("message-unread-divider")).toBeVisible(); + await expect + .poll(() => + page.evaluate( + ({ targetId }) => { + const scroller = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + const divider = scroller?.querySelector( + '[data-testid="message-unread-divider"]', + ); + const target = scroller?.querySelector( + `[data-message-id="${targetId}"]`, + ); + if (!scroller || !divider || !target) return null; + return { + dividerNearTop: (() => { + const offset = + divider.getBoundingClientRect().top - + scroller.getBoundingClientRect().top; + return offset >= 0 && offset <= 40; + })(), + targetVisible: + target.getBoundingClientRect().top < + scroller.getBoundingClientRect().bottom, + }; + }, + { targetId: firstUnreadId }, + ), + ) + .toEqual({ dividerNearTop: true, targetVisible: true }); + await page.evaluate(() => { + const scroller = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + if (!scroller) throw new Error("Expected inbox detail scroller"); + (window as MockWindow).__BUZZ_E2E_RETIRED_INBOX_SCROLLER__ = scroller; + }); + + await otherRow.click(); + await expect(getDetailPane(page)).toContainText( + "Other reopen thread reply", + ); + await longRow.click(); + await expect(getDetailPane(page)).toContainText( + "Long reopen older reply 1", + ); + await page.waitForFunction(() => { + const scroller = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + return scroller?.getAttribute("aria-busy") !== "true"; + }); + + const liveScrollerIsAtBottom = () => + page.evaluate(() => { + const scroller = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + if (!scroller) return false; + return ( + Math.abs( + scroller.scrollHeight - scroller.clientHeight - scroller.scrollTop, + ) <= 1 + ); + }); + await expect(page.getByTestId("message-unread-divider")).toHaveCount(0); + await expect.poll(liveScrollerIsAtBottom).toBe(true); + expect( + await page.evaluate(() => { + const retired = (window as MockWindow) + .__BUZZ_E2E_RETIRED_INBOX_SCROLLER__; + const live = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + return { + retiredIsConnected: retired?.isConnected ?? null, + sameNode: retired === live, + }; + }), + ).toEqual({ retiredIsConnected: false, sameNode: false }); + + await page.evaluate(() => { + const retired = (window as MockWindow) + .__BUZZ_E2E_RETIRED_INBOX_SCROLLER__; + if (!retired) throw new Error("Expected retired inbox detail scroller"); + retired.dispatchEvent(new Event("scroll", { bubbles: true })); + + const live = document.querySelector( + '[data-testid="home-inbox-detail-scroll"]', + ); + const firstRow = live?.querySelector("[data-message-id]"); + if (!firstRow) throw new Error("Expected a rendered inbox message row"); + firstRow.style.minHeight = `${firstRow.getBoundingClientRect().height + 1_200}px`; + }); + + await expect.poll(liveScrollerIsAtBottom).toBe(true); }); test("post-settle reaction hydration does not shift the centered selected message out of viewport", async ({ @@ -1210,11 +1525,9 @@ test.describe("inbox stable-conversation regressions", () => { const newestRow = page.getByTestId(`home-inbox-item-${fetchNewest.id}`); await expect(newestRow).toBeVisible(); - // Reset the spy so only the center from the click is counted. + // A URL-owned target is an explicit deep-link and remains centered. await resetScrollIntoViewCount(page); - - // ── Click the newest reply row ──────────────────────────────────── - await newestRow.click(); + await navigateToInboxItem(page, fetchNewest.id); const detail = getDetailPane(page); await expect(detail).toContainText("Reaction-drift selected reply"); @@ -1325,12 +1638,12 @@ test.describe("inbox stable-conversation regressions", () => { expect(await getScrollIntoViewCount(page)).toBe(1); }); - test("direct container scroll releases the hold before late reaction hydration", async ({ + test("pointer-driven container scroll releases the hold before late reaction hydration", async ({ page, }) => { - // A direct scroll (such as a scrollbar drag) must release the post-center - // anchor hold before late reaction hydration. The test asserts the user - // action is recognized without wheel, touch, or key events. + // A scrollbar drag must release the post-center anchor hold before late + // reaction hydration. Pointer-down supplies the user-intent signal; the + // following native scroll event must not be confused with target settling. // // Fixture (same shape as test 6): // - fetchRoot + 10 older replies in mockMessages only (fetch path). @@ -1421,11 +1734,9 @@ test.describe("inbox stable-conversation regressions", () => { const newestRow = page.getByTestId(`home-inbox-item-${fetchNewest.id}`); await expect(newestRow).toBeVisible(); - // Reset the spy so only the center from the click is counted. + // A URL-owned target is an explicit deep-link and remains centered. await resetScrollIntoViewCount(page); - - // ── Click the newest reply row ──────────────────────────────────── - await newestRow.click(); + await navigateToInboxItem(page, fetchNewest.id); const detail = getDetailPane(page); await expect(detail).toContainText("Reaction-drift selected reply"); @@ -1441,9 +1752,8 @@ test.describe("inbox stable-conversation regressions", () => { await expect(detail).toContainText("Reaction-drift older reply 1"); expect(await getScrollIntoViewCount(page)).toBe(1); - // Simulate a scrollbar drag: change the actual scroll container directly - // and dispatch only `scroll`, without wheel/touch/key input. This must - // release the post-center hold before late layout growth arrives. + // Simulate a scrollbar drag: pointer-down on the actual scroll container, + // then change its offset and dispatch the resulting native scroll event. const scrollTopAfterDirectScroll = await page.evaluate(() => { const pane = document.querySelector( '[data-testid="home-inbox-detail"] [aria-busy]', @@ -1457,6 +1767,7 @@ test.describe("inbox stable-conversation regressions", () => { }; (window as Window & { __scrollByCalls?: () => number }).__scrollByCalls = () => scrollByCalls; + pane.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); const scrollTopBefore = pane.scrollTop; pane.scrollTop = Math.min( pane.scrollHeight - pane.clientHeight, diff --git a/desktop/tests/e2e/thread-unread.spec.ts b/desktop/tests/e2e/thread-unread.spec.ts index ec3c53b1f2..a342fa80a0 100644 --- a/desktop/tests/e2e/thread-unread.spec.ts +++ b/desktop/tests/e2e/thread-unread.spec.ts @@ -122,7 +122,7 @@ async function expectAtBottom( .toBeLessThanOrEqual(1); } -async function expectInsideScrollRegion( +async function expectNearScrollRegionTop( target: import("@playwright/test").Locator, scrollRegion: import("@playwright/test").Locator, ) { @@ -132,14 +132,18 @@ async function expectInsideScrollRegion( target.boundingBox(), scrollRegion.boundingBox(), ]); - return Boolean( - targetBox && - regionBox && - targetBox.y >= regionBox.y && - targetBox.y + targetBox.height <= regionBox.y + regionBox.height, - ); + if (!targetBox || !regionBox) { + return { nearTop: false, regionBox, targetBox, topOffset: null }; + } + const topOffset = targetBox.y - regionBox.y; + return { + nearTop: topOffset >= 0 && topOffset <= 24, + regionBox, + targetBox, + topOffset, + }; }) - .toBe(true); + .toMatchObject({ nearTop: true }); } test.describe("thread unread indicator", () => { @@ -227,10 +231,23 @@ test.describe("thread unread indicator", () => { await page.getByTestId("auxiliary-panel-close").click(); await expect(page.getByTestId("message-thread-panel")).not.toBeVisible(); - // Re-opening with every reply read keeps the existing bottom behavior. + // Re-opening with every reply read targets the last reply through the + // race-safe id path and lands at the physical bottom. await threadSummary.click(); const threadBody = page.getByTestId("message-thread-body"); await expectAtBottom(threadBody); + + // Model a late image/embed expansion after the target resolves. The + // all-read anchor must remain bottom-glued through ResizeObserver rather + // than freezing at the old floor. + await threadBody.evaluate((element) => { + const lastRow = + element.querySelectorAll("[data-message-id]"); + const row = lastRow.item(lastRow.length - 1); + if (!row) throw new Error("Expected a rendered thread reply"); + row.style.minHeight = `${row.getBoundingClientRect().height + 800}px`; + }); + await expectAtBottom(threadBody); await page.getByTestId("auxiliary-panel-close").click(); await expect(page.getByTestId("message-thread-panel")).not.toBeVisible(); @@ -254,11 +271,12 @@ test.describe("thread unread indicator", () => { await page.getByTestId("message-thread-summary").first().click(); await expect(page.getByTestId("message-thread-panel")).toBeVisible(); - // The unread divider should already be visible above the first unread - // reply. Do not scroll it into view: that would hide a bottom-pin failure. + // The unread divider should already be visible near the top above the + // first unread reply. Do not scroll it into view: that would hide an + // alignment failure. const divider = page.getByTestId("message-unread-divider"); await expect(divider).toBeVisible(); - await expectInsideScrollRegion(divider, threadBody); + await expectNearScrollRegionTop(divider, threadBody); }); test("03-thread-badge-casual-browse", async ({ page }) => {