From ab75e575be77a0b326ada99448a4ef1514f6bbd3 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Tue, 6 Oct 2026 11:25:25 +0200 Subject: [PATCH 1/2] fix(editor): close top-bar menus on a click on the empty bar The top bar is the window's drag region, and the OS takes a press there as a caption click that never reaches the page, so the document mousedown that closes the wordmark menu (and the presets popover) missed it. The bar now drops its drag region while one of its menus is open. Fixes #1009 --- .../ai-edition/v4/EditorShellV4.module.css | 8 ++++++ .../v4/EditorTopBar.stylePresets.test.tsx | 17 ++++++++++++ .../ai-edition/v4/EditorTopBar.test.tsx | 26 +++++++++++++++++++ 3 files changed, 51 insertions(+) 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..f8a5a2e9c 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 = () => {}; @@ -475,3 +476,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); + }); +}); From 458689eb4985cd8b2690de4406bf453d3a5ad2cf Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Tue, 6 Oct 2026 11:30:43 +0200 Subject: [PATCH 2/2] fix: close menus on Escape and hand focus back to their trigger The wordmark menu listened for Escape on its own element, so the key went unheard once focus had left it: a press on the menu's padding or a separator drops focus to . It now listens on the document, as the dialogs do, hands focus back to the trigger and stops the key there. The HUD popovers already closed on Escape, but dropped focus to when it was inside them; it now goes back to the button that opened them. The colour popover (Radix) already closes and refocuses its swatch; a test pins it. Fixes #1015 --- src/components/ai-edition/ColorField.test.tsx | 13 ++++++- .../ai-edition/v4/EditorTopBar.test.tsx | 35 +++++++++++++++++-- src/components/ai-edition/v4/EditorTopBar.tsx | 34 ++++++++++-------- src/components/launch/LaunchWindow.test.tsx | 30 ++++++++++++++++ src/components/launch/LaunchWindow.tsx | 5 ++- 5 files changed, 97 insertions(+), 20 deletions(-) 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/EditorTopBar.test.tsx b/src/components/ai-edition/v4/EditorTopBar.test.tsx index f8a5a2e9c..afcb7d719 100644 --- a/src/components/ai-edition/v4/EditorTopBar.test.tsx +++ b/src/components/ai-edition/v4/EditorTopBar.test.tsx @@ -302,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 () => { 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