From 784067729c660d45371abe0670bb37a8c8212745 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 12:51:19 -0400 Subject: [PATCH 1/8] fix(channels): read a channel and its open thread when returning to it Returning to a channel with its thread pane open moved focus to the thread's tab and skipped the main composer, so only the thread was read and the channel stayed unread. Focus returns to the main composer, and while it stays in that channel's workspace the visible selected thread reads too. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- docs/unread.md | 14 +++-- src/bundled/channels/ChannelsPage.tsx | 31 ++--------- src/features/messages/use-reading.test.ts | 24 +++++++++ src/features/messages/use-reading.ts | 3 ++ tests/browser/channel-tabs.spec.mjs | 6 +-- tests/browser/thread-unread.spec.mjs | 65 ++++++++++++++++++++++- 6 files changed, 108 insertions(+), 35 deletions(-) diff --git a/docs/unread.md b/docs/unread.md index 6443f8da68..98a1c33f86 100644 --- a/docs/unread.md +++ b/docs/unread.md @@ -38,10 +38,16 @@ visible rows, and 300 ms dwell. Scroll/content/focus changes cancel/restart dwel scheduler when positioning finishes, even if an empty history page changes no rows or geometry. The list and its own composer share a reading surface; focus anywhere in an open -thread panel also qualifies, including its selected owning tab header. Explicitly returning to a channel resumes reading in its visible selected thread for that channel; a hidden pane or another channel’s tab keeps the main conversation’s focus. Pane motion completion restarts observation after unchanged rows enter the viewport. Inactive -or hidden tab content and passive restoration do not qualify. Another pane, a -dialog, background window, preload, -and mounted virtualizer overscan do not qualify. +thread panel also qualifies, including its selected owning tab header. Explicitly +returning to a channel reads both its main conversation and its visible selected +thread tab for that channel: while focus stays anywhere in that channel's +workspace (normally the main composer), the thread panel qualifies too. A hidden +or collapsed pane, another channel's tab, a members or media-review view, and a +passive restoration (`preserve` focus) keep the main conversation as the only +reading surface. Inactive or hidden tab content does not qualify. Pane motion +completion restarts observation after unchanged rows enter the viewport. Another +pane, a dialog, background window, preload, and mounted virtualizer overscan do +not qualify. Away from the bottom, observations mark individual fully visible messages. At the physical bottom, 300 ms visibility of the newest message's bottom edge also earns diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index 2b102703e5..d9e3aa7fc8 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -1304,8 +1304,9 @@ function ChannelWorkspace({ const hasPanel = companion || hasChannelPanel; const showingChannelPanel = tabState.paneOpen && hasChannelPanel; const showingPanel = companion || showingChannelPanel; - // An explicit channel opening resumes its visible selected thread. Passive - // restoration and another channel’s retained tab keep their existing focus. + // An explicit channel opening reads its visible selected thread along with + // the conversation that holds focus. Passive restoration and another + // channel’s retained tab do not. const readingThread = showingChannelPanel && !companion && @@ -1319,28 +1320,6 @@ function ChannelWorkspace({ tab.channelId === currentId, ); const split = usePanelSplit(); - const readingVisit = useRef(undefined); - useLayoutEffect(() => { - const visit = navigation?.signal ?? currentId; - if (!readingThread || readingVisit.current === visit) return; - readingVisit.current = visit; - const focused = document.activeElement; - // Only hand off the channel opener. A person who moved focus while the - // retained pane mounted keeps their new target. - if ( - focused !== document.body && - (!(focused instanceof Element) || - focused - .closest("[data-channel-id]") - ?.getAttribute("data-channel-id") !== currentId) - ) - return; - split.ref.current - ?.querySelector( - `[role="tab"][data-tab-value="${CSS.escape(selectedTab)}"]`, - ) - ?.focus({ preventScroll: true }); - }, [readingThread, navigation?.signal, currentId, selectedTab, split.ref]); useEffect(() => { // Only an explicit settings click moves focus; restoring a visit does not. if (settingsFocus) @@ -1514,8 +1493,7 @@ function ChannelWorkspace({ !current.readOnly && !requestedMessage && !requestedThread && - !routedThread && - !readingThread + !routedThread } trailingTool={ solo ? ( @@ -1535,6 +1513,7 @@ function ChannelWorkspace({ const workspace = (
{ + const h = setup({ focus: "outside" }); + const workspace = document.createElement("div"); + const surface = h.element.closest("aside"); + const unrelated = document.createElement("button"); + expect(surface).not.toBeNull(); + document.body.append(workspace, unrelated); + if (surface) workspace.append(surface, h.outside); + h.retarget("thread-room"); + h.outside.focus(); + vi.advanceTimersByTime(300); + expect(h.reading).not.toHaveBeenCalled(); + workspace.setAttribute("data-reading-thread", ""); + h.retarget("thread-room-again"); + h.outside.focus(); + vi.advanceTimersByTime(299); + expect(h.leases.at(-1)?.observe).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(h.dwelled()?.observe).toHaveBeenCalledExactlyOnceWith(["visible"]); + const count = h.reading.mock.calls.length; + unrelated.focus(); + vi.advanceTimersByTime(300); + expect(h.reading).toHaveBeenCalledTimes(count); +}); it("focus moving from the list to its composer keeps reading; another surface's composer cancels it", () => { const h = setup(); vi.advanceTimersByTime(100); diff --git a/src/features/messages/use-reading.ts b/src/features/messages/use-reading.ts index 962c43f57e..d8669fd8d1 100644 --- a/src/features/messages/use-reading.ts +++ b/src/features/messages/use-reading.ts @@ -70,6 +70,9 @@ export function useReading({ (element.contains(node) || !!surface?.contains(node) || focusedTab(node) || + // The workspace reads its visible selected thread while its channel + // conversation holds focus after an explicit visit. + !!surface?.closest("[data-reading-thread]")?.contains(node) || !!composer?.current?.contains(node)); // Everything but geometry, so an event can check it without a layout. const ready = () => diff --git a/tests/browser/channel-tabs.spec.mjs b/tests/browser/channel-tabs.spec.mjs index 7b40d345d2..e733118ffd 100644 --- a/tests/browser/channel-tabs.spec.mjs +++ b/tests/browser/channel-tabs.spec.mjs @@ -270,10 +270,8 @@ test("content-opened tabs restore drafts without offering blank-tab creation", a exact: true, }); await expect(alphaComposer).toHaveText("Alpha draft"); - // Returning to Alpha resumes reading in its visible selected thread. - await expect( - workspace.locator('[role="tab"][data-tab-value^="thread:alpha:"]'), - ).toBeFocused(); + // Restored side content must not take focus from the main conversation. + await expect(alphaComposer).toBeFocused(); await workspace.getByRole("tab", { name: "Beta", exact: true }).click(); await hoverMessage(row); await row.getByRole("button", { name: "Reply", exact: true }).click(); diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 5710a681fe..a3d16474c4 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -769,7 +769,7 @@ for (const { paneOpen, width } of [ panel.getByText("Unread reply 1", { exact: true }), ).toBeInViewport(); await expect( - page.locator(`[role="tab"][data-tab-value="thread:alpha:${root.id}"]`), + page.getByRole("textbox", { name: "Message #Alpha", exact: true }), ).toBeFocused(); await expect.poll(unread).toBe(0); // The other thread was never opened and remains unread. @@ -782,3 +782,66 @@ for (const { paneOpen, width } of [ .getByRole("button", { name: /^View thread:/ }), ).toHaveAccessibleName(/Observed unread replies/); }); + +// An explicit return reads the whole visit: the channel's own conversation, which +// holds focus in its composer, and the visible selected thread beside it. +test("returning to a channel reads both its conversation and its open thread", async ({ + page, + app, +}) => { + await page.clock.install(); + await holdReadingFocus(page); + await open(page, app); + const root = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 1"); + const alpha = page.locator("button[data-channel-id=alpha]"); + const beta = page.locator("button[data-channel-id=beta]"); + const panel = page.getByRole("complementary", { + name: "Thread", + exact: true, + }); + const observed = (target) => + page.evaluate( + (target) => + window.fixtureRelay.snapshot().session.unread.snapshot(target) + .observedCount, + target, + ); + const channelUnread = () => observed({ kind: "channel", channelId: "alpha" }); + const threadUnread = (rootId = root.id) => + observed({ kind: "thread", channelId: "alpha", rootId }); + // The neighboring thread is never opened, so its replies stay unread and + // are the only unread left in the channel. + const otherRoot = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 0"); + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect( + panel.getByText("Unread reply 1", { exact: true }), + ).toBeInViewport(); + // Setup must stay unread even when opening and switching channels is slow. + await page.clock.runFor(1000); + await beta.click(); + await expect( + page.getByRole("textbox", { name: "Message #Beta", exact: true }), + ).toBeVisible(); + expect(await threadUnread()).toBeGreaterThan(0); + const neighbor = () => threadUnread(otherRoot.id); + expect(await neighbor()).toBeGreaterThan(0); + expect(await channelUnread()).toBeGreaterThan(await neighbor()); + await releaseReadingFocus(page); + await alpha.click(); + await expect( + panel.getByText("Unread reply 1", { exact: true }), + ).toBeInViewport(); + await expect.poll(threadUnread).toBe(0); + // The conversation's own messages are read; only the neighbor remains. + await expect + .poll(async () => (await channelUnread()) - (await neighbor())) + .toBe(0); + expect(await neighbor()).toBeGreaterThan(0); +}); From 265700d7010d90f27482d220078fae63db33b667 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 12:55:09 -0400 Subject: [PATCH 2/8] test(browser): check channel reading in the existing return journey Fold the channel-versus-neighbor assertions into the open-pane branch of the returning-to-a-channel journey instead of repeating its setup. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- tests/browser/thread-unread.spec.mjs | 96 ++++++---------------------- 1 file changed, 20 insertions(+), 76 deletions(-) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index a3d16474c4..beedf9130b 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -709,7 +709,7 @@ for (const { paneOpen, width } of [ { paneOpen: true, width: 1280 }, { paneOpen: false, width: 1440 }, ]) - test(`returning to a channel ${paneOpen ? "reads its already-open thread" : "leaves its hidden thread unread"} at ${width}px`, async ({ + test(`returning to a channel ${paneOpen ? "reads it and its already-open thread" : "leaves its hidden thread unread"} at ${width}px`, async ({ page, app, }) => { @@ -726,16 +726,23 @@ for (const { paneOpen, width } of [ name: "Thread", exact: true, }); - const unread = () => + const otherRoot = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 0"); + const observed = (target) => page.evaluate( - (rootId) => - window.fixtureRelay.snapshot().session.unread.snapshot({ - kind: "thread", - channelId: "alpha", - rootId, - }).observedCount, - root.id, + (target) => + window.fixtureRelay.snapshot().session.unread.snapshot(target) + .observedCount, + target, ); + const unread = (rootId = root.id) => + observed({ kind: "thread", channelId: "alpha", rootId }); + // The neighboring thread is never opened, so once the channel is read its + // replies are all that remains unread there. + const unreadBeyondNeighbor = async () => + (await observed({ kind: "channel", channelId: "alpha" })) - + (await unread(otherRoot.id)); await page .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) .getByRole("button", { name: /^View thread:/ }) @@ -754,6 +761,7 @@ for (const { paneOpen, width } of [ page.getByRole("textbox", { name: "Message #Beta", exact: true }), ).toBeVisible(); expect(await unread()).toBeGreaterThan(0); + expect(await unreadBeyondNeighbor()).toBeGreaterThan(0); await releaseReadingFocus(page); await alpha.click(); if (!paneOpen) { @@ -771,77 +779,13 @@ for (const { paneOpen, width } of [ await expect( page.getByRole("textbox", { name: "Message #Alpha", exact: true }), ).toBeFocused(); - await expect.poll(unread).toBe(0); + await expect.poll(() => unread()).toBe(0); + // The channel's own conversation is read too. + await expect.poll(unreadBeyondNeighbor).toBe(0); // The other thread was never opened and remains unread. - const otherRoot = app.histories - .get("primary/alpha") - .find((row) => row.content === "Thread root 0"); await expect( page .locator(`[data-channel-timeline] [data-message-id="${otherRoot.id}"]`) .getByRole("button", { name: /^View thread:/ }), ).toHaveAccessibleName(/Observed unread replies/); }); - -// An explicit return reads the whole visit: the channel's own conversation, which -// holds focus in its composer, and the visible selected thread beside it. -test("returning to a channel reads both its conversation and its open thread", async ({ - page, - app, -}) => { - await page.clock.install(); - await holdReadingFocus(page); - await open(page, app); - const root = app.histories - .get("primary/alpha") - .find((row) => row.content === "Thread root 1"); - const alpha = page.locator("button[data-channel-id=alpha]"); - const beta = page.locator("button[data-channel-id=beta]"); - const panel = page.getByRole("complementary", { - name: "Thread", - exact: true, - }); - const observed = (target) => - page.evaluate( - (target) => - window.fixtureRelay.snapshot().session.unread.snapshot(target) - .observedCount, - target, - ); - const channelUnread = () => observed({ kind: "channel", channelId: "alpha" }); - const threadUnread = (rootId = root.id) => - observed({ kind: "thread", channelId: "alpha", rootId }); - // The neighboring thread is never opened, so its replies stay unread and - // are the only unread left in the channel. - const otherRoot = app.histories - .get("primary/alpha") - .find((row) => row.content === "Thread root 0"); - await page - .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) - .getByRole("button", { name: /^View thread:/ }) - .click(); - await expect( - panel.getByText("Unread reply 1", { exact: true }), - ).toBeInViewport(); - // Setup must stay unread even when opening and switching channels is slow. - await page.clock.runFor(1000); - await beta.click(); - await expect( - page.getByRole("textbox", { name: "Message #Beta", exact: true }), - ).toBeVisible(); - expect(await threadUnread()).toBeGreaterThan(0); - const neighbor = () => threadUnread(otherRoot.id); - expect(await neighbor()).toBeGreaterThan(0); - expect(await channelUnread()).toBeGreaterThan(await neighbor()); - await releaseReadingFocus(page); - await alpha.click(); - await expect( - panel.getByText("Unread reply 1", { exact: true }), - ).toBeInViewport(); - await expect.poll(threadUnread).toBe(0); - // The conversation's own messages are read; only the neighbor remains. - await expect - .poll(async () => (await channelUnread()) - (await neighbor())) - .toBe(0); - expect(await neighbor()).toBeGreaterThan(0); -}); From 2e6352fa10268443a06d06fdc5b76ff1a609c654 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 14:00:48 -0400 Subject: [PATCH 3/8] fix(channels): read an open thread only on plain channel visits A link to a top-level message focuses the main timeline, but the already-open selected thread still read beside it. Only a plain channel visit, with no requested message or thread, reads that thread now. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- docs/unread.md | 15 ++++--- src/bundled/channels/ChannelsPage.tsx | 8 ++-- tests/browser/thread-unread.spec.mjs | 63 +++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/docs/unread.md b/docs/unread.md index 98a1c33f86..20260300fe 100644 --- a/docs/unread.md +++ b/docs/unread.md @@ -41,13 +41,14 @@ The list and its own composer share a reading surface; focus anywhere in an open thread panel also qualifies, including its selected owning tab header. Explicitly returning to a channel reads both its main conversation and its visible selected thread tab for that channel: while focus stays anywhere in that channel's -workspace (normally the main composer), the thread panel qualifies too. A hidden -or collapsed pane, another channel's tab, a members or media-review view, and a -passive restoration (`preserve` focus) keep the main conversation as the only -reading surface. Inactive or hidden tab content does not qualify. Pane motion -completion restarts observation after unchanged rows enter the viewport. Another -pane, a dialog, background window, preload, and mounted virtualizer overscan do -not qualify. +workspace (normally the main composer), the thread panel qualifies too. A link to +a message or thread (Activity, a message link) is not a plain visit and reads +only what holds focus. A hidden or collapsed pane, another channel's tab, a +members or media-review view, and a passive restoration (`preserve` focus) keep +the main conversation as the only reading surface. Inactive or hidden tab +content does not qualify. Pane motion completion restarts observation after +unchanged rows enter the viewport. Another pane, a dialog, background window, +preload, and mounted virtualizer overscan do not qualify. Away from the bottom, observations mark individual fully visible messages. At the physical bottom, 300 ms visibility of the newest message's bottom edge also earns diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index d9e3aa7fc8..9057389007 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -1304,12 +1304,14 @@ function ChannelWorkspace({ const hasPanel = companion || hasChannelPanel; const showingChannelPanel = tabState.paneOpen && hasChannelPanel; const showingPanel = companion || showingChannelPanel; - // An explicit channel opening reads its visible selected thread along with - // the conversation that holds focus. Passive restoration and another - // channel’s retained tab do not. + // A plain channel visit reads its visible selected thread along with the + // conversation that holds focus. A link to a message or thread, passive + // restoration and another channel’s retained tab do not. const readingThread = showingChannelPanel && !companion && + !requestedMessage && + !requestedThread && !membersOpen && !showingMediaReview && (navigation?.focus ?? navigator?.snapshot().attempt.focus) !== "preserve" && diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index beedf9130b..d990cf815d 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -789,3 +789,66 @@ for (const { paneOpen, width } of [ .getByRole("button", { name: /^View thread:/ }), ).toHaveAccessibleName(/Observed unread replies/); }); + +// A link to a message reads only the conversation it focuses: the channel's +// already-open thread stays unread until the person goes there. +test("opening a message link reads the channel but not its already-open thread", async ({ + page, + app, +}) => { + await page.clock.install(); + await holdReadingFocus(page); + await open(page, app); + const history = app.histories.get("primary/alpha"); + const root = history.find((row) => row.content === "Thread root 1"); + const message = history.findLast( + (row) => !row.tags.some((tag) => tag[0] === "e"), + ); + const panel = page.getByRole("complementary", { + name: "Thread", + exact: true, + }); + const observed = (target) => + page.evaluate( + (target) => + window.fixtureRelay.snapshot().session.unread.snapshot(target) + .observedCount, + target, + ); + const unread = () => + observed({ kind: "thread", channelId: "alpha", rootId: root.id }); + const channel = () => observed({ kind: "channel", channelId: "alpha" }); + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect( + panel.getByText("Unread reply 1", { exact: true }), + ).toBeInViewport(); + // Setup must stay unread even when opening and switching channels is slow. + await page.clock.runFor(1000); + await page.locator("button[data-channel-id=beta]").click(); + await expect( + page.getByRole("textbox", { name: "Message #Beta", exact: true }), + ).toBeVisible(); + const before = await channel(); + expect(await unread()).toBeGreaterThan(0); + await releaseReadingFocus(page); + await page.evaluate((target) => window.fixtureNavigation.open(target), { + version: 1, + kind: "conversation", + channelId: "alpha", + messageId: message.id, + scope: { viewer: app.viewer, communityOrigin: "https://primary.example" }, + }); + await expect( + page.locator(`[data-channel-timeline] [data-message-id="${message.id}"]`), + ).toBeInViewport(); + await expect( + panel.getByText("Unread reply 1", { exact: true }), + ).toBeInViewport(); + // The channel earned its dwell; the thread had the same chance. + await expect.poll(channel).toBeLessThan(before); + await page.clock.runFor(1000); + expect(await unread()).toBeGreaterThan(0); +}); From 9ad1cae56601a90016ceb8a310f9155223e0c2a6 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 14:00:49 -0400 Subject: [PATCH 4/8] fix(channels): focus a routed thread once its collapsed pane opens Opening a thread from Activity while its channel's pane was collapsed on that thread tried to focus the tab while the dock was still inert. The pane then opened without a retry, so focus stayed on the body and the thread never read. Hand focus to the selected tab once the pane shows a routed thread, unless something else already took it. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- src/bundled/channels/ChannelsPage.tsx | 22 +++++++++ tests/browser/thread-unread.spec.mjs | 66 +++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index 9057389007..e9dfada1d9 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -1322,6 +1322,28 @@ function ChannelWorkspace({ tab.channelId === currentId, ); const split = usePanelSplit(); + // A routed thread can mount behind a collapsed pane, where it cannot take + // focus. Once the pane shows it, hand over focus nothing else took. + const showingRoutedThread = + showingChannelPanel && !companion && !membersOpen && !!routedThread; + const routedVisit = useRef(undefined); + useLayoutEffect(() => { + const visit = navigation?.signal ?? currentId; + if (!showingRoutedThread || routedVisit.current === visit) return; + routedVisit.current = visit; + if (document.activeElement !== document.body) return; + split.ref.current + ?.querySelector( + `[role="tab"][data-tab-value="${CSS.escape(selectedTab)}"]`, + ) + ?.focus({ preventScroll: true }); + }, [ + showingRoutedThread, + navigation?.signal, + currentId, + selectedTab, + split.ref, + ]); useEffect(() => { // Only an explicit settings click moves focus; restoring a visit does not. if (settingsFocus) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index d990cf815d..faffa1abcb 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -126,6 +126,72 @@ test.describe("mentioned reply priority", () => { } }); + test("opening a thread from Activity reads it when its channel's pane was collapsed", async ({ + page, + app, + }) => { + // Hold the clock so the thread cannot be read before Activity opens it. + await page.clock.install(); + await holdReadingFocus(page); + await open(page, app); + const root = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 1"); + const panel = page.getByRole("complementary", { + name: "Thread", + exact: true, + }); + const unread = () => + page.evaluate( + (rootId) => + window.fixtureRelay.snapshot().session.unread.snapshot({ + kind: "thread", + channelId: "alpha", + rootId, + }), + root.id, + ); + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect(panel).toBeVisible(); + await page.clock.runFor(1000); + // Collapse Alpha's pane around its selected thread, then leave the channel. + await page + .getByRole("button", { name: "Toggle tab pane", exact: true }) + .click(); + await page.locator("button[data-channel-id=beta]").click(); + await expect( + page.getByRole("textbox", { name: "Message #Beta", exact: true }), + ).toBeVisible(); + expect((await unread()).observedCount).toBeGreaterThan(0); + await page.locator('button[data-channel-id="alpha"]').hover(); + const item = page + .getByRole("dialog", { name: "Activity in Alpha" }) + .getByRole("button", { name: /Open unread thread from/ }) + .filter({ hasText: "Unread reply 1" }); + await expect(item).toBeVisible(); + await releaseReadingFocus(page); + await item.click(); + await expect(panel).toBeVisible(); + await page.clock.runFor(1000); + try { + await expect.poll(async () => (await unread()).observedCount).toBe(0); + } catch (error) { + const focus = await page.evaluate(() => ({ + active: + document.activeElement?.getAttribute("aria-label") ?? + document.activeElement?.tagName, + inThread: !!document.activeElement?.closest("[data-reading-surface]"), + })); + throw new Error( + `The thread opened but stayed unread. Focus: ${JSON.stringify(focus)}`, + { cause: error }, + ); + } + }); + test("an Activity reopening leaves focus the person moved meanwhile", async ({ page, app, From 812c492d514b92c34f1abded7bd26650b6b6eba1 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 15:22:43 -0400 Subject: [PATCH 5/8] fix(messages): limit the workspace reading grant to the side pane The channel workspace marker comes from the side pane's selected thread, but it also let a main-pane thread read while focus rested in the side pane, so a live reply in main-pane X was read while the person typed in side-pane T. Only a reading surface inside the side pane takes the grant. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- docs/unread.md | 5 +- src/features/messages/use-reading.test.ts | 13 +++- src/features/messages/use-reading.ts | 9 ++- tests/browser/thread-unread.spec.mjs | 82 +++++++++++++++++++++++ 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/docs/unread.md b/docs/unread.md index 20260300fe..040135b9a5 100644 --- a/docs/unread.md +++ b/docs/unread.md @@ -40,8 +40,9 @@ rows or geometry. The list and its own composer share a reading surface; focus anywhere in an open thread panel also qualifies, including its selected owning tab header. Explicitly returning to a channel reads both its main conversation and its visible selected -thread tab for that channel: while focus stays anywhere in that channel's -workspace (normally the main composer), the thread panel qualifies too. A link to +side-pane thread tab for that channel: while focus stays anywhere in that +channel's workspace (normally the main composer), that side-pane thread +qualifies too. A thread in the main pane never borrows this grant. A link to a message or thread (Activity, a message link) is not a plain visit and reads only what holds focus. A hidden or collapsed pane, another channel's tab, a members or media-review view, and a passive restoration (`preserve` focus) keep diff --git a/src/features/messages/use-reading.test.ts b/src/features/messages/use-reading.test.ts index e5f92ac7d3..1e32932836 100644 --- a/src/features/messages/use-reading.test.ts +++ b/src/features/messages/use-reading.test.ts @@ -343,11 +343,14 @@ it("only the owning selected tab earns dwell; inactive and restored content does it("a panel in a reading-thread workspace reads while focus is elsewhere in that workspace", () => { const h = setup({ focus: "outside" }); const workspace = document.createElement("div"); + const side = document.createElement("div"); + side.setAttribute("data-panel-workspace", ""); const surface = h.element.closest("aside"); const unrelated = document.createElement("button"); expect(surface).not.toBeNull(); document.body.append(workspace, unrelated); - if (surface) workspace.append(surface, h.outside); + if (surface) side.append(surface); + workspace.append(side, h.outside); h.retarget("thread-room"); h.outside.focus(); vi.advanceTimersByTime(300); @@ -363,6 +366,14 @@ it("a panel in a reading-thread workspace reads while focus is elsewhere in that unrelated.focus(); vi.advanceTimersByTime(300); expect(h.reading).toHaveBeenCalledTimes(count); + // A thread in the main pane belongs to no side-pane selection: the same + // workspace focus does not read it. + side.setAttribute("data-main-workspace", ""); + side.removeAttribute("data-panel-workspace"); + h.retarget("thread-room-main"); + h.outside.focus(); + vi.advanceTimersByTime(300); + expect(h.reading).toHaveBeenCalledTimes(count); }); it("focus moving from the list to its composer keeps reading; another surface's composer cancels it", () => { const h = setup(); diff --git a/src/features/messages/use-reading.ts b/src/features/messages/use-reading.ts index d8669fd8d1..88e95a2a54 100644 --- a/src/features/messages/use-reading.ts +++ b/src/features/messages/use-reading.ts @@ -70,9 +70,12 @@ export function useReading({ (element.contains(node) || !!surface?.contains(node) || focusedTab(node) || - // The workspace reads its visible selected thread while its channel - // conversation holds focus after an explicit visit. - !!surface?.closest("[data-reading-thread]")?.contains(node) || + // A plain channel visit also reads the side pane's visible selected + // thread while focus stays anywhere in that channel's workspace. + !!surface + ?.closest("[data-panel-workspace]") + ?.closest("[data-reading-thread]") + ?.contains(node) || !!composer?.current?.contains(node)); // Everything but geometry, so an event can check it without a layout. const ready = () => diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index faffa1abcb..0e192e9270 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -918,3 +918,85 @@ test("opening a message link reads the channel but not its already-open thread", await page.clock.runFor(1000); expect(await unread()).toBeGreaterThan(0); }); + +// A thread in the main pane is read only by focus in its own surface: a plain +// visit's workspace-wide grant belongs to the side pane's selected thread, so a +// live reply in main-pane X must stay unread while focus rests in side-pane T. +test("a live reply in a main-pane thread stays unread while focus is in the side-pane thread", async ({ + page, + app, +}) => { + await page.clock.install(); + await holdReadingFocus(page); + await open(page, app); + const roots = app.histories + .get("primary/alpha") + .filter((row) => row.content.startsWith("Thread root")); + for (const root of roots.slice(0, 2)) { + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect( + page.getByRole("complementary", { name: "Thread", exact: true }), + ).toBeVisible(); + } + const x = roots[1]; + const side = page.locator("[data-panel-workspace]"); + const main = page.locator("[data-main-workspace]"); + const tabs = (host) => + host.locator('[role="tab"][data-tab-value^="thread:"]'); + const unread = (rootId) => + page.evaluate( + (rootId) => + window.fixtureRelay.snapshot().session.unread.snapshot({ + kind: "thread", + channelId: "alpha", + rootId, + }).observedCount, + rootId, + ); + // Move X's tab (the second opened) to the main pane, leaving T in the side. + const ids = await tabs(side).evaluateAll((all) => + all.map((tab) => tab.dataset.tabValue), + ); + const moving = page.locator(`[role="tab"][data-tab-value="${ids[1]}"]`); + const from = await moving.boundingBox(); + const header = await main.locator("header").first().boundingBox(); + await page.mouse.move(from.x + from.width / 2, from.y + from.height / 2); + await page.mouse.down(); + await page.mouse.move( + header.x + header.width / 2, + header.y + header.height / 2, + { steps: 8 }, + ); + await page.mouse.up(); + await expect(tabs(main)).toHaveCount(1); + await expect(tabs(side)).toHaveCount(1); + await page.clock.runFor(1000); + expect(await unread(x.id)).toBeGreaterThan(0); + + // Leave and return by a plain sidebar click. + await page.locator("button[data-channel-id=beta]").click(); + await expect( + page.getByRole("textbox", { name: "Message #Beta", exact: true }), + ).toBeVisible(); + await releaseReadingFocus(page); + await page.locator("button[data-channel-id=alpha]").click(); + await expect( + page.getByRole("textbox", { name: "Message #Alpha", exact: true }), + ).toBeVisible(); + + // Select X (reading its own selected tab is legitimate), then park focus in + // T's composer. + await tabs(main).click(); + await expect.poll(() => unread(x.id)).toBe(0); + await side + .getByRole("textbox", { name: "Reply to thread", exact: true }) + .focus(); + app.reply(x.id); + await expect(main.getByText("New peer reply", { exact: true })).toBeVisible(); + await expect.poll(() => unread(x.id)).toBeGreaterThan(0); + await page.clock.runFor(1000); + expect(await unread(x.id)).toBeGreaterThan(0); +}); From 511b44b559c61aef31a5cde7ccf7cd8ce2efea65 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 20:32:11 -0400 Subject: [PATCH 6/8] fix(channels): hand focus to the routed thread's own tab Opening a thread from Activity when it was already selected in the main pane focused the side pane's selected thread instead, so the routed thread stayed unread and the other one was read. Focus the routed thread's tab wherever it is visible. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- src/bundled/channels/ChannelsPage.tsx | 29 ++++++----- tests/browser/thread-unread.spec.mjs | 71 +++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index e9dfada1d9..af2a3a19f1 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -1323,27 +1323,32 @@ function ChannelWorkspace({ ); const split = usePanelSplit(); // A routed thread can mount behind a collapsed pane, where it cannot take - // focus. Once the pane shows it, hand over focus nothing else took. - const showingRoutedThread = - showingChannelPanel && !companion && !membersOpen && !!routedThread; + // focus, or already be selected, so its pane does not move focus. Once its + // tab shows, hand it focus nothing else took. + const routedTab = + routedThread && !membersOpen + ? tabState.tabs.find( + (tab) => + tab.kind === "thread" && + tab.channelId === routedThread.channelId && + tab.messageId === routedThread.messageId && + (tabPane(tab) === "main" + ? tabState.mainSelected === tab.id + : showingChannelPanel && !companion && selectedTab === tab.id), + )?.id + : undefined; const routedVisit = useRef(undefined); useLayoutEffect(() => { const visit = navigation?.signal ?? currentId; - if (!showingRoutedThread || routedVisit.current === visit) return; + if (!routedTab || routedVisit.current === visit) return; routedVisit.current = visit; if (document.activeElement !== document.body) return; split.ref.current ?.querySelector( - `[role="tab"][data-tab-value="${CSS.escape(selectedTab)}"]`, + `[role="tab"][data-tab-value="${CSS.escape(routedTab)}"]`, ) ?.focus({ preventScroll: true }); - }, [ - showingRoutedThread, - navigation?.signal, - currentId, - selectedTab, - split.ref, - ]); + }, [routedTab, navigation?.signal, currentId, split.ref]); useEffect(() => { // Only an explicit settings click moves focus; restoring a visit does not. if (settingsFocus) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 0e192e9270..ef59f38ae2 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -192,6 +192,77 @@ test.describe("mentioned reply priority", () => { } }); + test("opening a main-pane thread from Activity reads it, not the side-pane thread", async ({ + page, + app, + }) => { + await page.clock.install(); + await holdReadingFocus(page); + await open(page, app); + const rows = app.histories.get("primary/alpha"); + const [x, t] = ["Thread root 1", "Thread root 0"].map((content) => + rows.find((row) => row.content === content), + ); + for (const root of [x, t]) { + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect( + page.getByRole("complementary", { name: "Thread", exact: true }), + ).toBeVisible(); + } + const side = page.locator("[data-panel-workspace]"); + const main = page.locator("[data-main-workspace]"); + const tabs = (host) => + host.locator('[role="tab"][data-tab-value^="thread:"]'); + const unread = (rootId) => + page.evaluate( + (rootId) => + window.fixtureRelay.snapshot().session.unread.snapshot({ + kind: "thread", + channelId: "alpha", + rootId, + }).observedCount, + rootId, + ); + // Move X's tab (the first opened) to the main pane, leaving T in the side. + const [id] = await tabs(side).evaluateAll((all) => + all.map((tab) => tab.dataset.tabValue), + ); + const from = await page + .locator(`[role="tab"][data-tab-value="${id}"]`) + .boundingBox(); + const header = await main.locator("header").first().boundingBox(); + await page.mouse.move(from.x + from.width / 2, from.y + from.height / 2); + await page.mouse.down(); + await page.mouse.move( + header.x + header.width / 2, + header.y + header.height / 2, + { steps: 8 }, + ); + await page.mouse.up(); + await expect(tabs(main)).toHaveCount(1); + await expect(tabs(side)).toHaveCount(1); + await page.clock.runFor(1000); + expect(await unread(x.id)).toBeGreaterThan(0); + expect(await unread(t.id)).toBeGreaterThan(0); + + // Activity reopens X, already selected in the main pane. + await page.locator('button[data-channel-id="alpha"]').hover(); + const item = page + .getByRole("dialog", { name: "Activity in Alpha" }) + .getByRole("button", { name: /Open unread thread from/ }) + .filter({ hasText: "Unread reply 1" }); + await expect(item).toBeVisible(); + await releaseReadingFocus(page); + await item.click(); + await expect(item).toBeHidden(); + await page.clock.runFor(1000); + await expect.poll(() => unread(x.id)).toBe(0); + expect(await unread(t.id)).toBeGreaterThan(0); + }); + test("an Activity reopening leaves focus the person moved meanwhile", async ({ page, app, From f098100a2c4886eb6916ad605f6fcbe63e973934 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 21:07:16 -0400 Subject: [PATCH 7/8] test(browser): link to a confirmed message in the message-link read test The test links to Thread root 1. Until Alpha's history confirms that root, the link opens its thread instead, which now takes focus and reads it. Wait for confirmed history first, so the link is always the timeline jump the test describes. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- tests/browser/thread-unread.spec.mjs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index ef59f38ae2..2ad38562c0 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -970,6 +970,22 @@ test("opening a message link reads the channel but not its already-open thread", ).toBeVisible(); const before = await channel(); expect(await unread()).toBeGreaterThan(0); + // The link is a timeline jump only once Alpha's history confirms the root. + // Before then it opens the root's thread, which takes focus and reads it. + await expect + .poll(() => + page.evaluate((id) => { + const history = window.fixtureRelay + .snapshot() + .session.channels.window("alpha"); + return ( + history.status === "ready" && + history.freshness === "verified" && + history.rows.some((row) => row.id === id) + ); + }, message.id), + ) + .toBe(true); await releaseReadingFocus(page); await page.evaluate((target) => window.fixtureNavigation.open(target), { version: 1, From fb18bf48afcaae38063e02e444e638e43a29e724 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 28 Oct 2026 21:29:04 -0400 Subject: [PATCH 8/8] test(browser): link to an ordinary message in the message-link read test Waiting for confirmed history did not stop CI from opening the link to Thread root 1 as that thread, which then takes focus and reads it. Link to an ordinary top-level message instead, as the test intends. It still fails without the link narrowing. Co-authored-by: John Tennant Co-Authored-By: Claude Opus 5.5 Signed-off-by: Oliver --- tests/browser/thread-unread.spec.mjs | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 2ad38562c0..4d77cb72e9 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -938,8 +938,10 @@ test("opening a message link reads the channel but not its already-open thread", await open(page, app); const history = app.histories.get("primary/alpha"); const root = history.find((row) => row.content === "Thread root 1"); - const message = history.findLast( - (row) => !row.tags.some((tag) => tag[0] === "e"), + // An ordinary message, not the open thread's root: a link to the root can + // open its thread instead, which then takes focus and reads it. + const message = history.findLast((row) => + row.content.startsWith("primary alpha message"), ); const panel = page.getByRole("complementary", { name: "Thread", @@ -970,22 +972,6 @@ test("opening a message link reads the channel but not its already-open thread", ).toBeVisible(); const before = await channel(); expect(await unread()).toBeGreaterThan(0); - // The link is a timeline jump only once Alpha's history confirms the root. - // Before then it opens the root's thread, which takes focus and reads it. - await expect - .poll(() => - page.evaluate((id) => { - const history = window.fixtureRelay - .snapshot() - .session.channels.window("alpha"); - return ( - history.status === "ready" && - history.freshness === "verified" && - history.rows.some((row) => row.id === id) - ); - }, message.id), - ) - .toBe(true); await releaseReadingFocus(page); await page.evaluate((target) => window.fixtureNavigation.open(target), { version: 1,