From 95e5c69e8fb75d510d754fa1e2b063628e22fc72 Mon Sep 17 00:00:00 2001
From: John Coffey <johnellis@linux.com>
Date: Thu, 3 Sep 2026 14:38:15 -0700
Subject: [PATCH] Keep shortcuts working after a checkbox is clicked

Ticking "select all" disabled every keyboard shortcut until the reader clicked
somewhere else (#260). Same for the per-message checkboxes, so selecting a few
messages and pressing e to archive them did nothing.

The guard that stops "a" archiving while you are typing into the search box
tested `tagName === "INPUT"`. That is also true of a checkbox, and a checkbox
keeps focus after a click -- correctly, since space should toggle it again.
So the guard was suppressing shortcuts for an element that swallows no
keystroke: space is handled by the browser before this listener runs.

The question is not "is this an input" but "does this input take text", which
is what isTextEntry now asks. A <select> counts, in the sense that matters
here: typing a letter jumps to the option starting with it, and a shortcut
would steal that.

Thirteen checkboxes and seven file inputs across the app were affected, not
just the one reported.

The regression test was checked against the old guard first: it fails there
and passes here, which is the only thing that makes it a regression test.
---
 web/src/lib/__tests__/keyboardFocus.test.ts | 94 +++++++++++++++++++++
 web/src/lib/keyboard.ts                     | 36 +++++++-
 2 files changed, 127 insertions(+), 3 deletions(-)
 create mode 100644 web/src/lib/__tests__/keyboardFocus.test.ts

diff --git a/web/src/lib/__tests__/keyboardFocus.test.ts b/web/src/lib/__tests__/keyboardFocus.test.ts
new file mode 100644
index 0000000..8497b3b
--- /dev/null
+++ b/web/src/lib/__tests__/keyboardFocus.test.ts
@@ -0,0 +1,94 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { isTextEntry, keyboard } from "@/lib/keyboard";
+
+/*
+ * Shortcuts after a click on a checkbox (#260).
+ *
+ * The guard that stops "a" archiving while you are typing into the search box
+ * tested `tagName === "INPUT"`, which is also true of a checkbox. A checkbox
+ * keeps focus after a click, so ticking "select all" disabled every shortcut
+ * until the reader clicked somewhere else — and nothing about a checkbox
+ * swallows a keystroke in the first place.
+ */
+
+const pressFrom = (el: Element, key: string) => {
+  const e = new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true });
+  el.dispatchEvent(e);
+  return e;
+};
+
+let pop: (() => void) | null = null;
+afterEach(() => {
+  pop?.();
+  pop = null;
+  document.body.innerHTML = "";
+  vi.restoreAllMocks();
+});
+
+describe("isTextEntry", () => {
+  const input = (type?: string) => {
+    const el = document.createElement("input");
+    if (type) el.setAttribute("type", type);
+    return el;
+  };
+
+  it("is false for the inputs you cannot type into", () => {
+    for (const type of ["checkbox", "radio", "button", "submit", "reset", "file", "color", "range"]) {
+      expect(isTextEntry(input(type)), type).toBe(false);
+    }
+  });
+
+  it("is true for the ones you can", () => {
+    for (const type of ["text", "search", "email", "url", "tel", "password", "number", "date", "time"]) {
+      expect(isTextEntry(input(type)), type).toBe(true);
+    }
+  });
+
+  it("treats an input with no type as text, which is what the browser does", () => {
+    expect(isTextEntry(input())).toBe(true);
+  });
+
+  it("covers textarea, select and contenteditable", () => {
+    expect(isTextEntry(document.createElement("textarea"))).toBe(true);
+    // A select takes letters too: typing jumps to the matching option, and a
+    // shortcut would steal that.
+    expect(isTextEntry(document.createElement("select"))).toBe(true);
+    const div = document.createElement("div");
+    div.contentEditable = "true";
+    Object.defineProperty(div, "isContentEditable", { value: true });
+    expect(isTextEntry(div)).toBe(true);
+  });
+
+  it("is false for a button and for nothing at all", () => {
+    expect(isTextEntry(document.createElement("button"))).toBe(false);
+    expect(isTextEntry(null)).toBe(false);
+  });
+});
+
+describe("shortcuts with a checkbox focused", () => {
+  it("still fire — the reported bug", () => {
+    const handler = vi.fn();
+    pop = keyboard.pushScope("test", [{ keys: "e", description: "Archive", group: "Mail", handler }]);
+
+    const box = document.createElement("input");
+    box.type = "checkbox";
+    document.body.appendChild(box);
+    box.focus();
+
+    pressFrom(box, "e");
+    expect(handler).toHaveBeenCalledTimes(1);
+  });
+
+  it("still do not fire from a text field", () => {
+    const handler = vi.fn();
+    pop = keyboard.pushScope("test", [{ keys: "e", description: "Archive", group: "Mail", handler }]);
+
+    const field = document.createElement("input");
+    field.type = "search";
+    document.body.appendChild(field);
+    field.focus();
+
+    pressFrom(field, "e");
+    expect(handler).not.toHaveBeenCalled();
+  });
+});
diff --git a/web/src/lib/keyboard.ts b/web/src/lib/keyboard.ts
index 3711db8..93f973b 100644
--- a/web/src/lib/keyboard.ts
+++ b/web/src/lib/keyboard.ts
@@ -54,9 +54,7 @@ class Keyboard {
     // Let modal dialogs and popovers handle their own keys (Escape, arrows, ...).
     if (document.querySelector(".dialog-backdrop, .popover")) return;
     const target = e.target as HTMLElement | null;
-    const inInput =
-      !!target &&
-      (target.tagName === "INPUT" || target.tagName === "TEXTAREA" || target.tagName === "SELECT" || target.isContentEditable);
+    const inInput = isTextEntry(target);
     const combo = comboOf(e);
     if (!combo) return;
 
@@ -102,6 +100,38 @@ class Keyboard {
   }
 }
 
+/**
+ * Is the focused element somewhere the reader is typing?
+ *
+ * This guard exists so that pressing "a" in the search box searches for "a"
+ * rather than archiving the message behind it. The test used to be
+ * `tagName === "INPUT"`, which is true of a checkbox — and a checkbox keeps
+ * focus after you click it, so ticking "select all" silently disabled every
+ * shortcut until the reader clicked somewhere else (#260). Nothing about a
+ * checkbox swallows a keystroke: space toggles it and the browser handles
+ * that before this listener ever runs.
+ *
+ * So the question is not "is this an input" but "does this input take text".
+ * A `<select>` does, in the sense that matters here: typing a letter jumps to
+ * the option beginning with it, which a shortcut would steal.
+ */
+const TEXT_ENTRY_TYPES = new Set([
+  "text", "search", "email", "url", "tel", "password", "number",
+  "date", "datetime-local", "month", "time", "week",
+]);
+
+export function isTextEntry(el: Element | null): boolean {
+  if (!el) return false;
+  const node = el as HTMLElement;
+  if (node.isContentEditable) return true;
+  const tag = node.tagName;
+  if (tag === "TEXTAREA" || tag === "SELECT") return true;
+  if (tag !== "INPUT") return false;
+  // An <input> with no type attribute is a text field.
+  const type = (node as HTMLInputElement).type?.toLowerCase() || "text";
+  return TEXT_ENTRY_TYPES.has(type);
+}
+
 const isMac = typeof navigator !== "undefined" && /Mac|iPhone|iPad/.test(navigator.platform);
 
 export function comboOf(e: KeyboardEvent): string | null {