Skip to content

[Bug]: Escape in the keybindings search clears the query and closes Settings #35146

Closed

Description

@coygeek

Before submitting

  • I searched existing issues and did not find a duplicate.
  • I included enough detail to reproduce or investigate the problem.

Area

apps/web

Steps to reproduce

In the app: open Settings → Keybindings, click "Search keybindings", type sidebar, and press Escape. The query clears and Settings closes.

For a deterministic reproduction, use the commit under Version or commit, with lockfile dependencies installed and the repository-pinned Playwright Chromium runtime available. Save the script below outside the checkout as repro.sh and run env -u NODE_ENV sh /path/to/repro.sh /path/to/t3code, passing the checkout root; the script changes into apps/web itself. It sets a private HOME, TMPDIR and T3CODE_HOME under a mktemp directory and reads PLAYWRIGHT_BROWSERS_PATH, defaulting to the Playwright cache under the original home directory (~/Library/Caches/ms-playwright on macOS, ~/.cache/ms-playwright elsewhere).

  1. The script installs one temporary browser test at apps/web/src/components/settings/verifyKeybindingsSearchEscape.browser.test.tsx, refusing to overwrite an existing file, and removes it and its mktemp directory on exit.
  2. Each test renders the production AppRouteModal labelled "Settings" around the production KeybindingsSettingsPanel (or, for the main-search control, SettingsSidebarNav inside SidebarProvider), as the Settings route does. Only environment and data boundaries are mocked, with the same environment-scope fixtures as the existing recorder browser tests: ../../state/session, ../../state/use-atom-command, ./SettingsScopeContext and ./useAvailableSettingsSearchItems. The search handler, recorder, modal, Base UI dialog dismissal and browser keyboard input run unmocked, and no T3 server is needed.
  3. Failure: it fills "Search keybindings" with sidebar, presses Escape, and asserts that the input value is empty and that the Settings dialog has been removed from the document.
  4. Recorder control: it clicks the "Sidebar: Toggle" shortcut to start recording, presses Escape, and asserts that recording was canceled, focus returned to the shortcut button and Settings is still visible; a second Escape then removes Settings.
  5. Main-search control: it fills "Search settings" with theme, presses Escape, and asserts that the query is empty and Settings is still visible.
  6. Workaround: it fills "Search keybindings" with sidebar, presses Select All then Backspace, and asserts that the query is empty and Settings is still visible.
  7. The script exits 0 only when all four tests pass, that is, when the failure, both controls and the workaround all hold.

This is a production-component browser test, not a run of the packaged desktop app.

Reproduction script: repro.sh
#!/bin/sh
set -eu
checkout=${1:?Usage: sh repro.sh /path/to/dependency-installed/t3code}
checkout=$(cd "$checkout" && pwd)
fixture=$(mktemp -d "${TMPDIR:-/tmp}/keybindings-escape.XXXXXX")
test_file="$checkout/apps/web/src/components/settings/verifyKeybindingsSearchEscape.browser.test.tsx"
if [ -e "$test_file" ]; then echo 'refusing to overwrite existing test fixture' >&2; exit 2; fi
cleanup() { rm -f "$test_file"; rm -rf "$fixture"; }
trap cleanup EXIT HUP INT TERM
if [ -n "${PLAYWRIGHT_BROWSERS_PATH:-}" ]; then browser_cache=$PLAYWRIGHT_BROWSERS_PATH
elif [ "$(uname -s)" = Darwin ]; then browser_cache="$HOME/Library/Caches/ms-playwright"
else browser_cache="$HOME/.cache/ms-playwright"; fi
mkdir -p "$fixture/fake-home" "$fixture/tmp" "$fixture/t3home"
cat > "$test_file" <<'TSX'
import { expect, it, vi } from "vite-plus/test";
import { page, userEvent } 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",
      label: "Build workstation",
      serverConfig: { keybindings: [], keybindingsConfigPath: null, availableEditors: [] },
    };
    return { scope: { kind: "environment" }, environment,
      environments: [environment], connectedEnvironments: [environment], targets: [], target: null };
  }) as never,
}));
vi.mock("./useAvailableSettingsSearchItems", () => ({
  useAvailableSettingsSearchItems: () => [],
  useAvailableSettingsNavGroups: () => [],
}));

import { AppRouteModal } from "../AppRouteModal";
import { KeybindingsSettingsPanel } from "./KeybindingsSettings";
import { SettingsSidebarNav } from "./SettingsSidebarNav";
import { SidebarProvider } from "../ui/sidebar";

