diff --git a/src/bundled/channels/ChannelActivityPopover.tsx b/src/bundled/channels/ChannelActivityPopover.tsx index 687c68844a..c8272fae0a 100644 --- a/src/bundled/channels/ChannelActivityPopover.tsx +++ b/src/bundled/channels/ChannelActivityPopover.tsx @@ -201,6 +201,10 @@ export function ChannelActivityPopover({ NonNullable["actionsRef"]>["current"] >(null); const pressedOpen = useRef(false); + // An opened thread takes focus itself. Base UI returns focus to the trigger + // in a microtask after closing, which can land after, and steal it from, the + // thread. + const handoff = useRef(false); // Hover opens a transient preview. Selecting its row may still take focus. const [preview, setPreview] = useState(false); const [now, setNow] = useState(() => Date.now()); @@ -275,7 +279,10 @@ export function ChannelActivityPopover({ pressedOpen.current = next && details.reason === "trigger-press"; setPreview(next && details.reason === "trigger-hover"); setOpen(next); - if (next) setNow(Date.now()); + if (next) { + handoff.current = false; + setNow(Date.now()); + } if (next && items.length) void session.profiles .ensure( @@ -310,6 +317,7 @@ export function ChannelActivityPopover({ size="wide" padding="list" aria-label={`Activity in ${channelName}`} + finalFocus={() => !handoff.current} > {open && (
@@ -366,6 +374,7 @@ export function ChannelActivityPopover({ session={session} agents={activeAgents} onOpen={(selected) => { + handoff.current = true; close(); onOpenThread(selected); }} diff --git a/src/features/messages/ThreadPanel.tsx b/src/features/messages/ThreadPanel.tsx index e41d228c4d..b20cf1731e 100644 --- a/src/features/messages/ThreadPanel.tsx +++ b/src/features/messages/ThreadPanel.tsx @@ -752,6 +752,34 @@ function ThreadMessages({ const rootTarget = navigation?.target.kind === "conversation" && navigation.target.threadRootId === messageId; + // Opening a thread that is already shown selects nothing new, so no tab + // hands focus to it. Without focus inside, reading never starts. A first + // opening (not yet positioned) is focused by its tab and must be left alone. + // The signal identifies one request; the navigation object is re-bound per + // relay snapshot, so a re-render of the same request must not refocus. + const handledRequest = useRef(navigation?.signal); + // biome-ignore lint/correctness/useExhaustiveDependencies: only a new request moves focus; the others merely qualify it. + useLayoutEffect(() => { + if (handledRequest.current === navigation?.signal) return; + handledRequest.current = navigation?.signal; + const element = scroller.current; + if ( + !navigation || + navigation.signal.aborted || + navigation.focus === "preserve" || + !rootTarget || + !active || + !positioned.current || + !element + ) + return; + // Only focus the opener left behind (its control unmounted) moves in. + // Focus the person has since placed anywhere, even a moment after a first + // opening whose request arrived late, is not stolen. + const focused = document.activeElement; + if (!focused || focused === document.body) + element.focus({ preventScroll: true }); + }, [navigation?.signal]); // Ordinary opens follow the latest reply after bounded history finishes. // Keep that first positioning invisible; exact-message navigation reveals itself. const positioning = diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index c988a82a36..d79d1d7551 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -47,6 +47,161 @@ test.describe("mentioned reply priority", () => { new Set(["Broadcast descendant", "Unread reply 1"]), ); }); + + test("opening an already-open thread from Activity reads it", async ({ + page, + app, + }) => { + // Reading needs a 300ms dwell (use-reading.ts); hold the clock so the + // thread cannot earn it while it opens. + 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 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.clock.pauseAt((await page.evaluate(() => Date.now())) + 1000); + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect(panel).toBeVisible(); + // The reader is looking elsewhere: the open thread stays unread. + await page.evaluate(() => document.activeElement?.blur()); + await page.clock.resume(); + expect((await unread()).observedCount).toBeGreaterThan(0); + await 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); + const previousAttempt = await page.evaluate( + () => window.fixtureNavigation.snapshot().attempt.id, + ); + // Activity opens the thread that is already selected. + await item.click(); + // The thread is already on screen, so its visibility proves nothing. Wait + // for the click's own navigation, so a failure below means the thread was + // opened and still not read, never that the click missed. + await expect + .poll(() => + page.evaluate((previous) => { + const { status, attempt } = window.fixtureNavigation.snapshot(); + return { status, freshAttempt: attempt.id !== previous }; + }, previousAttempt), + ) + .toEqual({ status: "opened", freshAttempt: true }); + try { + await expect.poll(async () => (await unread()).observedCount).toBe(0); + } catch (error) { + // Reading needs focus inside the thread; say where it actually was. + const focus = await page.evaluate(() => ({ + active: + document.activeElement?.getAttribute("aria-label") ?? + document.activeElement?.tagName, + inThread: !!document.activeElement?.closest("[data-reading-surface]"), + windowFocused: document.hasFocus(), + })); + 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, + }) => { + // As above, hold the clock so the thread cannot be read while it opens. + 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 composer = page.getByRole("textbox", { + name: "Message #Alpha", + exact: true, + }); + await page.clock.pauseAt((await page.evaluate(() => Date.now())) + 1000); + await page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }) + .click(); + await expect(panel).toBeVisible(); + await page.evaluate(() => document.activeElement?.blur()); + await page.clock.resume(); + 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); + const previousAttempt = await page.evaluate( + () => window.fixtureNavigation.snapshot().attempt.id, + ); + // Hold the reopening's reads so the person can move focus before the + // thread receives its request. + let held = false; + let release; + let sawRead; + const gate = new Promise((resolve) => { + release = resolve; + }); + const read = new Promise((resolve) => { + sawRead = resolve; + }); + await page.route("**/api/relay/**/query", async (route) => { + if (held) { + sawRead(); + await gate; + } + await route.continue().catch(() => {}); + }); + held = true; + try { + await item.click(); + await read; + await composer.click(); + await page.keyboard.type("Still typing"); + } finally { + held = false; + release(); + } + await expect + .poll(() => + page.evaluate((previous) => { + const { status, attempt } = window.fixtureNavigation.snapshot(); + return { status, freshAttempt: attempt.id !== previous }; + }, previousAttempt), + ) + .toEqual({ status: "opened", freshAttempt: true }); + await expect(composer).toBeFocused(); + await expect(composer).toHaveText("Still typing"); + }); }); test("thread buttons show observed unread independently, clear only after reading, and expose hover/focus affordance", async ({