diff --git a/apps/web/src/components/settings/KeybindingsSettings.browser.test.tsx b/apps/web/src/components/settings/KeybindingsSettings.browser.test.tsx index c65d2cdfbd906..fb17e32e163dc 100644 --- a/apps/web/src/components/settings/KeybindingsSettings.browser.test.tsx +++ b/apps/web/src/components/settings/KeybindingsSettings.browser.test.tsx @@ -1,9 +1,20 @@ import { RegistryProvider } from "@effect/atom-react"; -import { userEvent } from "vite-plus/test/browser"; -import { expect, it, vi } from "vite-plus/test"; +import { describe, expect, it, vi } from "vite-plus/test"; +import { page, userEvent, type Locator } from "vite-plus/test/browser"; import { render } from "../../../test/render"; +vi.mock(import("../../state/session"), async (original) => ({ + ...(await original()), + useEnvironmentScope: () => true, + useEnvironmentsWithScope: ((environments: ReadonlyArray<{ environmentId: string }>) => + new Set(environments.map(({ environmentId }) => environmentId))) as never, + readEnvironmentScope: () => true, +})); +vi.mock(import("../../state/use-atom-command"), async (original) => ({ + ...(await original()), + useAtomCommand: (() => vi.fn()) as never, +})); vi.mock(import("./SettingsScopeContext"), async (original) => ({ ...(await original()), useSettingsScope: (() => { @@ -22,16 +33,78 @@ vi.mock(import("./SettingsScopeContext"), async (original) => ({ }; }) as never, })); -vi.mock(import("../../state/session"), async (original) => ({ - ...(await original()), - useEnvironmentScope: () => true, - useEnvironmentsWithScope: ((environments: ReadonlyArray<{ environmentId: string }>) => - new Set(environments.map((environment) => environment.environmentId))) as never, - readEnvironmentScope: () => true, -})); +import { Dialog, DialogPopup, DialogTitle } from "../ui/dialog"; import { KeybindingsSettingsPanel } from "./KeybindingsSettings"; +// The modal dialog's inert layer swallows pointer events in this harness, so drive the DOM. +async function activate(locator: Locator, action: "click" | "focus") { + await expect.element(locator).toBeVisible(); + (locator.element() as HTMLElement)[action](); +} + +const existingField = page.getByRole("textbox", { name: "Keybinding for Sidebar: Toggle" }); +const existingPill = page.getByRole("button", { name: /^Edit shortcut for Sidebar: Toggle:/ }); +const newField = page.getByRole("textbox", { name: "Keybinding for new keybinding" }); + +describe("Esc in a shortcut recorder", () => { + it.each<{ + name: string; + field: Locator; + open: () => Promise; + expectReset: () => Promise; + }>([ + { + name: "existing binding", + field: existingField, + open: () => activate(existingPill, "click"), + expectReset: async () => { + await expect.element(existingField).not.toBeInTheDocument(); + await expect.element(existingPill).toHaveFocus(); + await expect.element(page.getByRole("button", { name: "Save" })).not.toBeInTheDocument(); + }, + }, + { + name: "new binding", + field: newField, + open: async () => { + await activate(page.getByRole("button", { name: "Add keybinding" }), "click"); + await activate(newField, "focus"); + }, + expectReset: async () => { + await expect.element(newField).toHaveValue(""); + await expect.element(newField).toHaveAttribute("placeholder", "Unassigned"); + }, + }, + ])("leaves the $name field without closing Settings", async ({ field, open, expectReset }) => { + const onOpenChange = vi.fn(); + await using _screen = await render( + + + Settings + + + , + ); + + await open(); + await userEvent.keyboard("{Control>}k{/Control}"); + await expect.element(field).not.toHaveValue(""); + await userEvent.tab(); + await activate(field, "focus"); + await expect.element(field).toHaveAttribute("placeholder", "Press shortcut"); + + await userEvent.keyboard("{Escape}"); + + expect(onOpenChange).not.toHaveBeenCalled(); + expect(document.activeElement?.matches("[data-keybinding-capture]")).toBe(false); + await expectReset(); + + await userEvent.keyboard("{Escape}"); + expect(onOpenChange).toHaveBeenCalledWith(false, expect.anything()); + }); +}); + it("re-enables Save when a When editor closes on an expression it could not parse", async () => { await using screen = await render( diff --git a/apps/web/src/components/settings/KeybindingsSettings.tsx b/apps/web/src/components/settings/KeybindingsSettings.tsx index f6298d778073e..cb00bca7e9cae 100644 --- a/apps/web/src/components/settings/KeybindingsSettings.tsx +++ b/apps/web/src/components/settings/KeybindingsSettings.tsx @@ -20,6 +20,7 @@ import { useRef, useState, } from "react"; +import { flushSync } from "react-dom"; import { AuthOrchestrationOperateScope, AuthSettingsWriteScope, @@ -717,6 +718,7 @@ function useKeybindingRowEditor({ onSave: (input: ServerUpsertKeybindingInput) => void; }) { const [draft, setDraft] = useReducer(keybindingRowDraftReducer, row, createKeybindingRowDraft); + const pillRef = useRef(null); const { keyDraft, whenDraft, isRecording, isWhenDraftValid } = draft; const whenDraftExpression = whenAstToExpression(whenDraft); const isDirty = keyDraft !== row.key || whenDraftExpression !== row.when; @@ -739,7 +741,13 @@ function useKeybindingRowEditor({ if (event.key === "Tab" && !isRecording) return; event.preventDefault(); if (event.key === "Escape") { - setDraft({ keyDraft: row.key, isRecording: false }); + // An unhandled Escape closes the settings page. + event.stopPropagation(); + const field = event.currentTarget; + flushSync(() => setDraft({ keyDraft: row.key, isRecording: false })); + // The field may unmount for the pill; focus would otherwise drop to the page. + if (pillRef.current) pillRef.current.focus(); + else field.blur(); return; } const next = keybindingFromKeyboardEvent(event.nativeEvent, navigator.platform, { @@ -760,6 +768,7 @@ function useKeybindingRowEditor({ setDraft, save, captureKeybinding, + pillRef, }; } @@ -790,8 +799,16 @@ function KeybindingKeyControl({ isSaving: boolean; pillClassName?: string | undefined; }) { - const { keyDraft, isRecording, isDirty, isWhenDraftValid, setDraft, save, captureKeybinding } = - editor; + const { + keyDraft, + isRecording, + isDirty, + isWhenDraftValid, + setDraft, + save, + captureKeybinding, + pillRef, + } = editor; const showPill = !isRecording && keyDraft === row.key && row.key.length > 0 && !isDirty; return ( @@ -807,6 +824,7 @@ function KeybindingKeyControl({ ) : null} {showPill ? (