it("failure: Escape while searching keybindings dismisses the actual Settings modal", async () => {
  await using screen = await render(
    <AppRouteModal label="Settings"><KeybindingsSettingsPanel /></AppRouteModal>,
    { router: "/settings/keybindings" },
  );
  const dialog = page.getByRole("dialog", { name: "Settings" });
  const search = page.getByRole("searchbox", { name: "Search keybindings" });
  await expect.element(dialog).toBeVisible();
  await search.fill("sidebar");
  await expect.element(search).toHaveValue("sidebar");
  expect(document.activeElement).toBe(search.element());
  const input = search.element() as HTMLInputElement;
  await userEvent.keyboard("{Escape}");
  expect(input.value).toBe("");
  await expect.element(dialog).not.toBeInTheDocument();
  console.log("FAILURE: query=sidebar; Escape cleared query; Settings dialog removed=true");
});

it("control: Escape cancels recording, then a second Escape dismisses Settings", async () => {
  await using screen = await render(
    <AppRouteModal label="Settings"><KeybindingsSettingsPanel /></AppRouteModal>,
    { router: "/settings/keybindings" },
  );
  const dialog = page.getByRole("dialog", { name: "Settings" });
  const pill = page.getByRole("button", { name: /^Edit shortcut for Sidebar: Toggle:/ });
  await expect.element(pill).toBeVisible();
  (pill.element() as HTMLElement).click();
  const field = page.getByRole("textbox", { name: "Keybinding for Sidebar: Toggle" });
  await expect.element(field).toHaveAttribute("placeholder", "Press shortcut");
  expect(document.activeElement).toBe(field.element());
  await userEvent.keyboard("{Escape}");
  await expect.element(dialog).toBeVisible();
  await expect.element(field).not.toBeInTheDocument();
  await expect.element(pill).toHaveFocus();
  console.log("CONTROL recorder: first Escape kept Settings open=true; recorder canceled=true");
  await userEvent.keyboard("{Escape}");
  await expect.element(dialog).not.toBeInTheDocument();
  console.log("CONTROL recorder: second Escape removed Settings=true");
});

it("control: main Settings search clears a query and retains Settings", async () => {
  await using screen = await render(
    <SidebarProvider defaultOpen><AppRouteModal label="Settings"><SettingsSidebarNav pathname="/settings/keybindings" /></AppRouteModal></SidebarProvider>,
    { router: "/settings/keybindings" },
  );
  const dialog = page.getByRole("dialog", { name: "Settings" });
  const search = page.getByRole("combobox", { name: "Search settings" });
  await expect.element(search).toBeVisible();
  await search.fill("theme");
  await expect.element(search).toHaveValue("theme");
  await userEvent.keyboard("{Escape}");
  await expect.element(search).toHaveValue("");
  await expect.element(dialog).toBeVisible();
  console.log("CONTROL Settings search: first Escape cleared query=true; kept Settings open=true");
});

it("workaround: deleting the keybindings query leaves Settings open", async () => {
  await using screen = await render(
    <AppRouteModal label="Settings"><KeybindingsSettingsPanel /></AppRouteModal>,
    { router: "/settings/keybindings" },
  );
  const search = page.getByRole("searchbox", { name: "Search keybindings" });
  await search.fill("sidebar");
  await userEvent.keyboard("{ControlOrMeta>}a{/ControlOrMeta}{Backspace}");
  await expect.element(search).toHaveValue("");
  await expect.element(page.getByRole("dialog", { name: "Settings" })).toBeVisible();
  console.log("WORKAROUND: Select All, Backspace cleared query and kept Settings open=true");
});
TSX
cd "$checkout/apps/web"
env -u NODE_ENV HOME="$fixture/fake-home" TMPDIR="$fixture/tmp" T3CODE_HOME="$fixture/t3home" \
  PLAYWRIGHT_BROWSERS_PATH="$browser_cache" PATH="$checkout/node_modules/.bin:$PATH" \
  vp test run --project browser --silent=false --reporter=verbose src/components/settings/verifyKeybindingsSearchEscape.browser.test.tsx

Expected behavior

