From 8911201eb84357060132ef23f3f76cd478b0becb Mon Sep 17 00:00:00 2001
From: Usman Abeeb <136495186+therealbibson@users.noreply.github.com>
Date: Sun, 27 Sep 2026 04:53:23 +0100
Subject: [PATCH] fix(components): portal keyboard shortcuts overlay out of the
blurred header
Closes #658
---
.../KeyboardShortcutsModal.portal.test.tsx | 196 ++++++++++++++++++
src/components/KeyboardShortcutsModal.tsx | 79 +++++--
2 files changed, 258 insertions(+), 17 deletions(-)
create mode 100644 src/__tests__/KeyboardShortcutsModal.portal.test.tsx
diff --git a/src/__tests__/KeyboardShortcutsModal.portal.test.tsx b/src/__tests__/KeyboardShortcutsModal.portal.test.tsx
new file mode 100644
index 00000000..77572414
--- /dev/null
+++ b/src/__tests__/KeyboardShortcutsModal.portal.test.tsx
@@ -0,0 +1,196 @@
+/**
+ * Unit tests for the KeyboardShortcutsModal overlay container.
+ *
+ * Covers the fix for the header `?` button appearing to "do nothing": the
+ * overlay is `fixed inset-0`, but it is mounted from the header, which sets a
+ * `backdrop-blur`, making the header a containing block for `position: fixed`
+ * descendants. That confined the overlay to the header box instead of the
+ * viewport, so it must be portalled to `document.body`.
+ *
+ * Covers:
+ * - the dialog renders outside the host that mounted the trigger (portal)
+ * - the dialog is attached to `document.body`
+ * - categories render in a stable order (Navigation, Invoices, Payments, General)
+ * - each section's `aria-labelledby` resolves to a real, DOM-id-safe element
+ * - the close button still dismisses the overlay
+ */
+
+import React from "react";
+import { render, screen, fireEvent, act } from "@testing-library/react";
+import HeaderShortcutsButton from "@/components/HeaderShortcutsButton";
+import KeyboardShortcutsModal from "@/components/KeyboardShortcutsModal";
+import {
+ ShortcutRegistryProvider,
+ useRegisterShortcuts,
+} from "@/context/ShortcutRegistry";
+
+vi.mock("next/navigation", () => ({
+ useRouter: () => ({ push: vi.fn(), replace: vi.fn(), prefetch: vi.fn() }),
+ usePathname: () => "/",
+ useSearchParams: () => new URLSearchParams(),
+}));
+
+function renderWithRegistry(ui: React.ReactElement) {
+ return render({ui});
+}
+
+function openViaButton() {
+ act(() => {
+ screen.getByRole("button", { name: /keyboard shortcuts/i }).click();
+ });
+}
+
+describe("KeyboardShortcutsModal overlay container", () => {
+ test("renders the dialog outside the host that mounted the header button", () => {
+ const { container } = renderWithRegistry(
+
+
+
+ );
+
+ expect(screen.queryByRole("dialog")).toBeNull();
+ openViaButton();
+
+ const dialog = screen.getByRole("dialog");
+
+ // The overlay must not live inside the header host — that is exactly what
+ // broke `fixed inset-0` under the header's `backdrop-blur`.
+ expect(container.querySelector('[role="dialog"]')).toBeNull();
+ expect(document.body.contains(dialog)).toBe(true);
+ expect(dialog.parentElement).toBe(document.body);
+ });
+
+ test("keeps the ? button in the header while the overlay is portalled out", () => {
+ const { container } = renderWithRegistry(
+
+
+
+ );
+
+ openViaButton();
+
+ expect(
+ container.querySelector('[aria-label="Show keyboard shortcuts"]')
+ ).not.toBeNull();
+ expect(container.querySelector('[role="dialog"]')).toBeNull();
+ });
+
+ test("close button still dismisses the portalled overlay", () => {
+ renderWithRegistry();
+
+ openViaButton();
+ expect(screen.getByRole("dialog")).toBeInTheDocument();
+
+ act(() => {
+ screen.getByRole("button", { name: /close keyboard shortcuts/i }).click();
+ });
+
+ expect(screen.queryByRole("dialog")).toBeNull();
+ });
+});
+
+describe("KeyboardShortcutsModal category grouping", () => {
+ function Registrar() {
+ useRegisterShortcuts([
+ {
+ id: "test:general",
+ keys: ["X"],
+ description: "General entry",
+ group: "General",
+ handler: () => {},
+ },
+ {
+ id: "test:payments",
+ keys: ["P"],
+ description: "Payments entry",
+ group: "Payments",
+ handler: () => {},
+ },
+ {
+ id: "test:navigation",
+ keys: ["Y"],
+ description: "Navigation entry",
+ group: "Navigation",
+ handler: () => {},
+ },
+ {
+ id: "test:invoices",
+ keys: ["I"],
+ description: "Invoices entry",
+ group: "Invoices",
+ handler: () => {},
+ },
+ {
+ id: "test:extra",
+ keys: ["Z"],
+ description: "Extras entry",
+ group: "Zebra Extras",
+ handler: () => {},
+ },
+ ]);
+ return null;
+ }
+
+ function renderGrouped() {
+ return renderWithRegistry(
+ <>
+
+ {}} />
+
+ );
+ }
+
+ test("leads with the documented categories and keeps General last", () => {
+ renderGrouped();
+
+ const headings = screen
+ .getAllByRole("heading", { level: 3 })
+ .map((h) => h.textContent);
+
+ expect(headings[0]).toBe("Navigation");
+ expect(headings[1]).toBe("Invoices");
+ expect(headings[2]).toBe("Payments");
+ expect(headings[headings.length - 1]).toBe("General");
+ // Uncategorised extras sort between the lead categories and General.
+ expect(headings).toContain("Zebra Extras");
+ expect(headings.indexOf("Zebra Extras")).toBeLessThan(
+ headings.indexOf("General")
+ );
+ });
+
+ test("section aria-labelledby resolves to a DOM-id-safe heading", () => {
+ renderGrouped();
+
+ const sections = Array.from(
+ document.body.querySelectorAll("section[aria-labelledby]")
+ );
+ expect(sections.length).toBeGreaterThan(0);
+
+ for (const section of sections) {
+ const id = section.getAttribute("aria-labelledby")!;
+ // Multi-word categories must not produce an id with whitespace.
+ expect(id).toMatch(/^kbd-group-[a-z0-9-]+$/);
+ expect(id.split(/\s+/)).toHaveLength(1);
+ const heading = document.getElementById(id);
+ expect(heading).not.toBeNull();
+ expect(section.contains(heading)).toBe(true);
+ }
+
+ // `Zebra Extras` slugifies rather than emitting an unusable space.
+ expect(document.getElementById("kbd-group-zebra-extras")).not.toBeNull();
+ });
+
+ test("renders every registered entry so nothing is silently dropped", () => {
+ renderGrouped();
+
+ for (const label of [
+ "Navigation entry",
+ "Invoices entry",
+ "Payments entry",
+ "Zebra Extras entry",
+ "General entry",
+ ]) {
+ expect(screen.getByText(label)).toBeInTheDocument();
+ }
+ });
+});
diff --git a/src/components/KeyboardShortcutsModal.tsx b/src/components/KeyboardShortcutsModal.tsx
index 9a276e93..ebfb9fd9 100644
--- a/src/components/KeyboardShortcutsModal.tsx
+++ b/src/components/KeyboardShortcutsModal.tsx
@@ -1,6 +1,7 @@
"use client";
-import { useMemo } from "react";
+import { useEffect, useMemo, useState } from "react";
+import { createPortal } from "react-dom";
import FocusTrap from "@/components/FocusTrap";
import { useShortcutRegistry, type ShortcutDefinition } from "@/context/ShortcutRegistry";
@@ -10,32 +11,59 @@ interface Props {
// ── Helpers ───────────────────────────────────────────────────────────────────
+/** Fallback category for shortcuts that do not declare a `group`. */
+const GENERAL_GROUP = "General";
+
+/**
+ * Categories the reference overlay leads with, in this order. Anything the
+ * registry registers under another category is listed next (alphabetically),
+ * and `General` is always last so it never pushes real categories down.
+ */
+const LEAD_CATEGORIES = ["Navigation", "Invoices", "Payments"] as const;
+
+/** Turn a category label into a DOM-id-safe fragment (`Payments & Tips` → `payments-tips`). */
+function groupDomId(group: string): string {
+ const slug = group
+ .toLowerCase()
+ .replace(/[^a-z0-9]+/g, "-")
+ .replace(/^-+|-+$/g, "");
+ return `kbd-group-${slug || "group"}`;
+}
+
/**
- * Group shortcuts by their `group` field.
- * Groups are returned in the order they first appear in the registry,
- * with "General" always first when present.
+ * Group shortcuts by their `group` field and return them in a stable order:
+ * the categories above first, then any other category alphabetically, then
+ * `General` last. Ordering is derived from the data only, so the overlay never
+ * reshuffles between renders.
*/
function groupShortcuts(
shortcuts: ShortcutDefinition[],
-): Array<{ group: string; entries: ShortcutDefinition[] }> {
+): Array<{ group: string; id: string; entries: ShortcutDefinition[] }> {
const map = new Map();
- // Always seed General first so it stays at the top
- map.set("General", []);
-
for (const s of shortcuts) {
// Hide internal chord-activation entries from the overlay
if (s.id === "global:g-chord") continue;
- const group = s.group ?? "General";
+ const group = s.group?.trim() || GENERAL_GROUP;
if (!map.has(group)) map.set(group, []);
map.get(group)!.push(s);
}
- // Remove empty groups
+ const rank = (group: string): number => {
+ const lead = LEAD_CATEGORIES.indexOf(group as (typeof LEAD_CATEGORIES)[number]);
+ if (lead !== -1) return lead;
+ if (group === GENERAL_GROUP) return Number.MAX_SAFE_INTEGER;
+ return LEAD_CATEGORIES.length + 1;
+ };
+
return Array.from(map.entries())
.filter(([, entries]) => entries.length > 0)
- .map(([group, entries]) => ({ group, entries }));
+ .sort(([a], [b]) => {
+ const diff = rank(a) - rank(b);
+ return diff !== 0 ? diff : a.localeCompare(b);
+ })
+ .map(([group, entries]) => ({ group, id: groupDomId(group), entries }));
}
// ── Kbd chip ──────────────────────────────────────────────────────────────────
@@ -54,20 +82,36 @@ function KbdKey({ label }: { label: string }) {
* KeyboardShortcutsModal
*
* A help overlay that lists **all shortcuts registered via ShortcutRegistry**.
- * Shortcuts are grouped by their `group` field (defaults to "General").
+ * Shortcuts are grouped by their `group` field (defaults to "General") and the
+ * categories lead in a stable order (Navigation, Invoices, Payments, …, General).
*
* Triggered by pressing `?` outside text inputs, or clicking the `?` icon in
* the header. Closed by pressing Escape (handled in useKeyboardShortcuts) or
* clicking the backdrop / close button.
*
+ * The overlay is rendered through a portal into `document.body`. It is mounted
+ * from the header (`HeaderShortcutsButton` → `Navbar`), and that header sets a
+ * `backdrop-blur`, which makes the header a containing block for `position:
+ * fixed` descendants. Rendering in place confined the `fixed inset-0` overlay
+ * to the header box instead of the viewport, so the dialog appeared clipped and
+ * the backdrop never covered the page.
+ *
* Components register shortcuts with `useRegisterShortcuts` — the overlay
* automatically reflects additions and removals without any manual wiring.
*/
export default function KeyboardShortcutsModal({ onClose }: Props) {
const { shortcuts } = useShortcutRegistry();
const grouped = useMemo(() => groupShortcuts(shortcuts), [shortcuts]);
+ const [mounted, setMounted] = useState(false);
- return (
+ useEffect(() => {
+ setMounted(true);
+ }, []);
+
+ // `createPortal` needs a live DOM; skip the server/first paint.
+ if (!mounted || typeof document === "undefined") return null;
+
+ return createPortal(