fix(web): Escape while recording a shortcut no longer closes Settings

apps/web/src/components/settings/KeybindingsSettings.browser.test.tsx

@@ -0,0 +1,94 @@
+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: (() => {
+    const environment = { environmentId: "env", serverConfig: { keybindings: [] } };
+    return { environment, connectedEnvironments: [environment] };
+  }) as never,
+}));
+
+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<void>;
+    expectReset: () => Promise<void>;
+  }>([
+    {
+      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(
+      <Dialog open onOpenChange={onOpenChange}>
+        <DialogPopup>
+          <DialogTitle>Settings</DialogTitle>
+          <KeybindingsSettingsPanel />
+        </DialogPopup>
+      </Dialog>,
+    );
+
+    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());
+  });
+});

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<HTMLButtonElement>(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 ? (
         <button
+          ref={pillRef}
           type="button"
           onClick={() => setDraft({ isRecording: true })}
           aria-label={`Edit shortcut for ${commandLabel(row.command)}: ${formatShortcutLabel(row.binding.shortcut)}`}
@@ -1051,7 +1069,10 @@ function useNewKeybindingDraft({
     if (event.key === "Tab" && !isRecording) return;
     event.preventDefault();
     if (event.key === "Escape") {
+      // An unhandled Escape closes the settings page.
+      event.stopPropagation();
       setDraft({ keyDraft: "", isRecording: false });
+      event.currentTarget.blur();
       return;
     }
     const next = keybindingFromKeyboardEvent(event.nativeEvent, navigator.platform, {