Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion src/features/messages/MessageActionBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<MessageActionBar copyText={() => "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(<MessageActionBar copyText={() => "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
Expand All @@ -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" }),
Expand Down
25 changes: 22 additions & 3 deletions src/features/messages/MessageActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -117,8 +118,24 @@ export function MessageActionBar({
/>
<MenuRoot
open={open}
onOpenChange={(next) => {
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) => {
Expand All @@ -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}
>
<AfterMenuClose.Provider
value={(action) => {
Expand Down
5 changes: 3 additions & 2 deletions src/features/messages/Messages.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
40 changes: 40 additions & 0 deletions src/features/messages/useFloatingActionBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(() =>
Expand Down Expand Up @@ -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(<Harness />);
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(<Harness open />);
expect(bar).toHaveAttribute("data-shown");
rerender(<Harness />);
expect(bar).not.toHaveAttribute("data-shown");
rerender(<Harness hasPicker={false} />);
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(<Harness />);
Expand Down
22 changes: 12 additions & 10 deletions src/features/messages/useFloatingActionBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -52,23 +57,20 @@ 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);
row.addEventListener("focusin", focusIn);
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, {
Expand Down
6 changes: 6 additions & 0 deletions tests/browser/message-actions-floating.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
57 changes: 57 additions & 0 deletions tests/browser/message-actions.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 })
Expand Down
35 changes: 29 additions & 6 deletions tests/browser/message-management.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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 })
Expand Down
Loading