Escape in the keybindings search should clear a nonempty query and leave Settings open. The search's own key handler claims Escape: it calls preventDefault() and clears a nonempty query (or blurs an empty one). The main Settings search handler does the same for its query and also stops propagation, and the browser control confirms it keeps Settings open. The recorder's Escape handler carries the comment "An unhandled Escape closes the settings page" and stops propagation for the Escape it handles. Before the modal Settings commit 3faeb9e (#24847), this same search handler kept Settings open, because the old useEscapeToGoBack hook ignored an Escape whose default was already prevented; that history was read from source and not run in a browser.

This expectation applies to query-clearing controls. Ordinary commit-on-blur text fields in Settings intentionally let Escape close Settings after committing their value (merged #25131), and this report does not ask to change that.

Actual behavior

Summary. In Settings → Keybindings, pressing Escape in the "Search keybindings" box with a query typed clears the query and also closes Settings, on the same keypress. The search's own key handler treats Escape as "clear the query", and the two comparable Escape handlers in Settings keep Settings open: Escape while recording a shortcut cancels the recording, and Escape in the main "Search settings" box clears its query. Merged #27151 stopped the two shortcut recorders' Escape from closing Settings and did not change this search input, so this is a sibling gap that fix did not cover, not a regression of #27151. A user who presses Escape to clear a keybindings filter is thrown out of Settings and has to reopen it and navigate back to Keybindings. Selecting the query text and pressing Backspace clears it without closing Settings.

Observed in Chromium on the tested commit: after typing sidebar in "Search keybindings" and pressing Escape, the input value became empty and the Settings dialog was removed on the same keypress. In the recorder control, the first Escape canceled recording, returned focus to the shortcut button and kept Settings open; the second Escape closed Settings. In the main-search control, Escape cleared theme and kept Settings open. Select All followed by Backspace cleared the keybindings query and kept Settings open.

The production keybindings route renders KeybindingsSettingsPanel, and the Settings layout route renders it inside AppRouteModal, whose onOpenChange handler closes the modal. The keybindings search calls preventDefault() without stopPropagation(), while both controls that keep Settings open stop propagation. From reading the installed Base UI source, its dialog Escape dismissal listens on document and does not check defaultPrevented, which would explain why the search's preventDefault() does not keep Settings open; that cause was not separately tested.

On current main (under Version or commit), KeybindingsSettings.tsx, AppRouteModal.tsx and both Settings route files are byte-identical to the tested commit, so the failure persists there by source comparison; the browser test was not rerun on main. Desktop serves the same web Settings component and the search has no Electron-specific handler, so the same code path applies there; the native Electron app was not run. Mobile was not tested.

Related, not duplicates: merged #27151 fixed Escape for the two shortcut recorders only. Open #32859 cancels Escape only when its target is outside the Settings popup, and this input is inside it. Open #25130 handles Escape in the theme editor. Open #31306 changes only the recorders' Escape handling in the same file and leaves this search handler unchanged.

Evidence

  • Expected source: the keybindings search's Escape handler in apps/web/src/components/settings/KeybindingsSettings.tsx lines 127-139, and the main Settings search's propagation-stopping handler in apps/web/src/components/settings/SettingsSidebarNav.tsx lines 227-233, whose behavior the browser control confirms.
  • Failure source: the observed Chromium output under Logs or stack traces, from the production AppRouteModal and KeybindingsSettingsPanel; the search handler at KeybindingsSettings.tsx lines 127-139 calls preventDefault() but not stopPropagation().
  • Evidence provenance: observed
  • Local verification: reproduced
  • Reproduction completeness: complete

The tests mock environment scope, RPC command and Settings search-index hooks so they need no server; everything that handles the key event runs unmocked. The browser output also logs "Could not discover the primary environment" warnings because no backend runs; all assertions completed. I did not establish whether unsaved keybinding row edits can be lost when Settings closes this way, so this report makes no such claim.

Restoration check

With the script's input, Escape in "Search keybindings" with the query sidebar clears the query and the Settings dialog stays visible. As controls, Escape while recording a shortcut still cancels recording and keeps Settings open, a second Escape still closes Settings, Escape in "Search settings" still clears its query and keeps Settings open, and Escape in an ordinary commit-on-blur text field still commits and closes Settings as #25131 intends. The script asserts the current failing result, so its failure test needs updating to the restored result for this check.

Impact

Minor bug or occasional failure

Version or commit

0.1.9 at d9805b2 (the relevant source is unchanged through main at dd9c2f8, as described under Actual behavior)

Environment

macOS arm64, Node v26.12.1, repository-pinned Vite+ 1.4.0, Vitest 5.0.5 and Playwright 1.64.1, Chromium through the repo's browser test project; no packaged desktop app, native Electron window or mobile client

Logs or stack traces

The script's output, with the test runner's path lines and backend-discovery warnings removed:

FAILURE: query=sidebar; Escape cleared query; Settings dialog removed=true
CONTROL recorder: first Escape kept Settings open=true; recorder canceled=true
CONTROL recorder: second Escape removed Settings=true
CONTROL Settings search: first Escape cleared query=true; kept Settings open=true
WORKAROUND: Select All, Backspace cleared query and kept Settings open=true
Test Files  1 passed (1)
Tests  4 passed (4)

Screenshots, recordings, or supporting files

No response

Workaround

Select the text in "Search keybindings" and press Backspace. The test's workaround step shows this clears the query and keeps Settings open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions