From b78a207313e2a99301691e9467880c65481bd0ee Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 14:45:56 +0200 Subject: [PATCH 1/7] feat(shortcuts): define every app-wide chord in one registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #243 asked for shortcuts "wherever they make sense" without a listener per page. What existed was three separate conventions: the new-conversation hook had its own idea of a press (`event.code`, refusing Ctrl/Cmd), the project switcher compared `event.key` and shipped its own chord string, and the chat page matched `/` inline. Two spellings of one rule is how a chord starts disagreeing with the hint that advertises it, so the matching, the labels and the catalogue live here from now on — `isShortcutPress`, `shortcutChord`, `SHORTCUTS` — and the components listen through them. Decisions worth naming: - `code` for letter chords, the produced character for punctuation. The handed-over plan matched `?` on `code: "Slash"`, which is dead on a German keyboard: `?` is Shift+ß there and `code: "Slash"` belongs to `-`. Character-matched chords (`?`, `/`, `,`) work on whichever layout the person brought, which is the entire point of a help key. - AltGr is refused by refusing Ctrl+Alt — how Windows reports it — because a bare `altKey` check fires on every `{`, `[`, `}` and `@` a European layout types with it. - Shift must match exactly on physical-key chords and is ignored on character chords: producing `?` requires Shift on every layout that has it, so the character is the whole claim there. - `scope: "global" | "surface"` says who answers. The destinations and `?` are the shortcuts layer's own; Ctrl/Cmd+K, Alt+N, Alt+S, `/` and Esc are answered where they act — the switcher, the chat, the sidebar, dialogs — but are registered here all the same, so the help view cannot document a chord the app does not answer or miss one it does. - Destination entries carry a `path`, not a permission callback: the gate is `canAccessRoute`, the rule the sidebar and the manager guard already use, instead of the plan's second spelling of the same rule. - `Alt+C` for Chat is new. The plan gave the buddy (`Alt+U`) a chord while the sidebar's busiest entry stayed unlabelled, even though `/buddy` is Chat's own second half. Part of #243. --- src/features/shortcuts/shortcuts.ts | 320 ++++++++++++++++++ .../unit/features/shortcuts/shortcuts.test.ts | 154 +++++++++ 2 files changed, 474 insertions(+) create mode 100644 src/features/shortcuts/shortcuts.ts create mode 100644 tests/unit/features/shortcuts/shortcuts.test.ts diff --git a/src/features/shortcuts/shortcuts.ts b/src/features/shortcuts/shortcuts.ts new file mode 100644 index 000000000..fac1764f9 --- /dev/null +++ b/src/features/shortcuts/shortcuts.ts @@ -0,0 +1,320 @@ +import type { AppRoute } from "../../auth/accessPolicy"; +import { isTypingTarget } from "../easter-eggs/lib/keyTargets"; + +/** + * Every chord in the app, defined once. + * + * Before this module, each shortcut was a one-off listener with its own idea of what a + * press is — `useNewConversationShortcut` refused Ctrl, `ProjectSwitcher` compared + * `event.key`. Two spellings of one rule is how a chord starts disagreeing with the hint + * that advertises it, so the matching lives here ({@link isShortcutPress}), the labels + * live here ({@link shortcutChord}), and the components listen through these definitions + * instead of their own. + * + * `code` rather than `key` is the convention for letter chords (`KeyH` is the H key + * wherever the keyboard was made). `key` is for characters no single physical key owns: + * `?` is Shift+ß on a German keyboard and Shift+/ on a US one, so the produced character + * is the only honest match — a `code: "Slash"` chord is dead on half the team's keyboards. + * + * `scope` says who answers a press. `global` items are the shortcuts layer's own + * (`GlobalShortcuts`): it navigates, or opens the help. `surface` items are answered by + * the component the chord belongs to — the sidebar owns its drawer, the switcher owns its + * dialog — and are listed here all the same, so the help view cannot drift from them. + */ +export type ShortcutCategory = "Navigation" | "Actions" | "General"; + +/** Who answers the keypress — see the module docstring. */ +export type ShortcutScope = "global" | "surface"; + +export type ShortcutItem = { + id: string; + label: string; + category: ShortcutCategory; + scope: ShortcutScope; + /** Physical key, layout-independent. One of `code` / `key` is set, never both. */ + code?: string; + /** The produced character, for keys no single `code` owns. */ + key?: string; + altKey?: boolean; + /** Ctrl on Windows/Linux, Cmd on macOS. */ + ctrlOrMeta?: boolean; + shiftKey?: boolean; + /** Navigation destination. Its presence means `canAccessRoute` gates the chord. */ + path?: AppRoute; + /** Fires while a text field has focus. Only the new-conversation chord wants that. */ + allowInInput?: boolean; + /** A word for the odd one out, where a chord only works somewhere specific. */ + note?: string; +}; + +/** + * The navigation chords, and why Alt: every Ctrl/Cmd combination a desktop browser uses + * belongs to the browser and a page cannot refuse it — the same reason `Alt+N` was chosen + * over `Ctrl+N` for a new conversation. The Alt row is free for all of these on Chrome, + * Edge and Firefox on Windows, the platform this team works on, and the letters are the + * destinations' first letters. + */ +const NAVIGATE_DASHBOARD: ShortcutItem = { + id: "nav-home", + label: "Dashboard", + category: "Navigation", + scope: "global", + code: "KeyH", + altKey: true, + path: "/", +}; + +const NAVIGATE_BOARD: ShortcutItem = { + id: "nav-board", + label: "Board", + category: "Navigation", + scope: "global", + code: "KeyB", + altKey: true, + path: "/board", +}; + +/** + * The chat's own chord. The handed-over plan left Chat without one while giving `Alt+U` to + * the buddy — which is the same sidebar entry's second half (`/buddy` lights up Chat), so + * the entry people actually click every day would have stayed the only unlabelled one. + */ +const NAVIGATE_CHAT: ShortcutItem = { + id: "nav-chat", + label: "Chat", + category: "Navigation", + scope: "global", + code: "KeyC", + altKey: true, + path: "/chat", +}; + +const NAVIGATE_BUDDY: ShortcutItem = { + id: "nav-buddy", + label: "Buddy", + category: "Navigation", + scope: "global", + code: "KeyU", + altKey: true, + path: "/buddy", +}; + +const NAVIGATE_KNOWLEDGE_BASE: ShortcutItem = { + id: "nav-kb", + label: "Knowledge Base", + category: "Navigation", + scope: "global", + code: "KeyK", + altKey: true, + path: "/knowledge-base", +}; + +/** + * PM Dashboard. `path` gating means the `canAccessRoute` rule decides who may use it — a + * hire pressing the chord is refused exactly like one typing the URL. + */ +const NAVIGATE_PM_DASHBOARD: ShortcutItem = { + id: "nav-pm", + label: "PM Dashboard", + category: "Navigation", + scope: "global", + code: "KeyP", + altKey: true, + path: "/pm-dashboard", +}; + +/** + * Character-matched like the other punctuation: the comma is the character somebody + * presses for, and every layout spells it the same way while placing it differently + * (QWERTZ and QWERTY leave it unshifted, AZERTY shifts it) — a `code` chord would work + * here and fail there. + */ +const NAVIGATE_SETTINGS: ShortcutItem = { + id: "nav-settings", + label: "Settings", + category: "Navigation", + scope: "global", + key: ",", + altKey: true, + path: "/settings", +}; + +/** + * Ctrl/Cmd+K, the one chord with a platform split. The dialog it opens is the project + * switcher's, so the switcher answers it (`scope: "surface"`) — the registry's job is only + * to guarantee the hint on its trigger and its handler read the same definition. + */ +export const SWITCH_PROJECT_SHORTCUT: ShortcutItem = { + id: "act-switcher", + label: "Switch project", + category: "Actions", + scope: "surface", + code: "KeyK", + ctrlOrMeta: true, +}; + +/** + * Alt+N fires even while the composer has focus — the moment somebody most wants a fresh + * conversation is halfway through typing into the wrong one. The cost is named in the + * hook: on macOS `Option+N` is the dead key for `ñ`, a trade this Windows-first team takes. + */ +export const NEW_CONVERSATION_SHORTCUT: ShortcutItem = { + id: "act-new-chat", + label: "New conversation", + category: "Actions", + scope: "surface", + code: "KeyN", + altKey: true, + allowInInput: true, +}; + +/** + * Alt+S works the drawer the mobile header's button works. There is no desktop collapse to + * toggle yet (#244); until then the chord is a no-op on a wide screen rather than a second + * behaviour, because inventing one here would only have to be undone there. + */ +export const SIDEBAR_TOGGLE_SHORTCUT: ShortcutItem = { + id: "act-sidebar", + label: "Toggle sidebar", + category: "Actions", + scope: "surface", + code: "KeyS", + altKey: true, + note: "small screens", +}; + +/** + * `/` focuses the chat composer (ChatPage, since before this registry existed, the way + * Slack and GitHub do it). Character-matched: on a German keyboard `/` is Shift+7. + */ +const FOCUS_COMPOSER: ShortcutItem = { + id: "act-focus-composer", + label: "Jump to the message box", + category: "Actions", + scope: "surface", + key: "/", + note: "in Chat", +}; + +/** + * The help itself. Character-matched for the same reason as `/`: `?` is Shift+ß on a + * German keyboard and Shift+/ on a US one — no single `code` describes "the question + * mark", and the handed-over plan's `code: "Slash"` would never have fired here. + */ +const SHOW_SHORTCUTS: ShortcutItem = { + id: "gen-shortcuts", + label: "Keyboard shortcuts", + category: "General", + scope: "global", + key: "?", +}; + +/** + * Esc is documented, not handled from here. Thirty-odd dialogs, popovers and the buddy + * dock already own it locally — several of them checking `event.defaultPrevented` before + * acting — and a global handler that also called `preventDefault` would break the layered + * cases in the name of listing them. + */ +const DISMISS: ShortcutItem = { + id: "gen-dismiss", + label: "Close a dialog or menu", + category: "General", + scope: "surface", + code: "Escape", +}; + +/** + * The catalogue, in the order the help view reads it: what you navigate with, what you do + * with, and the two general keys. + */ +export const SHORTCUTS: readonly ShortcutItem[] = [ + NAVIGATE_DASHBOARD, + NAVIGATE_BOARD, + NAVIGATE_CHAT, + NAVIGATE_BUDDY, + NAVIGATE_KNOWLEDGE_BASE, + NAVIGATE_PM_DASHBOARD, + NAVIGATE_SETTINGS, + SWITCH_PROJECT_SHORTCUT, + NEW_CONVERSATION_SHORTCUT, + SIDEBAR_TOGGLE_SHORTCUT, + FOCUS_COMPOSER, + SHOW_SHORTCUTS, + DISMISS, +]; + +const IS_MAC = typeof navigator !== "undefined" && /Mac|iPhone|iPad/.test(navigator.platform); + +/** Keys whose `code` is not their character, mapped to what a person sees on the cap. */ +const KEY_LABELS: Partial> = { + Comma: ",", + Period: ".", + Slash: "/", + Semicolon: ";", + Backquote: "`", + Escape: "Esc", + Space: "Space", +}; + +/** One key of a chord, as it is printed: "H" for `KeyH`, "," for `Comma`, "?" for `?`. */ +function shortcutKeyLabel(shortcut: ShortcutItem): string { + if (shortcut.key !== undefined) return shortcut.key; + + const code = shortcut.code ?? ""; + if (KEY_LABELS[code] !== undefined) return KEY_LABELS[code]; + if (code.startsWith("Key")) return code.slice(3); + if (code.startsWith("Digit")) return code.slice(5); + + return code; +} + +/** + * The chord as a person reads it — "Alt + H", "⌘ + K", "?". One spelling for the hint on + * a control, the help view and the handler's own definition, so they cannot drift. + */ +export function shortcutChord(shortcut: ShortcutItem): string { + const parts: string[] = []; + + if (shortcut.ctrlOrMeta) parts.push(IS_MAC ? "⌘" : "Ctrl"); + if (shortcut.altKey) parts.push("Alt"); + if (shortcut.shiftKey) parts.push("Shift"); + parts.push(shortcutKeyLabel(shortcut)); + + return parts.join(" + "); +} + +/** + * Whether this keypress is the shortcut — the only key-matching rule in the app. + * + * - Auto-repeat is refused: holding a navigation chord down should walk nowhere. + * - AltGr is refused by refusing Ctrl+Alt. Windows reports AltGr as exactly that pair, + * so a bare `altKey` check fires on every `{`/`[`/`@` a German layout types with it. + * - Text fields own their keys, except where a chord says otherwise (`allowInInput`). + * - Modifiers must match exactly. Shift is the exception for character chords: producing + * `?` *requires* Shift on every layout that has it, so the character is the whole claim. + */ +export function isShortcutPress(event: KeyboardEvent, shortcut: ShortcutItem): boolean { + if (event.repeat) return false; + if (event.altKey && event.ctrlKey) return false; + if (!shortcut.allowInInput && isTypingTarget(event.target)) return false; + + if ((shortcut.ctrlOrMeta === true) !== (event.ctrlKey || event.metaKey)) return false; + if ((shortcut.altKey === true) !== event.altKey) return false; + if (!shortcut.key && (shortcut.shiftKey === true) !== event.shiftKey) return false; + + if (shortcut.code !== undefined) return event.code === shortcut.code; + if (shortcut.key !== undefined) return event.key === shortcut.key; + + return false; +} + +/** + * The chord for the destination a control leads to, or `undefined` for one without a + * shortcut. The sidebar draws its hints through this, so an entry's hint and the global + * listener read the same line of the registry. + */ +export function navigationShortcut(path: AppRoute): string | undefined { + const shortcut = SHORTCUTS.find((item) => item.path === path); + + return shortcut ? shortcutChord(shortcut) : undefined; +} diff --git a/tests/unit/features/shortcuts/shortcuts.test.ts b/tests/unit/features/shortcuts/shortcuts.test.ts new file mode 100644 index 000000000..6cfb2c3de --- /dev/null +++ b/tests/unit/features/shortcuts/shortcuts.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it } from "vitest"; +import { + NEW_CONVERSATION_SHORTCUT, + SHORTCUTS, + SIDEBAR_TOGGLE_SHORTCUT, + SWITCH_PROJECT_SHORTCUT, + isShortcutPress, + navigationShortcut, + shortcutChord, + type ShortcutItem, +} from "../../../../src/features/shortcuts/shortcuts"; + +/** + * A real keydown, dispatched at an element so `event.target` is what a listener would see — + * that is the field half of the typing guard, and a hand-built object would only test the + * function's opinion of its own input. + */ +function pressFrom(target: EventTarget, init: KeyboardEventInit): KeyboardEvent { + const event = new KeyboardEvent("keydown", init); + target.dispatchEvent(event); + return event; +} + +function shortcutById(id: string): ShortcutItem { + const item = SHORTCUTS.find((shortcut) => shortcut.id === id); + if (!item) throw new Error(`No shortcut registered under "${id}"`); + return item; +} + +describe("the registered chords", () => { + it("prints each chord the way a person reads it", () => { + expect(navigationShortcut("/")).toBe("Alt + H"); + expect(navigationShortcut("/board")).toBe("Alt + B"); + expect(navigationShortcut("/chat")).toBe("Alt + C"); + expect(navigationShortcut("/buddy")).toBe("Alt + U"); + expect(navigationShortcut("/knowledge-base")).toBe("Alt + K"); + expect(navigationShortcut("/pm-dashboard")).toBe("Alt + P"); + // The comma is a key, not a word: "Alt + ," is what is printed on the cap. + expect(navigationShortcut("/settings")).toBe("Alt + ,"); + expect(shortcutChord(SIDEBAR_TOGGLE_SHORTCUT)).toBe("Alt + S"); + expect(shortcutChord(NEW_CONVERSATION_SHORTCUT)).toBe("Alt + N"); + }); + + it("prints the help and the platform chord as their own characters", () => { + // "?" and "Esc" are not derivable from a `Key…` code; this pins the label map both ways. + expect(shortcutChord(shortcutById("gen-shortcuts"))).toBe("?"); + expect(shortcutChord(shortcutById("gen-dismiss"))).toBe("Esc"); + // jsdom is not a Mac, so `ctrlOrMeta` renders as Ctrl; the ⌘ spelling is the same branch. + expect(shortcutChord(SWITCH_PROJECT_SHORTCUT)).toBe("Ctrl + K"); + }); + + it("gives every destination a chord and no two entries an id", () => { + const ids = SHORTCUTS.map((shortcut) => shortcut.id); + + expect(new Set(ids).size).toBe(ids.length); + expect(navigationShortcut("/settings")).toBeDefined(); + }); + + it("describes every chord with exactly one of code or key", () => { + // Both set would make the matcher prefer `code` silently; neither would make a dead entry. + for (const shortcut of SHORTCUTS) { + const described = [shortcut.code !== undefined, shortcut.key !== undefined].filter( + Boolean, + ).length; + + expect(described, `${shortcut.id} needs a code or a key`).toBe(1); + } + }); +}); + +describe("isShortcutPress", () => { + const dashboard = shortcutById("nav-home"); + const knowledgeBase = shortcutById("nav-kb"); + const help = shortcutById("gen-shortcuts"); + + it("matches an Alt chord for its own key only", () => { + expect(isShortcutPress(pressFrom(window, { code: "KeyH", altKey: true }), dashboard)).toBe( + true, + ); + expect(isShortcutPress(pressFrom(window, { code: "KeyB", altKey: true }), dashboard)).toBe( + false, + ); + expect(isShortcutPress(pressFrom(window, { code: "KeyH" }), dashboard)).toBe(false); + }); + + it("refuses Ctrl+Alt, which is how Windows reports AltGr", () => { + // A German layout types `@` and `{` with AltGr; those chords must never navigate. + const altGr = pressFrom(window, { code: "KeyH", altKey: true, ctrlKey: true }); + + expect(isShortcutPress(altGr, dashboard)).toBe(false); + }); + + it("refuses a held key repeating and modifiers it was not told about", () => { + expect( + isShortcutPress(pressFrom(window, { code: "KeyH", altKey: true, repeat: true }), dashboard), + ).toBe(false); + expect( + isShortcutPress(pressFrom(window, { code: "KeyH", altKey: true, shiftKey: true }), dashboard), + ).toBe(false); + expect( + isShortcutPress(pressFrom(window, { code: "KeyH", altKey: true, metaKey: true }), dashboard), + ).toBe(false); + }); + + it("keeps apart the two chords that share the K key", () => { + const switcher = pressFrom(window, { code: "KeyK", ctrlKey: true }); + const altK = pressFrom(window, { code: "KeyK", altKey: true }); + + expect(isShortcutPress(switcher, SWITCH_PROJECT_SHORTCUT)).toBe(true); + expect(isShortcutPress(switcher, knowledgeBase)).toBe(false); + expect(isShortcutPress(altK, SWITCH_PROJECT_SHORTCUT)).toBe(false); + expect(isShortcutPress(altK, knowledgeBase)).toBe(true); + }); + + it("accepts Cmd for the chord that declares the platform modifier", () => { + expect( + isShortcutPress(pressFrom(window, { code: "KeyK", metaKey: true }), SWITCH_PROJECT_SHORTCUT), + ).toBe(true); + expect(isShortcutPress(pressFrom(window, { code: "KeyK" }), SWITCH_PROJECT_SHORTCUT)).toBe( + false, + ); + }); + + it("matches the produced character, whatever Shift the layout needed for it", () => { + // US: Shift+/. German QWERTZ: Shift+ß. Neither is `code: "Slash"`, and both are "?". + expect(isShortcutPress(pressFrom(window, { key: "?", shiftKey: true }), help)).toBe(true); + expect(isShortcutPress(pressFrom(window, { key: "?" }), help)).toBe(true); + expect(isShortcutPress(pressFrom(window, { key: "?", ctrlKey: true }), help)).toBe(false); + expect(isShortcutPress(pressFrom(window, { key: "/" }), help)).toBe(false); + }); + + it("leaves a text field's keys to the text field", () => { + const input = document.createElement("input"); + document.body.append(input); + + try { + const altB = { code: "KeyB", altKey: true }; + + expect(isShortcutPress(pressFrom(input, altB), shortcutById("nav-board"))).toBe(false); + expect(isShortcutPress(pressFrom(window, altB), shortcutById("nav-board"))).toBe(true); + expect(isShortcutPress(pressFrom(input, { key: "?", shiftKey: true }), help)).toBe(false); + // …except the one chord that explicitly asks for the composer: a fresh conversation is + // most wanted halfway through typing into the wrong one. + expect( + isShortcutPress( + pressFrom(input, { code: "KeyN", altKey: true }), + NEW_CONVERSATION_SHORTCUT, + ), + ).toBe(true); + } finally { + input.remove(); + } + }); +}); From 14ad236621857a65d81df63f3df02b6db8a7626f Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 14:45:57 +0200 Subject: [PATCH 2/7] feat(shortcuts): answer the chords app-wide and open the help with ? MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GlobalShortcuts` is mounted once in the signed-in shell: one window listener answers every `scope: "global"` entry — the seven destinations and `?` — rather than a listener per page, because a shortcut that only works on the page you are already on is not a shortcut. Destinations go through `canAccessRoute` before navigating: a chord is a convenience, never a way around the access policy, so a hire pressing `Alt+P` is refused like a hire typing the URL. The help view filters the same way, since advertising a shortcut the profile may not use is the same leak in a friendlier font. The help itself is `ui/Modal`, so the focus trap, Escape-to-close, scroll lock and focus restoration are the ones every other dialog already has. The chords render as real text — a help nobody can hear is not the help this dialog exists to be (the hover chip stays `aria-hidden`, this does not) — and the surface-owned chords are listed alongside the global ones, because the dialog is the one place to learn they exist. `?` is ignored while any other dialog is open: two `z-50` overlays would otherwise appear at once and a single Escape would close both. `App` mounts the layer only while signed in — the login screen has no destinations to jump between, and the help would advertise routes a visitor cannot reach. Part of #243. --- src/App.tsx | 7 + .../shortcuts/components/GlobalShortcuts.tsx | 15 ++ .../components/KeyboardShortcutsModal.tsx | 98 +++++++++ .../shortcuts/hooks/useGlobalShortcuts.ts | 71 +++++++ .../shortcuts/hooks/useShortcutListener.ts | 34 +++ src/features/shortcuts/index.ts | 14 ++ .../a11y/KeyboardShortcutsModal.a11y.test.tsx | 86 ++++++++ .../shortcuts/GlobalShortcuts.test.tsx | 198 ++++++++++++++++++ .../shortcuts/KeyboardShortcutsModal.test.tsx | 138 ++++++++++++ 9 files changed, 661 insertions(+) create mode 100644 src/features/shortcuts/components/GlobalShortcuts.tsx create mode 100644 src/features/shortcuts/components/KeyboardShortcutsModal.tsx create mode 100644 src/features/shortcuts/hooks/useGlobalShortcuts.ts create mode 100644 src/features/shortcuts/hooks/useShortcutListener.ts create mode 100644 src/features/shortcuts/index.ts create mode 100644 tests/unit/a11y/KeyboardShortcutsModal.a11y.test.tsx create mode 100644 tests/unit/features/shortcuts/GlobalShortcuts.test.tsx create mode 100644 tests/unit/features/shortcuts/KeyboardShortcutsModal.test.tsx diff --git a/src/App.tsx b/src/App.tsx index c66affee8..7a320285f 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -22,6 +22,7 @@ import { EggEffectsLayer } from "./features/easter-eggs/components/EggEffectsLay import { MyKnowledgeGapsProvider } from "./features/knowledge-gaps/MyKnowledgeGapsProvider"; import { KnowledgeGapOwnerAnnouncement } from "./features/knowledge-gaps/components/KnowledgeGapOwnerAnnouncement"; import { useScrollRestoration } from "./hooks/useScrollRestoration"; +import { GlobalShortcuts } from "./features/shortcuts"; function AppContent() { const { status } = useAuth(); @@ -129,6 +130,12 @@ function AppContent() { where the page already is the buddy. */} {showBuddyDock && } + {/* The keyboard, app-wide: the destination chords and `?` open a listener here rather + than per page, because a shortcut that only works on the page you are already on is + not a shortcut. Signed-in only — the login screen has nothing to jump between, and + the help lists destinations a signed-out visitor has no access to. */} + {signedIn && } + {/* Offers to keep whatever the hire has highlighted, from any page. Mounted here for the same reason the buddy is: what is worth keeping is almost never found on the board. diff --git a/src/features/shortcuts/components/GlobalShortcuts.tsx b/src/features/shortcuts/components/GlobalShortcuts.tsx new file mode 100644 index 000000000..0b580253c --- /dev/null +++ b/src/features/shortcuts/components/GlobalShortcuts.tsx @@ -0,0 +1,15 @@ +import { useGlobalShortcuts } from "../hooks/useGlobalShortcuts"; +import { KeyboardShortcutsModal } from "./KeyboardShortcutsModal"; + +/** + * The shortcuts layer: one window listener for every app-wide chord, and the help it opens. + * + * Mounted once from the signed-in shell — chords are only worth having because they work + * without knowing where you are, which is the opposite of mounting a listener per page. + * Rendering nothing on its own: the help goes through `ui/Modal`'s portal. + */ +export function GlobalShortcuts() { + const { isHelpOpen, closeHelp } = useGlobalShortcuts(); + + return ; +} diff --git a/src/features/shortcuts/components/KeyboardShortcutsModal.tsx b/src/features/shortcuts/components/KeyboardShortcutsModal.tsx new file mode 100644 index 000000000..5eaa39127 --- /dev/null +++ b/src/features/shortcuts/components/KeyboardShortcutsModal.tsx @@ -0,0 +1,98 @@ +import { Fragment } from "react"; +import { Modal } from "../../../components/ui/Modal"; +import { canAccessRoute } from "../../../auth/accessPolicy"; +import { useAuth } from "../../../context/useAuth"; +import { useProjectContext } from "../../projects/useProjectContext"; +import { SHORTCUTS, shortcutChord, type ShortcutCategory } from "../shortcuts"; + +/** Reading order of the help — the registry's own order inside each group. */ +const CATEGORY_ORDER: readonly ShortcutCategory[] = ["Navigation", "Actions", "General"]; + +/** + * One chord as separate key caps, the way every shortcuts dialog draws them. + * + * Readable text rather than `aria-hidden` like the hover hint: this view exists to be + * read, and "Alt + H" read out as a chord is exactly what a screen-reader user came for. + */ +function ChordKeys({ chord }: { chord: string }) { + return ( + + {chord.split(" + ").map((key, index) => ( + + {index > 0 && +} + + + {key} + + + ))} + + ); +} + +type KeyboardShortcutsModalProps = { + isOpen: boolean; + onClose: () => void; +}; + +/** + * The whole keyboard in one dialog, grouped by what the keys are for. + * + * Every row comes out of the one registry the listeners match against + * (`features/shortcuts/shortcuts.ts`), so this view cannot document a chord the app does + * not answer or miss one it does. Destination rows are gated with `canAccessRoute` exactly + * as the listener and the sidebar are: a shortcut a profile may not use is not advertised + * to them. + * + * Opened with `?` from `GlobalShortcuts`; the focus trap, Escape handling and focus + * restoration are `ui/Modal`'s. + */ +export function KeyboardShortcutsModal({ isOpen, onClose }: KeyboardShortcutsModalProps) { + const { profile } = useAuth(); + const { canManageSelected } = useProjectContext(); + + const visibleShortcuts = SHORTCUTS.filter( + (shortcut) => !shortcut.path || canAccessRoute(profile, shortcut.path, canManageSelected), + ); + + return ( + +
+ {CATEGORY_ORDER.map((category) => { + const shortcuts = visibleShortcuts.filter((shortcut) => shortcut.category === category); + if (shortcuts.length === 0) return null; + + return ( +
+

+ {category} +

+ +
    + {shortcuts.map((shortcut) => ( +
  • + + {shortcut.label} + {shortcut.note ? ( + — {shortcut.note} + ) : null} + + + +
    • + ))} +
+
+ ); + })} +
+ + ); +} diff --git a/src/features/shortcuts/hooks/useGlobalShortcuts.ts b/src/features/shortcuts/hooks/useGlobalShortcuts.ts new file mode 100644 index 000000000..59069bc5d --- /dev/null +++ b/src/features/shortcuts/hooks/useGlobalShortcuts.ts @@ -0,0 +1,71 @@ +import { useCallback, useEffect, useState } from "react"; +import { useNavigate } from "react-router-dom"; +import { canAccessRoute } from "../../../auth/accessPolicy"; +import { useAuth } from "../../../context/useAuth"; +import { useProjectContext } from "../../projects/useProjectContext"; +import { SHORTCUTS, isShortcutPress } from "../shortcuts"; + +/** + * Open dialogs own the keyboard while they are up, so the help must not stack on top of + * one: two `z-50` overlays appear at once and a single Escape closes both. `?` is simply + * ignored while anything dialog-shaped is open — including this help itself. + */ +const OPEN_DIALOG_SELECTOR = '[role="dialog"], [role="alertdialog"]'; + +export type GlobalShortcutsState = { + isHelpOpen: boolean; + closeHelp: () => void; +}; + +/** + * The shortcuts layer's own chords: every `scope: "global"` entry — the seven destinations + * and `?`. + * + * Mounted from the signed-in shell, so nothing is bound on the login screen. Destination + * chords go through `canAccessRoute` exactly as the sidebar entries and `ManagerAreaGuard` + * do: a chord is a convenience, never a way around the access policy — a hire pressing + * `Alt+P` is refused like a hire typing the URL. + * + * The `scope: "surface"` items are deliberately not here. Their owners answer them + * (`useShortcutListener`), and nothing about this hook knows the sidebar's drawer exists. + */ +export function useGlobalShortcuts(): GlobalShortcutsState { + const navigate = useNavigate(); + const { profile } = useAuth(); + const { canManageSelected } = useProjectContext(); + const [isHelpOpen, setIsHelpOpen] = useState(false); + + const closeHelp = useCallback(() => setIsHelpOpen(false), []); + + useEffect(() => { + function onKeyDown(event: KeyboardEvent) { + for (const shortcut of SHORTCUTS) { + if (shortcut.scope !== "global") continue; + if (!isShortcutPress(event, shortcut)) continue; + + if (shortcut.path) { + // Spoken for either way: a chord the profile may not use does nothing, rather + // than falling through to whatever else might be listening. + if (!canAccessRoute(profile, shortcut.path, canManageSelected)) return; + + event.preventDefault(); + void navigate(shortcut.path); + return; + } + + if (shortcut.id === "gen-shortcuts") { + if (document.querySelector(OPEN_DIALOG_SELECTOR)) return; + + event.preventDefault(); + setIsHelpOpen(true); + return; + } + } + } + + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }, [canManageSelected, navigate, profile]); + + return { isHelpOpen, closeHelp }; +} diff --git a/src/features/shortcuts/hooks/useShortcutListener.ts b/src/features/shortcuts/hooks/useShortcutListener.ts new file mode 100644 index 000000000..66e82c5cb --- /dev/null +++ b/src/features/shortcuts/hooks/useShortcutListener.ts @@ -0,0 +1,34 @@ +import { useEffect } from "react"; +import { isShortcutPress, type ShortcutItem } from "../shortcuts"; + +/** + * Answers one registered chord outside the shortcuts layer. + * + * The listener every `scope: "surface"` owner uses: the sidebar's drawer, the project + * switcher's dialog, the chat surfaces' new conversation. What counts as a press is the + * registry's rule ({@link isShortcutPress}) — this hook only decides when to listen and + * what a press does. + * + * @param onTrigger Called with the event already `preventDefault`ed. + * @param enabled Gate the listener where the thing it acts on is not on screen — an + * unmounted surface must not answer for a mounted one. + */ +export function useShortcutListener( + shortcut: ShortcutItem, + onTrigger: () => void, + enabled = true, +): void { + useEffect(() => { + if (!enabled) return; + + function onKeyDown(event: KeyboardEvent) { + if (!isShortcutPress(event, shortcut)) return; + + event.preventDefault(); + onTrigger(); + } + + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }, [shortcut, onTrigger, enabled]); +} diff --git a/src/features/shortcuts/index.ts b/src/features/shortcuts/index.ts new file mode 100644 index 000000000..f809920a8 --- /dev/null +++ b/src/features/shortcuts/index.ts @@ -0,0 +1,14 @@ +export { GlobalShortcuts } from "./components/GlobalShortcuts"; +export { KeyboardShortcutsModal } from "./components/KeyboardShortcutsModal"; +export { useGlobalShortcuts } from "./hooks/useGlobalShortcuts"; +export { useShortcutListener } from "./hooks/useShortcutListener"; +export { + NEW_CONVERSATION_SHORTCUT, + SHORTCUTS, + SIDEBAR_TOGGLE_SHORTCUT, + SWITCH_PROJECT_SHORTCUT, + isShortcutPress, + navigationShortcut, + shortcutChord, +} from "./shortcuts"; +export type { ShortcutCategory, ShortcutItem, ShortcutScope } from "./shortcuts"; diff --git a/tests/unit/a11y/KeyboardShortcutsModal.a11y.test.tsx b/tests/unit/a11y/KeyboardShortcutsModal.a11y.test.tsx new file mode 100644 index 000000000..0457ba424 --- /dev/null +++ b/tests/unit/a11y/KeyboardShortcutsModal.a11y.test.tsx @@ -0,0 +1,86 @@ +import { useState } from "react"; +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi } from "vitest"; +import { axe } from "vitest-axe"; +import { KeyboardShortcutsModal } from "../../../src/features/shortcuts"; + +vi.mock("../../../src/context/useAuth", () => ({ + useAuth: () => ({ + profile: { + id: "user123", + username: "Test User", + email: "test@example.com", + permissionGroup: "ADMIN", + projectRoles: [], + profileIcon: "Test", + }, + logout: vi.fn(), + status: "authenticated", + }), +})); + +vi.mock("../../../src/features/projects/useProjectContext", () => ({ + useProjectContext: () => ({ + projects: [], + selectedProject: null, + selectedProjectId: "", + canManageSelected: false, + isSwitcherEnabled: true, + isLoading: false, + errorMessage: null, + setSelectedProjectId: vi.fn(), + reloadProjects: vi.fn(), + }), +})); + +function ShortcutsHarness() { + const [isOpen, setIsOpen] = useState(false); + + return ( +
+ + setIsOpen(false)} /> +
+ ); +} + +describe("Keyboard shortcuts help accessibility", () => { + it("has no axe violations, keeps focus inside and gives it back on Escape", async () => { + const user = userEvent.setup(); + const { baseElement } = render(); + + const openButton = screen.getByRole("button", { name: "Show shortcuts" }); + await user.click(openButton); + + const dialog = screen.getByRole("dialog", { name: "Keyboard shortcuts" }); + expect(dialog).toHaveAttribute("aria-modal", "true"); + // The chord list must be readable text: a help nobody can hear is not the help this + // dialog exists to be (the hover hint is the one that stays `aria-hidden`). + expect(within(dialog).getAllByText("Alt").length).toBeGreaterThan(0); + + expect(await axe(baseElement)).toHaveNoViolations(); + + await user.keyboard("{Escape}"); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(openButton).toHaveFocus(); + }); + + it("reaches the chord list for a keyboard-only reader", async () => { + const user = userEvent.setup(); + render(); + + await user.tab(); + await user.keyboard("{Enter}"); + + const dialog = screen.getByRole("dialog", { name: "Keyboard shortcuts" }); + const closeButton = within(dialog).getByRole("button", { name: "Close dialog" }); + await waitFor(() => expect(closeButton).toHaveFocus()); + + expect( + within(dialog).getByRole("heading", { level: 3, name: "Navigation" }), + ).toBeInTheDocument(); + }); +}); diff --git a/tests/unit/features/shortcuts/GlobalShortcuts.test.tsx b/tests/unit/features/shortcuts/GlobalShortcuts.test.tsx new file mode 100644 index 000000000..41a869b3b --- /dev/null +++ b/tests/unit/features/shortcuts/GlobalShortcuts.test.tsx @@ -0,0 +1,198 @@ +import type { ReactNode } from "react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { MemoryRouter, useLocation } from "react-router-dom"; +import { GlobalShortcuts } from "../../../../src/features/shortcuts"; +import * as useAuthHook from "../../../../src/context/useAuth"; +import { PermissionGroup, type UserProfile } from "../../../../src/services/types"; + +const { projectState } = vi.hoisted(() => ({ projectState: { canManageSelected: false } })); + +vi.mock("../../../../src/features/projects/useProjectContext", async () => { + const { createProjectContextValue } = await import("../../setup/projectContext"); + return { + useProjectContext: () => + createProjectContextValue({ canManageSelected: projectState.canManageSelected }), + }; +}); + +vi.mock("../../../../src/context/useAuth", () => ({ useAuth: vi.fn() })); + +const hireProfile: UserProfile = { + id: "1", + authId: "auth", + username: "Hire", + email: "hire@example.com", + firstName: "Test", + lastName: "Hire", + projectRoles: [], + projectIds: [], + permissionGroup: PermissionGroup.USER, + enabled: true, + profileIcon: null, + hasCompletedOnboarding: true, +}; + +function signIn(profile: UserProfile | null) { + vi.mocked(useAuthHook.useAuth).mockReturnValue({ + status: profile ? "authenticated" : "unauthenticated", + profile, + login: vi.fn(), + logout: vi.fn(), + refetchProfile: vi.fn(), + }); +} + +function LocationProbe() { + const { pathname } = useLocation(); + + return {pathname}; +} + +function Harness({ + children, + extraDialog = false, +}: { + children?: ReactNode; + extraDialog?: boolean; +}) { + return ( + + + + {extraDialog &&
} + {children} + + ); +} + +/** Where the router currently is — the observable a navigation chord must move. */ +function pathname(): string { + return screen.getByTestId("pathname").textContent ?? ""; +} + +describe("GlobalShortcuts", () => { + beforeEach(() => { + vi.clearAllMocks(); + projectState.canManageSelected = false; + signIn(hireProfile); + }); + + it("navigates to every destination the registry names", async () => { + const user = userEvent.setup(); + render(); + + const destinations: ReadonlyArray = [ + ["{Alt>}h{/Alt}", "/"], + ["{Alt>}b{/Alt}", "/board"], + ["{Alt>}c{/Alt}", "/chat"], + ["{Alt>}u{/Alt}", "/buddy"], + ["{Alt>}k{/Alt}", "/knowledge-base"], + // Character-matched, and typed as a character: the comma is what somebody presses, + // whatever their layout does with Shift to produce it. + ["{Alt>},{/Alt}", "/settings"], + ]; + + for (const [keys, path] of destinations) { + await user.keyboard(keys); + + expect(pathname()).toBe(path); + } + }); + + it("refuses the PM dashboard to a hire", async () => { + const user = userEvent.setup(); + render(); + + await user.keyboard("{Alt>}p{/Alt}"); + + expect(pathname()).toBe("/board"); + }); + + it("lets a PM managing the selected project through", async () => { + projectState.canManageSelected = true; + signIn({ ...hireProfile, permissionGroup: PermissionGroup.PM }); + const user = userEvent.setup(); + render(); + + await user.keyboard("{Alt>}p{/Alt}"); + + expect(pathname()).toBe("/pm-dashboard"); + }); + + it("does nothing while a text field has the keys", async () => { + const user = userEvent.setup(); + render( + + + , + ); + + await user.click(screen.getByRole("textbox", { name: "Message" })); + await user.keyboard("{Alt>}k{/Alt}"); + + expect(pathname()).toBe("/board"); + }); + + it("refuses Ctrl+Alt, the pair Windows reports for AltGr", () => { + render(); + + fireEvent.keyDown(window, { code: "KeyB", altKey: true, ctrlKey: true }); + + expect(pathname()).toBe("/board"); + }); + + it("ignores a held chord repeating", () => { + render(); + + fireEvent.keyDown(window, { code: "KeyB", altKey: true, repeat: true }); + + expect(pathname()).toBe("/board"); + }); + + it("does nothing at all without a profile", async () => { + signIn(null); + const user = userEvent.setup(); + render(); + + await user.keyboard("{Alt>}h{/Alt}"); + + expect(pathname()).toBe("/board"); + }); + + it("opens the help on ? and closes it again on Escape", async () => { + const user = userEvent.setup(); + render(); + + await user.keyboard("?"); + expect(await screen.findByRole("dialog", { name: "Keyboard shortcuts" })).toBeInTheDocument(); + + await user.keyboard("{Escape}"); + await waitFor(() => + expect(screen.queryByRole("dialog", { name: "Keyboard shortcuts" })).not.toBeInTheDocument(), + ); + }); + + it("keeps the help single while it is already open", async () => { + const user = userEvent.setup(); + render(); + + await user.keyboard("?"); + await screen.findByRole("dialog", { name: "Keyboard shortcuts" }); + await user.keyboard("?"); + + expect(screen.getAllByRole("dialog")).toHaveLength(1); + }); + + it("does not stack the help on top of another dialog", async () => { + const user = userEvent.setup(); + render(); + + await user.keyboard("?"); + + expect(screen.queryByRole("dialog", { name: "Keyboard shortcuts" })).not.toBeInTheDocument(); + // Only the stand-in is on screen — one Escape, one dialog, as it should stay. + expect(screen.getAllByRole("dialog")).toHaveLength(1); + }); +}); diff --git a/tests/unit/features/shortcuts/KeyboardShortcutsModal.test.tsx b/tests/unit/features/shortcuts/KeyboardShortcutsModal.test.tsx new file mode 100644 index 000000000..f58283f9f --- /dev/null +++ b/tests/unit/features/shortcuts/KeyboardShortcutsModal.test.tsx @@ -0,0 +1,138 @@ +import { render, screen, within } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { KeyboardShortcutsModal } from "../../../../src/features/shortcuts"; +import * as useAuthHook from "../../../../src/context/useAuth"; +import { PermissionGroup, type UserProfile } from "../../../../src/services/types"; + +const { projectState } = vi.hoisted(() => ({ projectState: { canManageSelected: false } })); + +vi.mock("../../../../src/features/projects/useProjectContext", async () => { + const { createProjectContextValue } = await import("../../setup/projectContext"); + return { + useProjectContext: () => + createProjectContextValue({ canManageSelected: projectState.canManageSelected }), + }; +}); + +vi.mock("../../../../src/context/useAuth", () => ({ useAuth: vi.fn() })); + +const hireProfile: UserProfile = { + id: "1", + authId: "auth", + username: "Hire", + email: "hire@example.com", + firstName: "Test", + lastName: "Hire", + projectRoles: [], + projectIds: [], + permissionGroup: PermissionGroup.USER, + enabled: true, + profileIcon: null, + hasCompletedOnboarding: true, +}; + +function signIn(profile: UserProfile | null) { + vi.mocked(useAuthHook.useAuth).mockReturnValue({ + status: profile ? "authenticated" : "unauthenticated", + profile, + login: vi.fn(), + logout: vi.fn(), + refetchProfile: vi.fn(), + }); +} + +function renderModal(isOpen = true) { + return render(); +} + +/** + * The list row a label belongs to — the chord's own unit. Queried by role rather than text + * because "Keyboard shortcuts" is both a row and the dialog's own heading. + */ +function rowFor(label: string): HTMLElement { + const row = screen.getAllByRole("listitem").find((item) => item.textContent?.startsWith(label)); + + if (!row) throw new Error(`No row for "${label}"`); + return row; +} + +describe("KeyboardShortcutsModal", () => { + beforeEach(() => { + vi.clearAllMocks(); + projectState.canManageSelected = false; + signIn(hireProfile); + }); + + it("renders nothing while it is closed", () => { + renderModal(false); + + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + it("groups the chords under the three headings", () => { + renderModal(); + + expect(screen.getByRole("heading", { level: 3, name: "Navigation" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { level: 3, name: "Actions" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { level: 3, name: "General" })).toBeInTheDocument(); + }); + + it("lists every destination a hire may reach, and none of the others", () => { + renderModal(); + + for (const label of ["Dashboard", "Board", "Chat", "Buddy", "Knowledge Base", "Settings"]) { + expect(screen.getByText(label)).toBeInTheDocument(); + } + + expect(screen.queryByText("PM Dashboard")).not.toBeInTheDocument(); + }); + + it("advertises the PM dashboard to a PM managing the selected project", () => { + projectState.canManageSelected = true; + signIn({ ...hireProfile, permissionGroup: PermissionGroup.PM }); + + renderModal(); + + expect(screen.getByText("PM Dashboard")).toBeInTheDocument(); + }); + + it("prints a chord one key cap at a time", () => { + renderModal(); + + const dashboard = within(rowFor("Dashboard")); + expect(dashboard.getByText("Alt")).toBeInTheDocument(); + expect(dashboard.getByText("H")).toBeInTheDocument(); + + // The one chord with a platform split, so it must not be hardcoded to Alt anywhere. + const switcher = within(rowFor("Switch project")); + expect(switcher.getByText("Ctrl")).toBeInTheDocument(); + expect(switcher.getByText("K")).toBeInTheDocument(); + + // Characters stay whole: splitting "?" or "Esc" on " + " must not cut them apart. + expect(within(rowFor("Keyboard shortcuts")).getByText("?")).toBeInTheDocument(); + expect(within(rowFor("Close a dialog or menu")).getByText("Esc")).toBeInTheDocument(); + expect(within(rowFor("Settings")).getByText(",")).toBeInTheDocument(); + }); + + it("says where a chord only works somewhere specific", () => { + renderModal(); + + expect(within(rowFor("Jump to the message box")).getByText(/in Chat/)).toBeInTheDocument(); + expect(within(rowFor("Toggle sidebar")).getByText(/small screens/)).toBeInTheDocument(); + }); + + it("documents the chords the surfaces answer, not only the global ones", () => { + renderModal(); + + // These belong to the switcher, the chat surfaces, the sidebar and the dialogs — their + // owners listen, but the help is the one place a person can learn they exist. + for (const label of [ + "Switch project", + "New conversation", + "Toggle sidebar", + "Close a dialog or menu", + ]) { + expect(screen.getByText(label)).toBeInTheDocument(); + } + }); +}); From cfda1cb5974c550274ce21e9a6ef0f571e200493 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 14:45:57 +0200 Subject: [PATCH 3/7] feat(shortcuts): hint the sidebar and unify the existing chord listeners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `SidebarNavLink` takes a `shortcut` and draws `ui/ShortcutHint` next to the label. The entries get their chord from `navigationShortcut(path)` rather than strings typed at the call site, so an entry's hint and the keypress that answers it read the same line of the registry. The chip stays `aria-hidden` and the link's own `title` carries the copy a screen reader hears — once. The icon-only Settings button has no room for a chip, so its title carries `Alt + ,`, the arrangement the project switcher's trigger already uses. - `Alt+S` works the drawer the mobile header's button works — one toggle for both ways in. There is no desktop collapse to reach yet (#244), and inventing a second behaviour for wide screens now would only have to be undone there. - `ProjectSwitcher` and `useNewConversationShortcut` lose their bespoke listeners: the chord and what counts as a press come from the registry through `useShortcutListener`. The switcher's old synthetic-event guard becomes the matcher's own refusal of events without a `code` (a real KeyboardEvent always has one; `new Event("keydown")` from a browser extension never will). Each component keeps its own gate — `isSwitcherEnabled`, the assistant's `enabled` — because those say what is on screen, which is knowledge the registry does not have. Closes #243. --- src/components/layout/SideBar.tsx | 27 ++++- src/components/layout/SidebarNavLink.tsx | 16 +++ .../projects/components/ProjectSwitcher.tsx | 39 +++---- src/hooks/useNewConversationShortcut.ts | 37 +++--- tests/unit/components/layout/SideBar.test.tsx | 109 ++++++++++++++++++ .../components/layout/SidebarNavLink.test.tsx | 31 +++++ .../projects/ProjectSwitcher.test.tsx | 4 +- 7 files changed, 211 insertions(+), 52 deletions(-) diff --git a/src/components/layout/SideBar.tsx b/src/components/layout/SideBar.tsx index b26492a31..f5207dbc7 100644 --- a/src/components/layout/SideBar.tsx +++ b/src/components/layout/SideBar.tsx @@ -1,4 +1,4 @@ -import { useId, useState } from "react"; +import { useCallback, useId, useState } from "react"; import { motion, useMotionValue } from "framer-motion"; import { NavLink, useLocation } from "react-router-dom"; import { LogOut, Menu, Settings, X } from "lucide-react"; @@ -13,6 +13,11 @@ import { useMyKnowledgeGaps } from "../../features/knowledge-gaps/useMyKnowledge import { usePmAttentionFlag } from "../../features/team-management/usePmAttentionFlag"; import { useOpenEscalationCount } from "../../features/knowledge-request/useOpenEscalationCount"; import { useUnseenSkipAnswerCount } from "../../features/onboarding/hooks/useUnseenSkipAnswerCount"; +import { + SIDEBAR_TOGGLE_SHORTCUT, + navigationShortcut, + useShortcutListener, +} from "../../features/shortcuts"; import { AdminIcon, BlueprintsIcon, @@ -206,6 +211,10 @@ function SidebarContent({ canAccessRoute(profile, item.path, canManageSelected), ); + // The icon-only footer button has no room for a chip, so its `title` carries the chord -- + // the same arrangement the project switcher's trigger uses. + const settingsShortcut = navigationShortcut("/settings"); + // `/insights/knowledge-requests` is deliberately absent: it has its own // sidebar entry, so listing it here would leave two entries active at once // -- including two active pills sharing one Framer Motion `layoutId`. @@ -306,6 +315,7 @@ function SidebarContent({ (item.path === "/pm-dashboard" && isPmSectionActive) || (item.path === "/chat" && isAssistantSectionActive) } + shortcut={navigationShortcut(item.path)} indicatorLayoutId={indicatorLayoutId} pointerY={pointerY} hasAttentionMarker={ @@ -377,7 +387,7 @@ function SidebarContent({ : "text-app-text-muted hover:bg-app-surface-hover hover:text-app-text" }` } - title="Settings" + title={settingsShortcut ? `Settings (${settingsShortcut})` : "Settings"} aria-label="Settings" > @@ -456,6 +466,17 @@ export function SideBar() { setIsMobileSidebarOpen(false); }; + /** + * Alt+S works whatever the header's button works — one definition of "toggle the sidebar" + * for both ways in. Today that is the drawer below `lg`; the desktop collapse (#244) will + * give the same chord something to do on a wide screen without this line changing. + */ + const toggleMobileSidebar = useCallback(() => { + setIsMobileSidebarOpen((isOpen) => !isOpen); + }, []); + + useShortcutListener(SIDEBAR_TOGGLE_SHORTCUT, toggleMobileSidebar); + return ( <>