From f73f65cd7852c7eb10513a0da4b85d47031ca38d Mon Sep 17 00:00:00 2001 From: Matt Cowger Date: Tue, 25 Aug 2026 11:02:18 -0700 Subject: [PATCH] feat(app): close settings with Escape (#2828) * feat(app): close settings with Escape * test(app): cover settings Escape overlay ownership --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com> --- .../e2e/browser/settings-navigation.spec.ts | 35 +++++++++++++++++++ .../app/src/hooks/use-keyboard-shortcuts.ts | 16 ++++++++- .../app/src/keyboard/route-shortcut.test.ts | 21 +++++++++++ packages/app/src/keyboard/route-shortcut.ts | 7 ++++ packages/app/src/lib/overlay-root.ts | 4 +++ 5 files changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/app/e2e/browser/settings-navigation.spec.ts b/packages/app/e2e/browser/settings-navigation.spec.ts index b5ffea7f4f..e13a29535c 100644 --- a/packages/app/e2e/browser/settings-navigation.spec.ts +++ b/packages/app/e2e/browser/settings-navigation.spec.ts @@ -121,6 +121,41 @@ test.describe("Settings sidebar navigation", () => { await clickSettingsBackToWorkspace(page); await expect(page).not.toHaveURL(/\/settings(\/|$)/); }); + + test("pressing Escape closes settings", async ({ page }) => { + await gotoAppShell(page); + await openSettings(page); + await page.keyboard.press("Escape"); + await expect(page).not.toHaveURL(/\/settings(\/|$)/); + }); + + test("Escape lets settings dropdowns and modals close before leaving settings", async ({ + page, + }) => { + await gotoAppShell(page); + await openSettings(page); + + await test.step("a dropdown owns Escape", async () => { + await openSettingsSection(page, "appearance"); + await page.getByLabel(/Theme:/).click(); + await expect(page.getByRole("menuitem", { name: "System", exact: true })).toBeVisible(); + + await page.keyboard.press("Escape"); + + await expect(page.getByRole("menuitem", { name: "System", exact: true })).toHaveCount(0); + await expect(page).toHaveURL(/\/settings(\/|$)/); + }); + + await test.step("a modal owns Escape", async () => { + await openAddHostFlow(page); + await expect(page.getByText("Add connection", { exact: true })).toBeVisible(); + + await page.keyboard.press("Escape"); + + await expect(page.getByText("Add connection", { exact: true })).toHaveCount(0); + await expect(page).toHaveURL(/\/settings(\/|$)/); + }); + }); }); test.describe("Settings — compact master-detail", () => { diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index 04d89cd6bc..057efef369 100644 --- a/packages/app/src/hooks/use-keyboard-shortcuts.ts +++ b/packages/app/src/hooks/use-keyboard-shortcuts.ts @@ -32,6 +32,8 @@ import { isNative } from "@/constants/platform"; import { keyboardShortcutsAvailable } from "@/keyboard/availability"; import { getDesktopHost, isElectronRuntime } from "@/desktop/host"; import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime"; +import { buildOpenProjectRoute } from "@/utils/host-routes"; +import { hasActiveWebOverlay } from "@/lib/overlay-root"; import { type ActiveWorkspaceSelection, navigateToLastWorkspace, @@ -179,7 +181,11 @@ export function useKeyboardShortcuts({ navigateToWorkspace({ serverId: action.serverId, workspaceId: action.workspaceId }); return true; case "navigate-last-workspace": - return navigateToLastWorkspace(); + if (navigateToLastWorkspace()) { + return true; + } + router.replace(buildOpenProjectRoute()); + return true; case "router-replace": router.replace(action.route as Parameters[0]); return true; @@ -330,6 +336,14 @@ export function useKeyboardShortcuts({ } const key = event.key ?? ""; + if ( + key === "Escape" && + pathname.startsWith("/settings") && + !isMobile && + hasActiveWebOverlay() + ) { + return; + } if (key === badgeModifierKey && !event.shiftKey) { setBadgeModifierDown(true); } diff --git a/packages/app/src/keyboard/route-shortcut.test.ts b/packages/app/src/keyboard/route-shortcut.test.ts index 0243f555fd..5eddf3eeff 100644 --- a/packages/app/src/keyboard/route-shortcut.test.ts +++ b/packages/app/src/keyboard/route-shortcut.test.ts @@ -59,6 +59,27 @@ describe("routeKeyboardShortcut — dispatch passthroughs", () => { action: expected, }); }); + + it("closes desktop settings when Escape routes through agent interrupt", () => { + expect( + routeKeyboardShortcut( + { action: "agent.interrupt", payload: null }, + makeCtx({ pathname: "/settings/general" }), + ), + ).toEqual({ kind: "navigate-last-workspace" }); + }); + + it("keeps agent interrupt behavior on compact settings layouts", () => { + expect( + routeKeyboardShortcut( + { action: "agent.interrupt", payload: null }, + makeCtx({ pathname: "/settings/general", isMobile: true }), + ), + ).toEqual({ + kind: "dispatch", + action: { id: "agent.interrupt", scope: "global" }, + }); + }); }); describe("routeKeyboardShortcut — workspace.tab.navigate", () => { diff --git a/packages/app/src/keyboard/route-shortcut.ts b/packages/app/src/keyboard/route-shortcut.ts index 33660994a7..aa7679a7bf 100644 --- a/packages/app/src/keyboard/route-shortcut.ts +++ b/packages/app/src/keyboard/route-shortcut.ts @@ -177,6 +177,13 @@ export function routeKeyboardShortcut( ): ShortcutAction { const passthrough = PASSTHROUGH_DISPATCH[input.action]; if (passthrough) { + if ( + input.action === "agent.interrupt" && + ctx.pathname.startsWith("/settings") && + !ctx.isMobile + ) { + return { kind: "navigate-last-workspace" }; + } return dispatch(passthrough); } diff --git a/packages/app/src/lib/overlay-root.ts b/packages/app/src/lib/overlay-root.ts index 489961c2f0..2c220bafac 100644 --- a/packages/app/src/lib/overlay-root.ts +++ b/packages/app/src/lib/overlay-root.ts @@ -107,6 +107,10 @@ function getTopWebOverlay(): WebOverlayEntry | undefined { }, undefined); } +export function hasActiveWebOverlay(): boolean { + return getTopWebOverlay() !== undefined; +} + function getFocusableElements(scope: HTMLElement): HTMLElement[] { const selector = [ "a[href]",