diff --git a/src/features/messages/MessageActionBar.test.tsx b/src/features/messages/MessageActionBar.test.tsx index f1a46c429..64d0e4dd2 100644 --- a/src/features/messages/MessageActionBar.test.tsx +++ b/src/features/messages/MessageActionBar.test.tsx @@ -43,12 +43,60 @@ it("opens with keyboard, invokes a sibling action, and returns focus on Escape", ); expect(action).toHaveBeenCalledOnce(); await waitFor(() => expect(screen.queryByRole("menu")).toBeNull()); - await user.click(trigger); + trigger.focus(); + await user.keyboard("{Enter}"); const menu = await screen.findByRole("menu"); await waitFor(() => expect(menu.contains(document.activeElement)).toBe(true)); await user.keyboard("{Escape}"); await waitFor(() => expect(document.activeElement).toBe(trigger)); }); +it.each(["{Escape}", "{ArrowDown}{Escape}"])( + "returns focus after pointer open then keyboard close: %s", + async (keys) => { + const user = userEvent.setup(); + render( "Hello"} />); + const trigger = screen.getByRole("button", { + name: "More message actions", + }); + await user.click(trigger); + const menu = await screen.findByRole("menu"); + await waitFor(() => + expect(menu.contains(document.activeElement)).toBe(true), + ); + await user.keyboard(keys); + await waitFor(() => expect(screen.queryByRole("menu")).toBeNull()); + await waitFor(() => expect(document.activeElement).toBe(trigger)); + await user.keyboard("{Enter}"); + await screen.findByRole("menu"); + }, +); +it("keeps keyboard retry reachable after pointer open and a failed copy", async () => { + const user = userEvent.setup(); + const write = vi + .spyOn(navigator.clipboard, "writeText") + .mockRejectedValueOnce(new Error("denied")) + .mockResolvedValueOnce(); + render( "Hello"} />); + const trigger = screen.getByRole("button", { name: "More message actions" }); + await user.click(trigger); + const menu = await screen.findByRole("menu"); + await waitFor(() => expect(menu.contains(document.activeElement)).toBe(true)); + await user.keyboard("{ArrowDown}"); + expect(document.activeElement).toBe( + screen.getByRole("menuitem", { name: "Copy message" }), + ); + await user.keyboard("{Enter}"); + await screen.findByText("Couldn’t copy. Try again from the message menu."); + await waitFor(() => expect(document.activeElement).toBe(trigger)); + await user.keyboard("{Enter}"); + const retry = await screen.findByRole("menuitem", { name: "Copy message" }); + await waitFor(() => expect(document.activeElement).toBe(retry)); + await user.keyboard("{Enter}"); + await screen.findByText("Message copied"); + expect(write).toHaveBeenCalledTimes(2); + expect(write).toHaveBeenLastCalledWith("Hello"); + await waitFor(() => expect(document.activeElement).toBe(trigger)); +}); it("copies the message, shows failure, and allows retry", async () => { const user = userEvent.setup(); const write = vi @@ -62,6 +110,8 @@ it("copies the message, shows failure, and allows retry", async () => { await screen.findByRole("menuitem", { name: "Copy message" }), ); await screen.findByText("Couldn’t copy. Try again from the message menu."); + await waitFor(() => expect(screen.queryByRole("menu")).toBeNull()); + expect(document.activeElement).not.toBe(trigger); await user.click(trigger); await user.click( await screen.findByRole("menuitem", { name: "Copy message" }), diff --git a/src/features/messages/MessageActionBar.tsx b/src/features/messages/MessageActionBar.tsx index 8982b46ea..84ebc932c 100644 --- a/src/features/messages/MessageActionBar.tsx +++ b/src/features/messages/MessageActionBar.tsx @@ -60,6 +60,7 @@ export function MessageActionBar({ const busy = useRef(false); const afterClose = useRef<(() => void) | undefined>(undefined); const [handingOffFocus, setHandingOffFocus] = useState(false); + const [openedByPointer, setOpenedByPointer] = useState(false); const copy = async (text: () => string, label: string) => { if (busy.current) return; busy.current = true; @@ -117,8 +118,24 @@ export function MessageActionBar({ /> { - if (next) setHandingOffFocus(false); + onOpenChange={(next, details) => { + if (next) { + setHandingOffFocus(false); + // Base UI opens on mousedown, so a real pointer press carries a + // click count; keyboard and assistive presses arrive as a click + // with 0. + setOpenedByPointer( + details.event instanceof MouseEvent && + details.event.detail > 0, + ); + } else if ( + details.event.type.startsWith("key") || + (details.event.type === "click" && + "detail" in details.event && + details.event.detail === 0) + ) { + setOpenedByPointer(false); + } setOpen(next); }} onOpenChangeComplete={(opened) => { @@ -145,7 +162,9 @@ export function MessageActionBar({ data-message-id={messageId} // A boolean preserves Base UI's safeguard when focus already moved. // A callback returning true would force focus back over a newer action. - finalFocus={!handingOffFocus} + // Pointer-only interactions hand nothing back; switching to the + // keyboard restores the trigger for continued navigation. + finalFocus={!handingOffFocus && !openedByPointer} > { diff --git a/src/features/messages/Messages.module.css b/src/features/messages/Messages.module.css index 985ae2873..efedae34f 100644 --- a/src/features/messages/Messages.module.css +++ b/src/features/messages/Messages.module.css @@ -1745,8 +1745,9 @@ html[data-keyboard-navigation] .imageReviewToolbar :focus-visible { opacity: 0; } .message:hover .continuationTime, - .message:focus-within .continuationTime, - .message[data-actions-revealed] .continuationTime { + .message:has(:focus-visible) .continuationTime, + .message[data-actions-revealed] .continuationTime, + html[data-keyboard-navigation] .message:focus-within .continuationTime { opacity: 1; } } diff --git a/src/features/messages/useFloatingActionBar.test.tsx b/src/features/messages/useFloatingActionBar.test.tsx index 1fd7b7107..b4d90dfd7 100644 --- a/src/features/messages/useFloatingActionBar.test.tsx +++ b/src/features/messages/useFloatingActionBar.test.tsx @@ -48,6 +48,8 @@ beforeEach(() => { }); afterEach(() => { cleanup(); + delete document.documentElement.dataset.keyboardNavigation; + vi.restoreAllMocks(); Reflect.deleteProperty(HTMLElement.prototype, "showPopover"); Reflect.deleteProperty(HTMLElement.prototype, "hidePopover"); vi.unstubAllGlobals(); @@ -116,6 +118,7 @@ it("retains reveal across row/bar hover, focus, menu and expanded picker transit expect(bar).toHaveAttribute("data-shown"); fireEvent.pointerLeave(row); expect(bar).not.toHaveAttribute("data-shown"); + document.documentElement.dataset.keyboardNavigation = ""; act(() => screen.getByRole("button", { name: "Avatar" }).focus()); expect(bar).toHaveAttribute("data-shown"); act(() => @@ -144,6 +147,43 @@ it("retains reveal across row/bar hover, focus, menu and expanded picker transit expect(screen.getByTestId("bar")).toBe(bar); }); +it("does not retain pointer focus on leave, popup changes or control removal", async () => { + const { rerender } = render(); + const row = screen.getByTestId("row"); + const bar = screen.getByTestId("bar"); + // jsdom treats all focus as visible. Supply only the missing browser modality; + // focus, events, React effects and mutation delivery remain real. + const matches = row.matches.bind(row); + vi.spyOn(row, "matches").mockImplementation((selector) => + selector === ":has(:focus-visible)" ? false : matches(selector), + ); + const avatar = screen.getByRole("button", { name: "Avatar" }); + fireEvent.pointerEnter(row); + act(() => avatar.focus()); + fireEvent.pointerLeave(row); + expect(avatar).toHaveFocus(); + expect(bar).not.toHaveAttribute("data-shown"); + rerender(); + expect(bar).toHaveAttribute("data-shown"); + rerender(); + expect(bar).not.toHaveAttribute("data-shown"); + rerender(); + await act(() => Promise.resolve()); + expect(bar).not.toHaveAttribute("data-shown"); + + // The fallback reveals keyboard focus even without native focus-visible. + act(() => screen.getByRole("button", { name: "Outside" }).focus()); + document.documentElement.dataset.keyboardNavigation = ""; + act(() => avatar.focus()); + expect(bar).toHaveAttribute("data-shown"); + // Re-read modality on leave even if a pointer press did not move focus. + fireEvent.pointerEnter(row); + delete document.documentElement.dataset.keyboardNavigation; + fireEvent.pointerLeave(row); + expect(avatar).toHaveFocus(); + expect(bar).not.toHaveAttribute("data-shown"); +}); + it("leaves coarse-pointer controls static and cleans up floating mode on media changes and unmount", () => { mediaMatches = false; const { unmount } = render(); diff --git a/src/features/messages/useFloatingActionBar.ts b/src/features/messages/useFloatingActionBar.ts index 20141dec6..91235746b 100644 --- a/src/features/messages/useFloatingActionBar.ts +++ b/src/features/messages/useFloatingActionBar.ts @@ -34,8 +34,13 @@ export function useFloatingActionBar( if (!floating || !row || !bar) return; // Top-layer descendants don't contribute to :hover or :focus-within. let hovered = row.matches(":hover") || bar.matches(":hover"); - let focused = row.contains(document.activeElement); - const update = () => { + // Read modality with focus on every update: pointer focus must not pin the + // bar when hover leaves, including after keyboard-to-pointer transitions. + const keyboardFocus = () => + row.matches(":has(:focus-visible)") || + (document.documentElement.hasAttribute("data-keyboard-navigation") && + row.contains(document.activeElement)); + const update = (focused = keyboardFocus()) => { const next = hovered || focused || @@ -52,15 +57,13 @@ export function useFloatingActionBar( hovered = false; update(); }; - const focusIn = () => { - focused = true; - update(); - }; + const focusIn = () => update(); const focusOut = (event: FocusEvent) => { - focused = + update( event.relatedTarget instanceof Node && - row.contains(event.relatedTarget); - update(); + row.contains(event.relatedTarget) && + keyboardFocus(), + ); }; row.addEventListener("pointerenter", enter); row.addEventListener("pointerleave", leave); @@ -68,7 +71,6 @@ export function useFloatingActionBar( row.addEventListener("focusout", focusOut); const observer = new MutationObserver(() => { // Recheck focus if a control is removed without dispatching focusout. - focused = row.contains(document.activeElement); update(); }); observer.observe(bar, { diff --git a/tests/browser/message-actions-floating.spec.mjs b/tests/browser/message-actions-floating.spec.mjs index c4d85db9d..863e102ad 100644 --- a/tests/browser/message-actions-floating.spec.mjs +++ b/tests/browser/message-actions-floating.spec.mjs @@ -353,6 +353,12 @@ test("menus and pickers retain actions, but modal dialogs cover them", async ({ .toBe(false); await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); await expect(trigger).toBeFocused(); + // Pointer dismissal restores focus without pinning an unhovered toolbar. + await expect(actions).toHaveCSS("opacity", "0"); + await trigger.press("Enter"); + await expect(menu).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(trigger).toBeFocused(); await shown(actions); const own = app.append("primary", "alpha", "Delete confirmation check"); const ownRow = page.locator( diff --git a/tests/browser/message-actions.spec.mjs b/tests/browser/message-actions.spec.mjs index 8a88cca5b..7ea5adc81 100644 --- a/tests/browser/message-actions.spec.mjs +++ b/tests/browser/message-actions.spec.mjs @@ -58,6 +58,7 @@ test("message actions reveal, copy, restore focus and reply across responsive la expect(await page.evaluate(() => window.copiedMessages)).toEqual([ event.content, ]); + await row.hover(); await row.getByRole("button", { name: "Copy link", exact: true }).click(); await expect( page @@ -73,6 +74,62 @@ test("message actions reveal, copy, restore focus and reply across responsive la }); const root = panel.locator(`[data-message-id="${event.id}"]`); await expect(root).toBeVisible(); + // A pointer click leaves focus on the control; the bar still follows hover. + const copyLink = row.getByRole("button", { name: "Copy link", exact: true }); + await row.hover(); + await copyLink.click(); + await root.hover(); + await expect(actions).toHaveCSS("opacity", "0"); + // Tab only exercises the keyboard path if focus is still on Copy link, so + // it lands on the row's next control. Copy link is briefly disabled while + // copying, which would otherwise let focus fall back to the body. + await expect(copyLink).toBeFocused(); + // Keyboard focus still reveals it while the mouse is elsewhere. + await page.keyboard.press("Tab"); + await expect(actions).toHaveCSS("opacity", "1"); + // Assistive presses arrive without navigation keydowns; focus alone reveals + // the bar. Clicking plain text first clears the keyboard-modality flag. + const menu = page.getByRole("menu"); + await row.getByText(event.content, { exact: true }).click(); + await page.mouse.move(0, 0); + await trigger.focus(); + // Script focus after a pointer click stays quiet. + await expect(actions).toHaveCSS("opacity", "0"); + await trigger.press("Enter"); + await expect( + page.getByRole("menuitem", { name: "Copy message", exact: true }), + ).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(trigger).toBeFocused(); + await expect(actions).toHaveCSS("opacity", "1"); + // Keyboard dismissal restores focus even when the menu was pointer-opened. + // Waiting for deferred initial focus keeps Escape off the trigger. + await row.hover(); + await trigger.click(); + await expect(menu).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(menu).toHaveCount(0); + await expect(trigger).toBeFocused(); + await page.mouse.move(0, 0); + await expect(actions).toHaveCSS("opacity", "1"); + await page.keyboard.press("Enter"); + await expect(menu).toBeVisible(); + await expect( + page.getByRole("menuitem", { name: "Copy message", exact: true }), + ).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(trigger).toBeFocused(); + // Pointer-only selection still leaves the bar free to follow hover. + await row.hover(); + await trigger.click(); + await expect(menu).toBeFocused(); + await page + .getByRole("menuitem", { name: "Copy message", exact: true }) + .click(); + await expect(menu).toHaveCount(0); + await expect(trigger).not.toBeFocused(); + await page.mouse.move(0, 0); + await expect(actions).toHaveCSS("opacity", "0"); await root.hover(); await root .getByRole("button", { name: "React with 👍", exact: true }) diff --git a/tests/browser/message-management.spec.mjs b/tests/browser/message-management.spec.mjs index e1433f94c..0311c7a3e 100644 --- a/tests/browser/message-management.spec.mjs +++ b/tests/browser/message-management.spec.mjs @@ -45,12 +45,23 @@ test("manage a channel message, peer unread state, and its thread", async ({ ); for (const width of [1440, 900, 390]) { await page.setViewportSize({ width, height: 850 }); - const bounds = await editor.boundingBox(); - const chip = await editor.locator(".inline-chip").boundingBox(); - expect(bounds.x).toBeGreaterThanOrEqual(0); - expect(bounds.x + bounds.width).toBeLessThanOrEqual(width); - expect(chip.x).toBeGreaterThanOrEqual(bounds.x); - expect(chip.x + chip.width).toBeLessThanOrEqual(bounds.x + bounds.width); + await expect + .poll(() => + editor.evaluate((element) => { + // Read parent and child in the same layout, after responsive reflow. + const bounds = element.getBoundingClientRect(); + const chip = element + .querySelector('.inline-chip[data-kind="person"]') + .getBoundingClientRect(); + return ( + bounds.x >= 0 && + bounds.right <= window.innerWidth && + chip.x >= bounds.x && + chip.right <= bounds.right + ); + }), + ) + .toBe(true); } await page.setViewportSize({ width: 1440, height: 950 }); await editor.press("End"); @@ -140,7 +151,19 @@ test("manage a channel message, peer unread state, and its thread", async ({ await confirmation.getByRole("button", { name: "Cancel" }).click(); await expect(row).toBeVisible(); await expect(trigger).toBeFocused(); + // Pointer cancellation retains focus, but need not reveal an unhovered + // toolbar. Continue from that focus with the keyboard, not a hidden click. + await page.mouse.move(0, 0); + const actions = row.getByRole("group", { name: "Message actions" }); + await page.keyboard.press("Enter"); + await expect( + page.getByRole("menuitem", { name: "Delete message", exact: true }), + ).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(trigger).toBeFocused(); + await expect(actions).toHaveCSS("opacity", "1"); } + await row.hover(); await trigger.click(); await page .getByRole("menuitem", { name: "Delete message", exact: true })