From ab1afdb10dcb41295ca4f1cb32db14dff8e781ab Mon Sep 17 00:00:00 2001 From: Aatman Jain Date: Sun, 25 Oct 2026 20:01:52 +0000 Subject: [PATCH] fix(web): Esc closes the theme editor, not Settings Esc while inspecting cancelled the inspector and also closed Settings, and Esc inside the editor closed Settings while the editor stayed open. Stop the inspector's Esc from reaching other listeners, and close the editor on an Esc pressed inside it. Popups opened from the editor are portaled outside it and keep their own Esc. Co-Authored-By: Claude Opus 5.5 --- .../ThemeEditorPanel.escape.browser.test.tsx | 80 +++++++++++++++++++ .../components/settings/ThemeEditorPanel.tsx | 9 +++ 2 files changed, 89 insertions(+) create mode 100644 apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx diff --git a/apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx b/apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx new file mode 100644 index 0000000000000..5fcd2c237dda1 --- /dev/null +++ b/apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx @@ -0,0 +1,80 @@ +import { Suspense } from "react"; +import { describe, expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; + +import { render } from "../../../test/render"; +import { Dialog, DialogPopup, DialogTitle } from "../ui/dialog"; +import { ThemeEditorPanel } from "./ThemeEditorPanel"; + +// The modal Settings dialog marks everything outside it inert in this harness, so the +// test drives the editor through the DOM instead of pointer actions. +async function editorElement(selector: string): Promise { + await expect + .poll(() => document.querySelector(`[data-theme-editor-panel] ${selector}`)) + .toBeTruthy(); + return document.querySelector(`[data-theme-editor-panel] ${selector}`)!; +} + +// The editor floats beside the Settings dialog, whose Esc handler sits on the document. +function SettingsWithEditor(props: { + onSettingsOpenChange: (open: boolean) => void; + onEditorOpenChange: (open: boolean) => void; +}) { + return ( + <> + + + Settings + + + + true} + open + restoreTheme={() => undefined} + /> + + + ); +} + +describe("Esc in the theme editor", () => { + it("closes the editor, not Settings", async () => { + const onSettingsOpenChange = vi.fn(); + const onEditorOpenChange = vi.fn(); + await using _screen = await render( + , + ); + + (await editorElement("input")).focus(); + await userEvent.keyboard("{Escape}"); + + expect(onEditorOpenChange).toHaveBeenCalledWith(false); + expect(onSettingsOpenChange).not.toHaveBeenCalled(); + }); + + it("cancels inspecting only", async () => { + const onSettingsOpenChange = vi.fn(); + const onEditorOpenChange = vi.fn(); + await using _screen = await render( + , + ); + + (await editorElement('[aria-label="Inspect app colors"]')).click(); + await editorElement('[aria-label="Cancel inspecting app colors"]'); + await userEvent.keyboard("{Escape}"); + + await editorElement('[aria-label="Inspect app colors"]'); + expect(onEditorOpenChange).not.toHaveBeenCalled(); + expect(onSettingsOpenChange).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/components/settings/ThemeEditorPanel.tsx b/apps/web/src/components/settings/ThemeEditorPanel.tsx index 329493fe3f97b..3498ae6361aef 100644 --- a/apps/web/src/components/settings/ThemeEditorPanel.tsx +++ b/apps/web/src/components/settings/ThemeEditorPanel.tsx @@ -667,7 +667,9 @@ export function ThemeEditorPanel({ }; const cancelInspection = (event: KeyboardEvent) => { if (event.key !== "Escape") return; + // Esc leaves inspection only; Settings must not see it and close too. event.preventDefault(); + event.stopImmediatePropagation(); clearHover(); clearInspectorSelection(); }; @@ -1077,6 +1079,13 @@ export function ThemeEditorPanel({ isMinimized && "max-h-none", )} data-theme-editor-panel + onKeyDown={(event) => { + // Popups opened from the editor are portaled outside it and handle their own Esc. + if (event.key !== "Escape" || event.defaultPrevented) return; + if (!event.currentTarget.contains(event.target as Node)) return; + event.stopPropagation(); + onOpenChange(false); + }} ref={panelRef} role="dialog" style={{