diff --git a/src/components/ai-edition/ColorField.test.tsx b/src/components/ai-edition/ColorField.test.tsx index 636167367..230852f24 100644 --- a/src/components/ai-edition/ColorField.test.tsx +++ b/src/components/ai-edition/ColorField.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom import "@testing-library/jest-dom"; -import { fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { beforeAll, describe, expect, it, vi } from "vitest"; // Le traducteur renvoie la clé : une assertion lit mieux contre une clé que contre une phrase qui @@ -99,4 +99,15 @@ describe("ColorField", () => { fireEvent.keyDown(document.body, { key: "Escape" }); expect(onCommit).toHaveBeenCalledTimes(1); }); + + it("closes on Escape and hands focus back to the swatch (#1015)", async () => { + const { trigger } = renderField(); + fireEvent.click(trigger); + // Radix moves focus into the picker as it opens; the key is pressed from there. + const inside = document.activeElement as HTMLElement; + expect(screen.getByLabelText("annotation.colorPalette").parentElement).toContainElement(inside); + fireEvent.keyDown(inside, { key: "Escape" }); + expect(screen.queryByLabelText("annotation.colorPalette")).not.toBeInTheDocument(); + await waitFor(() => expect(trigger).toHaveFocus()); + }); }); diff --git a/src/components/ai-edition/v4/EditorShellV4.module.css b/src/components/ai-edition/v4/EditorShellV4.module.css index ee3d11fab..8188f58d9 100644 --- a/src/components/ai-edition/v4/EditorShellV4.module.css +++ b/src/components/ai-edition/v4/EditorShellV4.module.css @@ -65,6 +65,14 @@ -webkit-app-region: no-drag; app-region: no-drag; } +/* While a menu hanging off the bar is open (the wordmark's, the presets'), the bar stops being a + titlebar. The OS takes a press on a drag region as a caption click and the page never sees it, + so the outside-click that closes the menu missed every click on the empty bar (#1009). That + click now closes the menu; the bar drags again from the next one. */ +.topbar:has([aria-expanded="true"]) { + -webkit-app-region: no-drag; + app-region: no-drag; +} .sep { width: 1px; height: 18px; diff --git a/src/components/ai-edition/v4/EditorTopBar.stylePresets.test.tsx b/src/components/ai-edition/v4/EditorTopBar.stylePresets.test.tsx index 896ef04ae..320438d1d 100644 --- a/src/components/ai-edition/v4/EditorTopBar.stylePresets.test.tsx +++ b/src/components/ai-edition/v4/EditorTopBar.stylePresets.test.tsx @@ -3,6 +3,8 @@ // catalog: the bridge, the settings hook, the platform and the toaster are the only fakes. import "@testing-library/jest-dom"; +import { readFileSync } from "node:fs"; +import path from "node:path"; import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "vitest"; import { TooltipProvider } from "@/components/ui/tooltip"; @@ -469,3 +471,18 @@ describe("Presets menu in the editor top bar", () => { expect(state.set).not.toHaveBeenCalled(); }); }); + +// The other menu hanging off the bar: while it is open the bar stops being a window-drag region +// too, or a click on the empty bar never reaches the page to close it (#1009). +describe("Presets menu and the bar's drag region", () => { + const css = readFileSync(path.join(__dirname, "EditorShellV4.module.css"), "utf8"); + const [, condition = ""] = css.match(/\n\.topbar(:has\([^{]*\))\s*\{/) ?? []; + + it("puts the bar under the no-drag rule while it is open", async () => { + renderPane(); + const bar = screen.getByRole("button", { name: "Presets" }).closest("header") as HTMLElement; + expect(bar.matches(condition)).toBe(false); + await openMenu(); + expect(bar.matches(condition)).toBe(true); + }); +}); diff --git a/src/components/ai-edition/v4/EditorTopBar.test.tsx b/src/components/ai-edition/v4/EditorTopBar.test.tsx index f357f1121..afcb7d719 100644 --- a/src/components/ai-edition/v4/EditorTopBar.test.tsx +++ b/src/components/ai-edition/v4/EditorTopBar.test.tsx @@ -35,6 +35,7 @@ vi.mock("@/hooks/useTheme", () => ({ useTheme: () => ({ theme: "dark", toggle: toggleTheme }), })); +import styles from "./EditorShellV4.module.css"; import { EditorTopBar } from "./EditorTopBar"; const noop = () => {}; @@ -301,11 +302,40 @@ describe("AppMenu", () => { ); }); - it("closes on Escape", () => { + it("closes on Escape and hands focus back to the trigger", () => { renderTopBar("Demo Project"); - fireEvent.click(screen.getByRole("button", { name: /OpenScreen/ })); - fireEvent.keyDown(screen.getByRole("menu"), { key: "Escape" }); + const trigger = screen.getByRole("button", { name: /OpenScreen/ }); + fireEvent.click(trigger); + // Pressed on the first row, where the menu put focus. + expect(document.activeElement).toHaveAttribute("role", "menuitem"); + fireEvent.keyDown(document.activeElement as HTMLElement, { key: "Escape" }); + expect(screen.queryByRole("menu")).not.toBeInTheDocument(); + expect(trigger).toHaveFocus(); + }); + + // A press on the menu's padding or a separator drops focus to , outside the menu, where + // a listener on the menu itself never heard the key (#1015). + it("closes on Escape when focus is no longer in the menu", () => { + renderTopBar("Demo Project"); + const trigger = screen.getByRole("button", { name: /OpenScreen/ }); + fireEvent.click(trigger); + act(() => (document.activeElement as HTMLElement).blur()); + fireEvent.keyDown(document.body, { key: "Escape" }); expect(screen.queryByRole("menu")).not.toBeInTheDocument(); + expect(trigger).toHaveFocus(); + }); + + it("keeps the Escape that closed it from the window's shortcut handlers", () => { + const onWindowKeyDown = vi.fn(); + window.addEventListener("keydown", onWindowKeyDown); + try { + renderTopBar("Demo Project"); + fireEvent.click(screen.getByRole("button", { name: /OpenScreen/ })); + fireEvent.keyDown(document.activeElement as HTMLElement, { key: "Escape" }); + expect(onWindowKeyDown).not.toHaveBeenCalled(); + } finally { + window.removeEventListener("keydown", onWindowKeyDown); + } }); it("hides Check for Updates when the install channel owns updates", async () => { @@ -475,3 +505,28 @@ describe("AppMenu sizing (issue #969)", () => { expect(version).toMatch(/flex-shrink:\s*0/); }); }); + +// jsdom has no window manager, so it cannot play the caption click that swallowed the press. What +// it can pin is the mechanism: the rule that drops the bar's drag region exists, and it matches the +// bar exactly while a menu hanging off it is open (#1009). +describe("top bar drag region while a menu is open (issue #1009)", () => { + const css = readFileSync(path.join(__dirname, "EditorShellV4.module.css"), "utf8"); + const [, condition = "", body = ""] = css.match(/\n\.topbar(:has\([^{]*\))\s*\{([^}]*)\}/) ?? []; + + it("declares the bar no-drag under that condition", () => { + expect(condition).not.toBe(""); + expect(body).toMatch(/-webkit-app-region:\s*no-drag/); + expect(body).toMatch(/(^|\s)app-region:\s*no-drag/); + }); + + it("puts the bar under it only while the wordmark menu is open", () => { + renderTopBar("Demo Project"); + const bar = document.querySelector(`header.${styles.topbar}`) as HTMLElement; + expect(bar.matches(condition)).toBe(false); + fireEvent.click(screen.getByRole("button", { name: /OpenScreen/ })); + expect(bar.matches(condition)).toBe(true); + fireEvent.mouseDown(bar); + expect(screen.queryByRole("menu")).not.toBeInTheDocument(); + expect(bar.matches(condition)).toBe(false); + }); +}); diff --git a/src/components/ai-edition/v4/EditorTopBar.tsx b/src/components/ai-edition/v4/EditorTopBar.tsx index 5ee571417..ed391cb55 100644 --- a/src/components/ai-edition/v4/EditorTopBar.tsx +++ b/src/components/ai-edition/v4/EditorTopBar.tsx @@ -335,8 +335,23 @@ function AppMenu({ actions }: { actions: TopBarActions }) { const onDocMouseDown = (e: MouseEvent) => { if (ref.current && !ref.current.contains(e.target as Node)) setOpen(false); }; + // On the document, as the dialogs do (Modals.tsx), not on the menu: a press on the menu's + // padding or a separator drops focus to , where a listener on the menu never hears + // the key (#1015). Escape hands focus back to the trigger, and goes no further, so nothing + // else bound to it acts on the same press. + const onDocKeyDown = (e: KeyboardEvent) => { + if (e.key !== "Escape") return; + e.preventDefault(); + e.stopPropagation(); + setOpen(false); + triggerRef.current?.focus(); + }; document.addEventListener("mousedown", onDocMouseDown); - return () => document.removeEventListener("mousedown", onDocMouseDown); + document.addEventListener("keydown", onDocKeyDown); + return () => { + document.removeEventListener("mousedown", onDocMouseDown); + document.removeEventListener("keydown", onDocKeyDown); + }; }, [open]); // Focus the first item as the menu appears, so it is operable from the keyboard without a @@ -346,20 +361,7 @@ function AppMenu({ actions }: { actions: TopBarActions }) { menuRef.current?.querySelector('[role="menuitem"]')?.focus(); }, [open]); - const close = (restoreFocus: boolean) => { - setOpen(false); - // Escape and Tab-out hand focus back to the trigger; a click does not, because the - // pointer user did not come from there and a focus ring appearing under the cursor - // reads as a bug. - if (restoreFocus) triggerRef.current?.focus(); - }; - const onMenuKeyDown = (e: ReactKeyboardEvent) => { - if (e.key === "Escape") { - e.preventDefault(); - close(true); - return; - } if (e.key !== "ArrowDown" && e.key !== "ArrowUp") return; e.preventDefault(); const items = Array.from( @@ -375,7 +377,9 @@ function AppMenu({ actions }: { actions: TopBarActions }) { }; const run = (action: () => void) => () => { - close(false); + // Unlike Escape, a click does not hand focus back to the trigger: the pointer user did not + // come from there, and a focus ring appearing under the cursor reads as a bug. + setOpen(false); action(); }; diff --git a/src/components/launch/LaunchWindow.test.tsx b/src/components/launch/LaunchWindow.test.tsx index ee37e40f7..eea0580eb 100644 --- a/src/components/launch/LaunchWindow.test.tsx +++ b/src/components/launch/LaunchWindow.test.tsx @@ -1510,6 +1510,22 @@ describe("LaunchWindow popover dismissal", () => { expect(i18nState.value.setLocale).not.toHaveBeenCalled(); }); + it("hands focus back to the language button when Escape closes its menu", async () => { + renderLaunchWindow(); + const menu = await openLanguageMenu(); + // Tabbed into the list: unmounting it must not leave focus on . + const item = within(menu).getAllByRole("menuitemradio")[0]; + act(() => item.focus()); + + fireEvent.keyDown(item, { key: "Escape" }); + + await waitFor(() => { + expect(screen.queryByTestId("hud-language-menu")).not.toBeInTheDocument(); + }); + expect(screen.getByRole("button", { name: "Language: English" })).toHaveFocus(); + expect(i18nState.value.setLocale).not.toHaveBeenCalled(); + }); + it("closes the device-settings panel on Escape", async () => { renderLaunchWindow(); await openDeviceSettings(); @@ -1521,6 +1537,20 @@ describe("LaunchWindow popover dismissal", () => { }); }); + it("hands focus back to the gear when Escape closes the device-settings panel", async () => { + renderLaunchWindow(); + const panel = await openDeviceSettings(); + const close = within(panel).getByRole("button", { name: "Close" }); + act(() => close.focus()); + + fireEvent.keyDown(close, { key: "Escape" }); + + await waitFor(() => { + expect(screen.queryByTestId("hud-device-settings")).not.toBeInTheDocument(); + }); + expect(screen.getByTestId("launch-device-settings-button")).toHaveFocus(); + }); + it("closes the device-settings panel on a pointerdown outside the trigger and the panel", async () => { renderLaunchWindow(); await openDeviceSettings(); diff --git a/src/components/launch/LaunchWindow.tsx b/src/components/launch/LaunchWindow.tsx index ca9dd076a..715a941b1 100644 --- a/src/components/launch/LaunchWindow.tsx +++ b/src/components/launch/LaunchWindow.tsx @@ -331,6 +331,9 @@ export function LaunchWindow() { const handleEscape = (event: KeyboardEvent) => { if (event.key === "Escape") { + // Back to the control that opened it, as the editor's menus do: with focus inside + // the popover, unmounting it would drop focus to . + (isLanguageMenuOpen ? languageTriggerRef : settingsTriggerRef).current?.focus(); closePopovers(); } }; @@ -353,7 +356,7 @@ export function LaunchWindow() { window.removeEventListener("keydown", handleEscape); window.removeEventListener("blur", closePopovers); }; - }, [closePopovers, isPopoverOpen]); + }, [closePopovers, isPopoverOpen, isLanguageMenuOpen]); // --------------------------------------------------------------------------- // Overlay window sizing