From 54935715b9c2872620905f7d7202e388065356c5 Mon Sep 17 00:00:00 2001 From: Daniels-Main Date: Mon, 15 Jun 2026 23:45:21 +0200 Subject: [PATCH] feat(keyboard): configurable shortcuts + keyboard-first nav fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the app fully keyboard-driven with rebindable global shortcuts, and fix file-tree keyboard navigation to match the tree's display order. - Central keybinding registry (ui/src/lib/keys.ts): single source of truth for every global command, with event→binding, override resolution, platform-aware formatting, and muda-accelerator conversion (keys.test.ts). - Push = Mod+P, Pull = Mod+Shift+P (per request), plus fetch / sync / open-editor / open-terminal / refresh defaults. - App.tsx window keydown is registry-driven; native menu accelerators and palette shortcut chips resolve through the same map and reinstall on change. - Settings → Keyboard section: record / unassign / reset / restore-all with shared-binding warnings; overrides persist in settings.keybindings. - Shift+J / Shift+K scroll the diff pane (Review + Local Changes). - j/k walk files in the tree's display order (folders-first natural sort via ui/src/lib/treeOrder.ts) instead of flat path order, so they no longer "dive into folders" — matches the arrow keys. Applied to the Review queue and Local Changes (treeOrder.test.ts). - Docs: README shortcuts table + context keys, ROADMAP, TASKS, learnings. Verified: pnpm exec tsc --noEmit (clean), vitest 131 passing. Co-Authored-By: Claude Opus 4.8 --- README.md | 30 +++ ROADMAP.md | 13 ++ TASKS.md | 13 +- docs/learnings.md | 90 +++++++++ ui/src/App.tsx | 166 +++++++++++----- ui/src/lib/keys.test.ts | 128 ++++++++++++ ui/src/lib/keys.ts | 232 ++++++++++++++++++++++ ui/src/lib/menu.ts | 49 +++-- ui/src/lib/treeOrder.test.ts | 62 ++++++ ui/src/lib/treeOrder.ts | 101 ++++++++++ ui/src/stores/settings.ts | 22 ++ ui/src/styles/features.css | 72 +++++++ ui/src/views/LocalChanges.tsx | 27 ++- ui/src/views/Review.tsx | 23 ++- ui/src/views/SettingsDialog.tsx | 5 +- ui/src/views/settings/KeyboardSection.tsx | 184 +++++++++++++++++ 16 files changed, 1136 insertions(+), 81 deletions(-) create mode 100644 ui/src/lib/keys.test.ts create mode 100644 ui/src/lib/keys.ts create mode 100644 ui/src/lib/treeOrder.test.ts create mode 100644 ui/src/lib/treeOrder.ts create mode 100644 ui/src/views/settings/KeyboardSection.tsx diff --git a/README.md b/README.md index 75cf74d..7e6cb46 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,36 @@ **[Roadmap](./ROADMAP.md)** · **[Commercial license](./COMMERCIAL.md)** +Keyboard-first: almost every action is operable from the keyboard alone — +never keyboard-only, the mouse stays first-class. Shortcuts are listed below +and **every global one is rebindable** in Settings → Keyboard. + +### Keyboard shortcuts + +`Mod` is ⌘ on macOS, Ctrl elsewhere. All of the following are configurable in +Settings → Keyboard (also reachable via the palette: "Settings: Keyboard +shortcuts"). + +| Shortcut | Action | +| --- | --- | +| `Mod+K` | Command palette | +| `Mod+O` | Open repository | +| `Mod+,` | Settings | +| `Mod+1…5` | Local Changes · All Commits · Reflog · Review · Worktrees | +| `Mod+P` | Push | +| `Mod+Shift+P` | Pull | +| `Mod+Shift+Y` | Fetch | +| `Mod+Shift+S` | Sync (fetch + pull + push) | +| `Mod+Shift+E` | Open in editor | +| `Mod+Shift+C` | Open in terminal | +| `Mod+R` | Refresh | +| `Mod+Shift+T` | Toggle light/dark theme | + +Surface-local keys (not rebindable, documented in Settings → Keyboard): +`Mod+Enter` commit · `Mod+F` search in diff · `/` search commits · `j`/`k` +walk the file list · `n`/`p` step change blocks · `Shift+J`/`Shift+K` scroll the +diff · palette `↑↓`/`↵`/`⇥`/`Esc`. + Strand is a native, cross-platform Git client (Tauri 2 + Rust + React) with a dedicated surface for reviewing an agent's changes: whole-file-context diffs, a review queue, and worktree-aware baselines that include what the diff --git a/ROADMAP.md b/ROADMAP.md index 86b4fbb..9bbe16a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -971,6 +971,19 @@ from the Keychain and repackaged Developer-ID-only), and download flakes (`8a53544`). Auto-update remains endpoint-blocked: the manifest ships, the host doesn't. +**Configurable keyboard shortcuts (2026-06-15):** Global shortcuts moved to a +single registry (`ui/src/lib/keys.ts`) resolved against persisted user overrides +(`settings.keybindings`); the window keydown handler, native menu accelerators, +command-palette chips, and the new **Settings → Keyboard** section all read the +same map, so a remap propagates everywhere and the palette/menu hints stay +platform-correct (⌘ vs Ctrl). Push is `Mod+P` and pull `Mod+Shift+P` per request, +plus fetch / sync / open-in-editor / open-in-terminal / refresh defaults. The +Keyboard section records a combo by listening in the capture phase, with +unassign / reset / restore-all and shared-binding warnings; surface-local keys +(commit, in-diff search, commit search, Review j/k) stay with their views and are +documented there. Verified with `tsc`, `vitest` (123 pass, +18 `keys.test.ts`). +See `docs/learnings.md` "Global shortcuts live in one registry." + --- ## 1.1+ — Post-1.0 diff --git a/TASKS.md b/TASKS.md index 169be89..fccbe74 100644 --- a/TASKS.md +++ b/TASKS.md @@ -676,10 +676,21 @@ Legend: ☐ not started · ◐ in progress · ☑ done · ✗ blocked - ☑ Extension point for future custom themes — `THEME_OPTIONS` registry + `[data-theme]` token blocks; adding high-contrast / solarized is add-a-block + add-an-entry, no other code changes. -- ☐ **Keyboard operability pass.** Almost every action reachable from the +- ◐ **Keyboard operability pass.** Almost every action reachable from the keyboard, not just the palette (PRD §6.7, `docs/learnings.md`). Per-surface focus models + palette entries; audit for mouse-only actions. Drag-and-drop (folder open, tab / file reorder) may stay pointer-only. + - ☑ Configurable global-shortcut registry (`ui/src/lib/keys.ts` `COMMANDS` + + `resolveBindings`/`eventToBinding`/`formatBinding`/`toMudaAccelerator`, + tested in `keys.test.ts`). Window keydown (`App.tsx`), native menu + (`lib/menu.ts`), palette chips, and Settings all resolve through it. + - ☑ Push = `Mod+P`, Pull = `Mod+Shift+P` (+ Fetch `Mod+Shift+Y`, Sync + `Mod+Shift+S`, open-editor/terminal, refresh `Mod+R`). + - ☑ Settings → Keyboard section: rebind (record-a-combo) / unassign / reset / + restore-all, shared-binding warnings, context-shortcut reference + (`views/settings/KeyboardSection.tsx`). Persisted as `settings.keybindings`. + - ☐ Make surface-local keys (commit, in-diff search, commit search, Review + j/k) rebindable too — currently fixed + documented in the Keyboard section. - ☐ Status-bar: real GPG / LFS / sync state - ☐ Toast system → proper notification component - ☐ Empty-state copy for every panel (PRD §9: "no 'no data' labels") diff --git a/docs/learnings.md b/docs/learnings.md index 07c62e5..f1d4189 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -753,6 +753,19 @@ items take `enabled: hasRepo`; App reinstalls the menu when that flips (menu handlers read the latest callbacks through a ref, so no rebuild per render). +**Update (2026-06-15): accelerators come from the keybinding registry, not +literals.** Global shortcuts now live in a single registry — `lib/keys.ts`, +`COMMANDS` — resolved against the user's overrides (`settings.keybindings`) by +`resolveBindings`. `lib/menu.ts` no longer hardcodes `accelerator:` strings; its +`item()` helper takes a `cmd: CommandId` and looks the accelerator up through an +`accel` resolver App passes in (`toMudaAccelerator(resolved)`), so a shortcut the +user remaps in Settings → Keyboard updates the menu too — App's menu effect now +deps on `keyMap` and reinstalls on change. The window keydown handler is also +registry-driven: it computes `eventToBinding(e)`, looks up the command, and still +defers to the native menu for menu-owned, representable combos +(`appMenuInstalled() && MENU_COMMANDS.has(cmd) && toMudaAccelerator(binding)`). +See the keybinding-registry learning below. + --- ## Pierre tree rows only repaint on data pushes — decoration changes need a key bump @@ -861,3 +874,80 @@ review-feedback palette actions are the canonical sites. (`useRepo(s => s.xs.length)`), spread the action conditionally, and inside `run` call `useRepo.getState().xs`. If the action needs multiple slices, read them all at run time rather than widening the subscription. + +--- + +## Global shortcuts live in one registry; context shortcuts stay with their views + +**Rule.** Every *global* app shortcut is declared once in `ui/src/lib/keys.ts` +(`COMMANDS`: id, label, category, `defaultBinding`, `menu`, `needsRepo`). Bindings +use a canonical `Mod+Alt+Shift+` string where `Mod` = ⌘/Ctrl. Three consumers +resolve through the same module so they can never drift: + +- **Window keydown** (`App.tsx`): `eventToBinding(e)` → `resolveBindings(overrides) + .byBinding` → command id → a handler from the per-render `commandHandlers` map + (read via ref so settings changes don't re-subscribe the listener). +- **Native menu** (`lib/menu.ts`): accelerators via `toMudaAccelerator` (see the + menu-ownership learning's 2026-06-15 update). +- **Palette + Settings chips**: `formatBinding(binding, platform)` (⌘⇧P on mac, + Ctrl+Shift+P elsewhere). + +User overrides persist in `settings.keybindings` (`KeyOverrides`: id → binding, or +`null` to unbind, or **absent** = default). `setKeybinding(id, undefined)` resets one +row; `resetKeybindings()` clears all. Settings → Keyboard (`KeyboardSection.tsx`) +records a combo by listening in the **capture** phase (fires before App's handler; +`stopPropagation` keeps the in-progress chord from triggering an app command or the +dialog's Esc), and flags clashes via `conflictingCommands`. + +**Why.** The push/pull request (⌘P / ⌘⇧P) plus "make shortcuts configurable" forced +a single source of truth — the old hardcoded keydown chain + literal menu +accelerators couldn't be remapped or kept in sync. Keeping the pure logic in `lib/` +(not the view) follows the testable-logic learning; `keys.test.ts` covers +event→binding folding, override resolution, conflict detection, and formatting. + +**Scope / how to apply.** Only *global* commands are in the registry. Surface-local +keys that depend on what's focused — commit `Mod+Enter` (LocalChanges), in-diff +search `Mod+F` (LocalChanges), commit search `/` (Commits), Review `j`/`k` — stay in +their own components and are documented (not rebindable) in the Keyboard section's +"Context shortcuts" card. To add a global command: add a `COMMANDS` row, a handler in +App's `commandHandlers`, and (if it should sit on the macOS menu) a `cmd:` on the +menu item + `menu: true`. Plain (modifier-less) bindings are suppressed while a text +field/combobox is focused (`isPlainKey`); repo-scoped commands no-op without a repo +(`REPO_COMMANDS`). `Mod+R` refresh calls `preventDefault`, so it doesn't reload the +webview in dev. + +--- + +## Tree keyboard nav must use Pierre's display order, not flat path order + +**Rule.** When walking a `@pierre/trees` file list by keyboard (the Review +queue's `j`/`k`), step through the paths in the tree's **visible display order**, +not the diff list's flat full-path sort. Use `treeFileOrder(paths)` / +`compareTreePaths` (`ui/src/lib/treeOrder.ts`). + +**Why.** Pierre sorts **directories before files at each path level**, then by a +**case-insensitive natural** comparison (`a2` < `a10`). A flat string sort of +full paths interleaves nested files with their siblings differently — e.g. +`[src/app.ts, src/lib/keys.ts, src/lib/menu.ts, src/zebra.ts]` flat vs. +`[src/lib/keys.ts, src/lib/menu.ts, src/app.ts, src/zebra.ts]` in the tree. `j` +over the flat order looked like it was "diving into folders" because the next +flat entry was a nested file the user hadn't visually reached. The arrows were +always correct because Pierre's own keydown (`focusNextItem` in +`render/FileTreeView.js`) moves through the rendered rows. + +**Why we re-implement the comparator.** The public `model` from `useFileTree` is +the render `FileTree` handle, which exposes `focusPath`/`getFocusedPath` but +**not** `focusNextItem`/`getVisibleRows` (those are on the internal +`FileTreeController`). And the tree renders into an **open shadow root with its +own React root**, so synthesizing arrow keydowns to drive Pierre's handler is +fragile. `treeOrder.ts` is a faithful port of Pierre's +`path-store/src/sort.js` `comparePreparedPaths`, unit-tested +(`treeOrder.test.ts`) so the two stay in agreement. It orders *files* only — +folder flattening and collapsed-folder visibility don't change the relative +order of files, so they're ignored (consistent with the old behavior, which +also ignored collapse). + +**How to apply.** Any new tree keyboard walk sorts its candidate paths through +`treeFileOrder` first, then indexes by the active path. Both the Review queue +(`Review.tsx` `navOrder`) and Local Changes' `j`/`k` (`LocalChanges.tsx` `nav`, +each of the unstaged + staged groups sorted independently) route through it. diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 0ea2e46..b862b7c 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -18,6 +18,16 @@ import { editorTemplate, osType, terminalTemplate } from './lib/integrations'; import { concatPatches, patchesToMarkdown } from './lib/patchExport'; import { buildReviewFeedback, collectFeedbackFiles } from './lib/reviewExport'; import { appMenuInstalled, installAppMenu, type MenuHandlers } from './lib/menu'; +import { + eventToBinding, + formatBinding, + isPlainKey, + MENU_COMMANDS, + REPO_COMMANDS, + resolveBindings, + toMudaAccelerator, + type CommandId, +} from './lib/keys'; import { errMessage, isCancelled, isTauri, tauri } from './lib/tauri'; import { useTheme } from './lib/theme'; import { CloneDialog } from './views/CloneDialog'; @@ -102,6 +112,7 @@ export function App() { const monoFont = useSettings((s) => s.monoFont); const diffFont = useSettings((s) => s.diffFont); const accent = useSettings((s) => s.accent); + const keybindings = useSettings((s) => s.keybindings); // Theme preference → resolved theme; `useTheme` applies `data-theme` on // , subscribes to the OS, and exposes setters for the picker/palette. const { resolved: theme, setPref: setTheme, cycle: cycleTheme } = useTheme(); @@ -446,6 +457,55 @@ export function App() { } }, [pushRepo, onNetProgress, showToast, flashDone, pushing, nextOpId]); + // Keyboard refresh — same snapshot-based refresh the header button runs + // (meta/refs/tree/submodules ride along with status), guarded on an open repo. + const onRefresh = useCallback(() => { + if (!useRepo.getState().activePath) return; + void refreshLocalChanges(); + void refreshLog(); + }, [refreshLocalChanges, refreshLog]); + + // Resolved keybindings: defaults from the registry overlaid with the user's + // overrides. Drives the window keydown handler, the palette shortcut chips, + // and the native-menu accelerators. + const keyMap = useMemo(() => resolveBindings(keybindings), [keybindings]); + /** Formatted binding for a command (e.g. "⌘P" / "Ctrl+P"), or undefined. */ + const keyHint = useCallback( + (id: CommandId) => formatBinding(keyMap.byCommand.get(id) ?? null, platform) || undefined, + [keyMap, platform], + ); + + // One handler per global command. Rebuilt each render so it closes over the + // latest callbacks; the keydown effect reads it through a ref (below) so it + // never has to re-subscribe. + const commandHandlers = useMemo void>>(() => ({ + 'palette': () => setPaletteOpen((o) => !o), + 'open-repo': () => { void openViaDialog(); }, + 'clone-repo': () => setCloneOpen(true), + 'settings': () => openSettingsAt('appearance'), + 'view-local': () => { setView('local'); selectFile(null); }, + 'view-commits': () => { setView('commits'); selectFile(null); }, + 'view-reflog': () => { setView('reflog'); selectFile(null); }, + 'view-review': () => { setView('review'); selectFile(null); }, + 'view-worktrees': () => { setView('worktrees'); selectFile(null); }, + 'theme-toggle': () => { + const next = cycleTheme(); + showToast(`Theme: ${next[0].toUpperCase()}${next.slice(1)}`); + }, + 'fetch': () => { void onSync(); }, + 'pull': () => { void onPull(); }, + 'push': () => { void onPush(); }, + 'sync': () => { void onSync(); }, + 'open-editor': openInEditor, + 'open-terminal': openInTerminal, + 'refresh': onRefresh, + }), [openViaDialog, openSettingsAt, setView, selectFile, cycleTheme, showToast, + onSync, onPull, onPush, openInEditor, openInTerminal, onRefresh]); + const commandHandlersRef = useRef(commandHandlers); + commandHandlersRef.current = commandHandlers; + const keyMapRef = useRef(keyMap); + keyMapRef.current = keyMap; + // Load recents + restore the tabs the user had open last time. Both run // once on first mount; restoreSession is idempotent so StrictMode's // double-invoke is harmless. @@ -481,9 +541,13 @@ export function App() { const hasRepo = Boolean(meta); useEffect(() => { if (!isTauri() || osType() !== 'macos') return; - installAppMenu(() => menuHandlersRef.current, hasRepo) + // Accelerators track the resolved bindings, so a remap in Settings updates + // the menu too (this effect re-runs when `keyMap` changes). + const accel = (id: CommandId) => + toMudaAccelerator(keyMap.byCommand.get(id) ?? null) ?? undefined; + installAppMenu(() => menuHandlersRef.current, hasRepo, accel) .catch((e) => console.warn('app menu install failed', e)); - }, [hasRepo]); + }, [hasRepo, keyMap]); // Density, platform, and font tokens live on the document root so // portal-rendered popovers (which attach to document.body, outside .os-bg) @@ -577,50 +641,38 @@ export function App() { }; }, [refreshLocalChanges, refreshLog]); - // Global ⌘K / Ctrl+K + // Global keyboard dispatch — every binding is resolved from the registry + + // user overrides (see `lib/keys.ts`). Attached once; the latest bindings and + // handlers are read through refs so settings changes never re-subscribe. useEffect(() => { const onKey = (e: KeyboardEvent) => { - const mod = e.metaKey || e.ctrlKey; - // Combos registered as native-menu accelerators (macOS): AppKit fires - // the menu action before the webview sees the key, so skip them here — - // belt-and-braces against double-handling either way. - if (appMenuInstalled() && mod) { - const k = e.key.toLowerCase(); - if ( - k === 'k' || k === 'o' || k === ',' || ['1', '2', '3', '4', '5'].includes(e.key) || - (e.shiftKey && (k === 't' || k === 's')) - ) return; - } - if (mod && e.key.toLowerCase() === 'k') { - e.preventDefault(); - setPaletteOpen((o) => !o); - } else if (mod && e.key.toLowerCase() === 'o') { - e.preventDefault(); - void openViaDialog(); - } else if (mod && e.key === '1') { - e.preventDefault(); setView('local'); selectFile(null); - } else if (mod && e.key === '2') { - e.preventDefault(); setView('commits'); selectFile(null); - } else if (mod && e.key === '3') { - e.preventDefault(); setView('reflog'); selectFile(null); - } else if (mod && e.key === '4') { - e.preventDefault(); setView('review'); selectFile(null); - } else if (mod && e.key === '5') { - e.preventDefault(); setView('worktrees'); selectFile(null); - } else if (mod && e.shiftKey && e.key.toLowerCase() === 't') { - e.preventDefault(); - const next = cycleTheme(); - showToast(`Theme: ${next[0].toUpperCase()}${next.slice(1)}`); - } else if (mod && e.key === ',') { - e.preventDefault(); - openSettingsAt('appearance'); - } else if (e.key === 'Escape') { - setPaletteOpen(false); + // Esc always closes the palette (the palette's own handler covers the + // case where it has focus; this is the global fallback). + if (e.key === 'Escape') { setPaletteOpen(false); return; } + const binding = eventToBinding(e); + if (!binding) return; + const cmd = keyMapRef.current.byBinding.get(binding); + if (!cmd) return; + // On macOS the native menu owns its accelerators — AppKit fires the menu + // action before the webview sees the key — so defer to it for menu-owned, + // representable combos (no menu installed elsewhere ⇒ JS handles them). + if (appMenuInstalled() && MENU_COMMANDS.has(cmd) && toMudaAccelerator(binding)) return; + // A plain (modifier-less) binding must not steal keystrokes from a text + // field; mod-combos are safe to handle globally. + if (isPlainKey(binding)) { + const t = e.target as HTMLElement | null; + if (t?.closest('input, textarea, [contenteditable="true"], [role="combobox"]')) return; } + // Repo-scoped commands no-op without a repository open. + if (REPO_COMMANDS.has(cmd) && !useRepo.getState().meta) return; + const handler = commandHandlersRef.current[cmd]; + if (!handler) return; + e.preventDefault(); + handler(); }; window.addEventListener('keydown', onKey); return () => window.removeEventListener('keydown', onKey); - }, [setView, selectFile, openViaDialog, cycleTheme, showToast, openSettingsAt]); + }, []); // The working-tree file list is otherwise lazy (only the Files sidebar tab // loads it). Pull it when the palette opens so file search has data; keyed on @@ -765,23 +817,23 @@ export function App() { const paletteActions = useMemo(() => { // Repo-independent — always available. const base: PaletteAction[] = [ - { id: 'open', label: 'Open repository…', group: 'Actions', shortcut: '⌘O', run: () => { void openViaDialog(); } }, - { id: 'clone', label: 'Clone repository…', group: 'Actions', run: () => setCloneOpen(true) }, + { id: 'open', label: 'Open repository…', group: 'Actions', shortcut: keyHint('open-repo'), run: () => { void openViaDialog(); } }, + { id: 'clone', label: 'Clone repository…', group: 'Actions', shortcut: keyHint('clone-repo'), run: () => setCloneOpen(true) }, ]; // Repo-scoped actions only make sense — and only succeed — with a repo // open, so don't surface them (the network ones would fail confusingly). if (meta) { base.push( - { id: 'local', label: 'Show: Local Changes', group: 'Actions', shortcut: '⌘1', run: () => { setView('local'); selectFile(null); } }, - { id: 'commits', label: 'Show: All Commits', group: 'Actions', shortcut: '⌘2', run: () => { setView('commits'); selectFile(null); } }, - { id: 'reflog', label: 'Show: Reflog', group: 'Actions', shortcut: '⌘3', keywords: 'history head recover lost orphan', run: () => { setView('reflog'); selectFile(null); } }, - { id: 'review-view', label: 'Show: Review', group: 'Actions', shortcut: '⌘4', keywords: 'ai agent review session changes verdict', run: () => { setView('review'); selectFile(null); } }, - { id: 'worktrees', label: 'Show: Worktrees', group: 'Actions', shortcut: '⌘5', keywords: 'worktree agent feature checkout overview', run: () => { setView('worktrees'); selectFile(null); } }, + { id: 'local', label: 'Show: Local Changes', group: 'Actions', shortcut: keyHint('view-local'), run: () => { setView('local'); selectFile(null); } }, + { id: 'commits', label: 'Show: All Commits', group: 'Actions', shortcut: keyHint('view-commits'), run: () => { setView('commits'); selectFile(null); } }, + { id: 'reflog', label: 'Show: Reflog', group: 'Actions', shortcut: keyHint('view-reflog'), keywords: 'history head recover lost orphan', run: () => { setView('reflog'); selectFile(null); } }, + { id: 'review-view', label: 'Show: Review', group: 'Actions', shortcut: keyHint('view-review'), keywords: 'ai agent review session changes verdict', run: () => { setView('review'); selectFile(null); } }, + { id: 'worktrees', label: 'Show: Worktrees', group: 'Actions', shortcut: keyHint('view-worktrees'), keywords: 'worktree agent feature checkout overview', run: () => { setView('worktrees'); selectFile(null); } }, { id: 'worktree-new', label: 'New worktree…', group: 'Actions', keywords: 'worktree add branch checkout agent', run: () => setWorktreeOpen(true) }, { id: 'search-commits', label: 'Search commits…', group: 'Actions', shortcut: '/', keywords: 'find filter grep message author hash', run: () => { requestCommitSearch(); } }, // Opens the ⌘F bar in whichever diff view is showing; other views // route to Local Changes first (the signal is consumed on mount). - { id: 'search-diff', label: 'Search in diff…', group: 'Actions', shortcut: '⌘F', keywords: 'find in diff grep text content search', run: () => { + { id: 'search-diff', label: 'Search in diff…', group: 'Actions', shortcut: formatBinding('Mod+F', platform), keywords: 'find in diff grep text content search', run: () => { const v = useRepo.getState().view; if (v !== 'local' && v !== 'review') setView('local'); requestDiffSearch(); @@ -832,8 +884,8 @@ export function App() { ...(baseline && baselineDiffCount > 0 ? [{ id: 'copy-review-diff', label: `Copy review diff (since ${baseline.short})`, group: 'Actions', keywords: 'patch clipboard export review baseline session', run: () => copyDiffs(useRepo.getState().baselineDiffs, false) } satisfies PaletteAction] : []), - { id: 'open-editor', label: 'Open in editor', group: 'Actions', keywords: 'external code reveal vscode editor', run: openInEditor }, - { id: 'open-terminal', label: 'Open in terminal', group: 'Actions', keywords: 'shell console cwd iterm terminal', run: openInTerminal }, + { id: 'open-editor', label: 'Open in editor', group: 'Actions', shortcut: keyHint('open-editor'), keywords: 'external code reveal vscode editor', run: openInEditor }, + { id: 'open-terminal', label: 'Open in terminal', group: 'Actions', shortcut: keyHint('open-terminal'), keywords: 'shell console cwd iterm terminal', run: openInTerminal }, { id: 'snapshot', label: 'Save snapshot…', group: 'Actions', run: () => setStashDialog({ snapshot: true }) }, { id: 'stash', label: 'Stash changes…', group: 'Actions', run: () => setStashDialog({ snapshot: false }) }, { id: 'branch-new', label: 'Create branch…', group: 'Actions', keywords: 'new branch from head', run: () => setBranchDialog({ start: null, label: 'HEAD' }) }, @@ -857,7 +909,10 @@ export function App() { } })(); } }, - { id: 'sync', label: 'Sync (Fetch + Pull + Push)', group: 'Actions', shortcut: '⌘⇧S', run: onSync }, + { id: 'fetch', label: 'Fetch', group: 'Actions', shortcut: keyHint('fetch'), keywords: 'fetch remote refs download', run: onSync }, + { id: 'pull', label: 'Pull', group: 'Actions', shortcut: keyHint('pull'), keywords: 'pull merge remote download integrate', run: onPull }, + { id: 'push', label: 'Push', group: 'Actions', shortcut: keyHint('push'), keywords: 'push upload publish remote', run: onPush }, + { id: 'sync', label: 'Sync (Fetch + Pull + Push)', group: 'Actions', shortcut: keyHint('sync'), run: onSync }, // Gated on a non-root HEAD commit — HEAD~1 must exist to reset to. ...(meta.head_oid && commits.find((c) => c.hash === meta.head_oid)?.parents.length ? [{ @@ -882,10 +937,11 @@ export function App() { ); } base.push( - { id: 'settings', label: 'Settings…', group: 'Actions', shortcut: '⌘,', run: () => openSettingsAt('appearance') }, + { id: 'settings', label: 'Settings…', group: 'Actions', shortcut: keyHint('settings'), keywords: 'preferences shortcuts keyboard config options', run: () => openSettingsAt('appearance') }, + { id: 'keybindings', label: 'Settings: Keyboard shortcuts', group: 'Actions', keywords: 'keyboard shortcuts keybindings rebind configure customize', run: () => openSettingsAt('keyboard') }, { id: 'theme-light', label: 'Theme: Light', group: 'Actions', run: () => setTheme('light') }, { id: 'theme-dark', label: 'Theme: Dark', group: 'Actions', run: () => setTheme('dark') }, - { id: 'theme-system', label: 'Theme: System', group: 'Actions', shortcut: '⌘⇧T', run: () => setTheme('system') }, + { id: 'theme-system', label: 'Theme: System', group: 'Actions', shortcut: keyHint('theme-toggle'), run: () => setTheme('system') }, ); // Surface "Abort" in the palette only while an op is actually paused. if (meta?.operation) { @@ -915,13 +971,13 @@ export function App() { run: () => { void openByPath(r.path); }, })); return [...base, ...repoActions, ...recentActions]; - }, [setView, selectFile, onSync, openViaDialog, openByPath, setTheme, recents, + }, [setView, selectFile, onSync, onPull, onPush, openViaDialog, openByPath, setTheme, recents, pushAllTags, onNetProgress, showToast, meta, abortOperation, requestCommitSearch, requestDiffSearch, requestSelectSinceBaseline, openInEditor, openInTerminal, openSettingsAt, repoActions, setRebaseDialog, setRemoteDialog, setRenameBranchDialog, baseline, setBaseline, clearBaseline, stageReviewed, commits, resetTo, unstagedCount, stagedCount, baselineDiffCount, copyDiffs, - reviewNoteCount, clearReviewNotes]); + reviewNoteCount, clearReviewNotes, keyHint, platform]); const rootStyle = { '--font-ui': FONTS.ui[uiFont], diff --git a/ui/src/lib/keys.test.ts b/ui/src/lib/keys.test.ts new file mode 100644 index 0000000..a2bd775 --- /dev/null +++ b/ui/src/lib/keys.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from 'vitest'; + +import { + COMMANDS, + conflictingCommands, + eventToBinding, + formatBinding, + isPlainKey, + resolveBindings, + toMudaAccelerator, + type KeyLike, +} from './keys'; + +const ev = (over: Partial): KeyLike => ({ + key: '', metaKey: false, ctrlKey: false, altKey: false, shiftKey: false, ...over, +}); + +describe('eventToBinding', () => { + it('folds letter case and carries Shift explicitly', () => { + // ⌘P and ⌘⇧P (push vs pull) must stay distinct regardless of key case. + expect(eventToBinding(ev({ key: 'p', metaKey: true }))).toBe('Mod+P'); + expect(eventToBinding(ev({ key: 'P', metaKey: true, shiftKey: true }))).toBe('Mod+Shift+P'); + }); + + it('treats Ctrl and Meta as the same Mod token', () => { + expect(eventToBinding(ev({ key: 'k', ctrlKey: true }))).toBe('Mod+K'); + expect(eventToBinding(ev({ key: 'k', metaKey: true }))).toBe('Mod+K'); + }); + + it('keeps a fixed modifier order Mod+Alt+Shift+key', () => { + expect(eventToBinding(ev({ key: 'a', metaKey: true, altKey: true, shiftKey: true }))) + .toBe('Mod+Alt+Shift+A'); + }); + + it('returns null for a lone modifier press', () => { + expect(eventToBinding(ev({ key: 'Shift', shiftKey: true }))).toBeNull(); + expect(eventToBinding(ev({ key: 'Meta', metaKey: true }))).toBeNull(); + }); + + it('keeps digits and named keys verbatim', () => { + expect(eventToBinding(ev({ key: '1', metaKey: true }))).toBe('Mod+1'); + expect(eventToBinding(ev({ key: ',', metaKey: true }))).toBe('Mod+,'); + }); +}); + +describe('isPlainKey', () => { + it('flags bindings with no Mod/Alt modifier', () => { + expect(isPlainKey('/')).toBe(true); + expect(isPlainKey('Shift+J')).toBe(true); + expect(isPlainKey('Mod+P')).toBe(false); + expect(isPlainKey('Alt+P')).toBe(false); + }); +}); + +describe('resolveBindings', () => { + it('uses defaults with no overrides', () => { + const { byCommand } = resolveBindings(); + expect(byCommand.get('push')).toBe('Mod+P'); + expect(byCommand.get('pull')).toBe('Mod+Shift+P'); + }); + + it('applies an override and unbinds with null', () => { + const { byCommand, byBinding } = resolveBindings({ push: 'Mod+U', pull: null }); + expect(byCommand.get('push')).toBe('Mod+U'); + expect(byCommand.get('pull')).toBeNull(); + expect(byBinding.get('Mod+U')).toBe('push'); + expect(byBinding.has('Mod+Shift+P')).toBe(false); + }); + + it('first-declared command wins a shared binding for dispatch', () => { + // Bind push to the palette's default; palette is declared earlier. + const { byBinding } = resolveBindings({ push: 'Mod+K' }); + expect(byBinding.get('Mod+K')).toBe('palette'); + }); +}); + +describe('conflictingCommands', () => { + it('flags every command sharing a binding', () => { + const { byCommand } = resolveBindings({ push: 'Mod+K' }); + const clash = conflictingCommands(byCommand); + expect(clash.has('push')).toBe(true); + expect(clash.has('palette')).toBe(true); + expect(clash.has('pull')).toBe(false); + }); + + it('reports none for the default set', () => { + expect(conflictingCommands(resolveBindings().byCommand).size).toBe(0); + }); +}); + +describe('formatBinding', () => { + it('renders mac glyphs joined tight', () => { + expect(formatBinding('Mod+Shift+P', 'mac')).toBe('⌘⇧P'); + expect(formatBinding('Mod+,', 'mac')).toBe('⌘,'); + }); + + it('renders windows words joined with +', () => { + expect(formatBinding('Mod+Shift+P', 'win11')).toBe('Ctrl+Shift+P'); + expect(formatBinding('Mod+1', 'win11')).toBe('Ctrl+1'); + }); + + it('returns empty for an unbound command', () => { + expect(formatBinding(null, 'mac')).toBe(''); + }); +}); + +describe('toMudaAccelerator', () => { + it('maps modifiers and comma', () => { + expect(toMudaAccelerator('Mod+Shift+P')).toBe('CmdOrControl+Shift+P'); + expect(toMudaAccelerator('Mod+,')).toBe('CmdOrControl+Comma'); + }); + + it('returns null for an unrepresentable key', () => { + expect(toMudaAccelerator('Mod+↑')).toBeNull(); + }); + + it('returns null for unbound', () => { + expect(toMudaAccelerator(null)).toBeNull(); + }); +}); + +describe('COMMANDS table', () => { + it('has unique ids and no conflicting default bindings', () => { + const ids = COMMANDS.map((c) => c.id); + expect(new Set(ids).size).toBe(ids.length); + expect(conflictingCommands(resolveBindings().byCommand).size).toBe(0); + }); +}); diff --git a/ui/src/lib/keys.ts b/ui/src/lib/keys.ts new file mode 100644 index 0000000..4a24644 --- /dev/null +++ b/ui/src/lib/keys.ts @@ -0,0 +1,232 @@ +/** + * Central keyboard-shortcut registry — the single source of truth for every + * *global* app command and its binding. Pure logic only (no React, no store + * imports) so it lives in `lib/` and is unit-testable (see the testable-logic + * learning), and so both `App.tsx` (the window keydown handler + palette chips) + * and `lib/menu.ts` (native-menu accelerators) resolve bindings the same way. + * + * ## Binding format + * + * A binding is a canonical `+`-joined string built in a fixed modifier order: + * `Mod` · `Alt` · `Shift` · ``, e.g. `Mod+P`, `Mod+Shift+P`, `Mod+1`. + * `Mod` is the platform primary — ⌘ on macOS, Ctrl elsewhere — so we never + * hardcode one family (AGENTS.md cross-platform rule). The `` is a + * single uppercased letter, a digit, a punctuation char (`,` `/`), or a named + * key (`Enter`, `Escape`, `ArrowUp`…). `null` means "unbound". + * + * User overrides live in `settings.keybindings` ({@link resolveBindings} + * overlays them on the defaults here); `formatBinding` renders a binding for + * display and `toMudaAccelerator` converts one for the macOS menu. + */ + +/** A global command id. The owning handlers live in `App.tsx`. */ +export type CommandId = + | 'palette' + | 'open-repo' + | 'clone-repo' + | 'settings' + | 'view-local' + | 'view-commits' + | 'view-reflog' + | 'view-review' + | 'view-worktrees' + | 'theme-toggle' + | 'fetch' + | 'pull' + | 'push' + | 'sync' + | 'open-editor' + | 'open-terminal' + | 'refresh'; + +export type CommandCategory = 'General' | 'Navigation' | 'Git' | 'Repository' | 'Appearance'; + +export interface CommandDef { + id: CommandId; + /** Human label shown in Settings and (some) menus. */ + label: string; + category: CommandCategory; + /** Default binding in canonical form, or `null` for "no default binding". */ + defaultBinding: string | null; + /** True if a macOS native-menu item owns an accelerator for this command — + * the window keydown handler defers to AppKit for these while the menu is + * installed (see the menu-ownership learning). */ + menu?: boolean; + /** True if the command only makes sense with a repository open (the keydown + * handler no-ops it otherwise; the menu item is `enabled: hasRepo`). */ + needsRepo?: boolean; +} + +/** + * The command table. Order here is the order rows render in Settings, grouped + * by `category`. `Mod` = ⌘ (macOS) / Ctrl (elsewhere). + * + * Push/Pull follow the user's request: push on `Mod+P`, pull on `Mod+Shift+P`. + */ +export const COMMANDS: readonly CommandDef[] = [ + { id: 'palette', label: 'Command palette', category: 'General', defaultBinding: 'Mod+K', menu: true }, + { id: 'open-repo', label: 'Open repository…', category: 'General', defaultBinding: 'Mod+O', menu: true }, + { id: 'clone-repo', label: 'Clone repository…', category: 'General', defaultBinding: null }, + { id: 'settings', label: 'Settings', category: 'General', defaultBinding: 'Mod+,', menu: true }, + + { id: 'view-local', label: 'Go to Local Changes', category: 'Navigation', defaultBinding: 'Mod+1', menu: true, needsRepo: true }, + { id: 'view-commits', label: 'Go to All Commits', category: 'Navigation', defaultBinding: 'Mod+2', menu: true, needsRepo: true }, + { id: 'view-reflog', label: 'Go to Reflog', category: 'Navigation', defaultBinding: 'Mod+3', menu: true, needsRepo: true }, + { id: 'view-review', label: 'Go to Review', category: 'Navigation', defaultBinding: 'Mod+4', menu: true, needsRepo: true }, + { id: 'view-worktrees', label: 'Go to Worktrees', category: 'Navigation', defaultBinding: 'Mod+5', menu: true, needsRepo: true }, + + { id: 'fetch', label: 'Fetch', category: 'Git', defaultBinding: 'Mod+Shift+Y', needsRepo: true }, + { id: 'pull', label: 'Pull', category: 'Git', defaultBinding: 'Mod+Shift+P', menu: true, needsRepo: true }, + { id: 'push', label: 'Push', category: 'Git', defaultBinding: 'Mod+P', menu: true, needsRepo: true }, + { id: 'sync', label: 'Sync (fetch + pull + push)', category: 'Git', defaultBinding: 'Mod+Shift+S', menu: true, needsRepo: true }, + + { id: 'open-editor', label: 'Open in editor', category: 'Repository', defaultBinding: 'Mod+Shift+E', needsRepo: true }, + { id: 'open-terminal', label: 'Open in terminal', category: 'Repository', defaultBinding: 'Mod+Shift+C', needsRepo: true }, + { id: 'refresh', label: 'Refresh', category: 'Repository', defaultBinding: 'Mod+R', needsRepo: true }, + + { id: 'theme-toggle', label: 'Toggle light/dark theme', category: 'Appearance', defaultBinding: 'Mod+Shift+T', menu: true }, +] as const; + +export const CATEGORY_ORDER: readonly CommandCategory[] = + ['General', 'Navigation', 'Git', 'Repository', 'Appearance']; + +const COMMAND_BY_ID = new Map(COMMANDS.map((c) => [c.id, c])); + +/** Commands whose accelerator the macOS native menu owns. */ +export const MENU_COMMANDS: ReadonlySet = new Set( + COMMANDS.filter((c) => c.menu).map((c) => c.id), +); +/** Commands the keydown handler must gate on an open repository. */ +export const REPO_COMMANDS: ReadonlySet = new Set( + COMMANDS.filter((c) => c.needsRepo).map((c) => c.id), +); + +/** User-override map: command id → binding (or `null` to unbind). A missing + * key falls back to the command's default. */ +export type KeyOverrides = Partial>; + +/** Minimal shape of the parts of a KeyboardEvent we read — lets tests pass a + * plain object without constructing a real event. */ +export type KeyLike = Pick< + KeyboardEvent, + 'key' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey' +>; + +/** Keys that are modifiers themselves — pressing one alone isn't a binding. */ +const LONE_MODIFIERS = new Set(['Shift', 'Control', 'Alt', 'Meta', 'CapsLock', 'Dead']); + +/** + * Normalize a keyboard event into a canonical binding string, or `null` if the + * event isn't a usable shortcut (a lone modifier press). Letters fold to + * uppercase (case is carried by the explicit `Shift` token, so `Mod+P` and + * `Mod+Shift+P` stay distinct regardless of the OS-reported key case). + */ +export function eventToBinding(e: KeyLike): string | null { + if (LONE_MODIFIERS.has(e.key)) return null; + let key = e.key; + if (key === ' ') key = 'Space'; + else if (key.length === 1) key = key.toUpperCase(); + // `Mod` collapses ⌘ (mac) and Ctrl (win/linux) into one token. + const parts: string[] = []; + if (e.metaKey || e.ctrlKey) parts.push('Mod'); + if (e.altKey) parts.push('Alt'); + if (e.shiftKey) parts.push('Shift'); + parts.push(key); + return parts.join('+'); +} + +/** True if a binding uses no `Mod`/`Alt` modifier — a "plain" key that must be + * suppressed while a text field is focused. */ +export function isPlainKey(binding: string): boolean { + return !binding.startsWith('Mod+') && !binding.includes('+Mod') + && !binding.startsWith('Alt+') && !binding.includes('+Alt') + && binding !== 'Mod' && binding !== 'Alt'; +} + +export interface ResolvedBindings { + /** binding string → command id (for keydown dispatch). When two commands + * share a binding the *earlier* command in {@link COMMANDS} wins, so dispatch + * stays deterministic; the Settings UI surfaces the clash separately. */ + byBinding: Map; + /** command id → its effective binding (or `null` if unbound). */ + byCommand: Map; +} + +/** Overlay user overrides on the defaults and build both lookup directions. */ +export function resolveBindings(overrides: KeyOverrides = {}): ResolvedBindings { + const byBinding = new Map(); + const byCommand = new Map(); + for (const cmd of COMMANDS) { + const has = Object.prototype.hasOwnProperty.call(overrides, cmd.id); + const binding = has ? overrides[cmd.id]! ?? null : cmd.defaultBinding; + byCommand.set(cmd.id, binding); + if (binding && !byBinding.has(binding)) byBinding.set(binding, cmd.id); + } + return { byBinding, byCommand }; +} + +/** + * Return the set of command ids that share a binding with another command, + * given the resolved per-command map — drives the Settings conflict warning. + */ +export function conflictingCommands(byCommand: Map): Set { + const seen = new Map(); + for (const [id, binding] of byCommand) { + if (!binding) continue; + const list = seen.get(binding) ?? []; + list.push(id); + seen.set(binding, list); + } + const out = new Set(); + for (const list of seen.values()) { + if (list.length > 1) for (const id of list) out.add(id); + } + return out; +} + +export function commandDef(id: CommandId): CommandDef | undefined { + return COMMAND_BY_ID.get(id); +} + +const MAC_MOD: Record = { Mod: '⌘', Shift: '⇧', Alt: '⌥' }; +const WIN_MOD: Record = { Mod: 'Ctrl', Shift: 'Shift', Alt: 'Alt' }; +const MAC_KEY: Record = { + Enter: '↵', Escape: 'Esc', ArrowUp: '↑', ArrowDown: '↓', ArrowLeft: '←', ArrowRight: '→', Space: '␣', +}; +const WIN_KEY: Record = { + Enter: 'Enter', Escape: 'Esc', ArrowUp: '↑', ArrowDown: '↓', ArrowLeft: '←', ArrowRight: '→', Space: 'Space', +}; + +/** + * Render a binding for display. macOS uses glyphs joined tight (`⌘⇧P`); other + * platforms use words joined with `+` (`Ctrl+Shift+P`). + */ +export function formatBinding(binding: string | null, platform: 'mac' | 'win11'): string { + if (!binding) return ''; + const mac = platform === 'mac'; + const mods = mac ? MAC_MOD : WIN_MOD; + const keys = mac ? MAC_KEY : WIN_KEY; + const parts = binding.split('+').map((p) => mods[p] ?? keys[p] ?? p); + return parts.join(mac ? '' : '+'); +} + +const MUDA_MOD: Record = { Mod: 'CmdOrControl', Shift: 'Shift', Alt: 'Alt' }; +const MUDA_KEY: Record = { ',': 'Comma', '/': 'Slash', '.': 'Period' }; + +/** + * Convert a binding to a muda accelerator string for the native menu, or + * `null` if it can't be represented (the menu item then shows no accelerator + * and the keydown handler keeps owning the combo). Covers the keys our menu + * commands actually use (letters, digits, comma). + */ +export function toMudaAccelerator(binding: string | null): string | null { + if (!binding) return null; + const out: string[] = []; + for (const p of binding.split('+')) { + if (MUDA_MOD[p]) out.push(MUDA_MOD[p]); + else if (MUDA_KEY[p]) out.push(MUDA_KEY[p]); + else if (/^[A-Z0-9]$/.test(p)) out.push(p); + else return null; // unrepresentable key — let the JS handler own it + } + return out.join('+'); +} diff --git a/ui/src/lib/menu.ts b/ui/src/lib/menu.ts index 0586c7d..732b8e8 100644 --- a/ui/src/lib/menu.ts +++ b/ui/src/lib/menu.ts @@ -6,6 +6,8 @@ import { type MenuItemOptions, } from '@tauri-apps/api/menu'; +import type { CommandId } from './keys'; + /** * Native macOS menubar, wired to the same callbacks the in-app UI uses. * macOS only — Windows/Linux get an in-window menubar later (PRD §7, @@ -19,6 +21,13 @@ import { * * Repo-scoped items (views, sync, editor/terminal) are disabled with no * repo open; App reinstalls the menu when that flips. + * + * Accelerators are not hardcoded here — they come from the resolved keybinding + * registry via the `accel` resolver, so a shortcut the user remaps in Settings + * shows (and fires) consistently on the menu. App reinstalls the menu when the + * keybindings change. A binding that can't be represented as a muda accelerator + * yields `undefined`, so the menu shows none and the JS keydown handler keeps + * owning that combo. */ export type MenuViewId = 'local' | 'commits' | 'reflog' | 'review' | 'worktrees'; @@ -46,12 +55,22 @@ export function appMenuInstalled(): boolean { return installed; } +/** Resolve a command's muda accelerator string (or `undefined`). */ +export type AccelResolver = (id: CommandId) => string | undefined; + export async function installAppMenu( handlers: () => MenuHandlers, hasRepo: boolean, + accel: AccelResolver, ): Promise { const sep = () => PredefinedMenuItem.new({ item: 'Separator' }); - const item = (opts: MenuItemOptions) => MenuItem.new(opts); + // muda rejects `accelerator: undefined`? It accepts an omitted key, so only + // include the field when we have a string. + const item = (opts: MenuItemOptions & { cmd?: CommandId }) => { + const { cmd, ...rest } = opts; + const a = cmd ? accel(cmd) : undefined; + return MenuItem.new(a ? { ...rest, accelerator: a } : rest); + }; const appMenu = await Submenu.new({ text: 'Strand', @@ -61,7 +80,7 @@ export async function installAppMenu( await item({ id: 'settings', text: 'Settings…', - accelerator: 'Cmd+Comma', + cmd: 'settings', action: () => handlers().openSettings(), }), await item({ @@ -86,7 +105,7 @@ export async function installAppMenu( await item({ id: 'open-repo', text: 'Open Repository…', - accelerator: 'Cmd+O', + cmd: 'open-repo', action: () => handlers().openRepo(), }), await item({ @@ -113,12 +132,12 @@ export async function installAppMenu( ], }); - const views: { id: MenuViewId; text: string; key: string }[] = [ - { id: 'local', text: 'Local Changes', key: 'Cmd+1' }, - { id: 'commits', text: 'All Commits', key: 'Cmd+2' }, - { id: 'reflog', text: 'Reflog', key: 'Cmd+3' }, - { id: 'review', text: 'Review', key: 'Cmd+4' }, - { id: 'worktrees', text: 'Worktrees', key: 'Cmd+5' }, + const views: { id: MenuViewId; text: string; cmd: CommandId }[] = [ + { id: 'local', text: 'Local Changes', cmd: 'view-local' }, + { id: 'commits', text: 'All Commits', cmd: 'view-commits' }, + { id: 'reflog', text: 'Reflog', cmd: 'view-reflog' }, + { id: 'review', text: 'Review', cmd: 'view-review' }, + { id: 'worktrees', text: 'Worktrees', cmd: 'view-worktrees' }, ]; const viewMenu = await Submenu.new({ text: 'View', @@ -126,7 +145,7 @@ export async function installAppMenu( await item({ id: 'palette', text: 'Command Palette…', - accelerator: 'Cmd+K', + cmd: 'palette', action: () => handlers().openPalette(), }), await sep(), @@ -135,7 +154,7 @@ export async function installAppMenu( item({ id: `view-${v.id}`, text: v.text, - accelerator: v.key, + cmd: v.cmd, enabled: hasRepo, action: () => handlers().showView(v.id), }), @@ -145,7 +164,7 @@ export async function installAppMenu( await item({ id: 'cycle-theme', text: 'Toggle Light/Dark Theme', - accelerator: 'Cmd+Shift+T', + cmd: 'theme-toggle', action: () => handlers().cycleTheme(), }), ], @@ -157,12 +176,12 @@ export async function installAppMenu( await item({ id: 'sync', text: 'Sync (Fetch + Pull + Push)', - accelerator: 'Cmd+Shift+S', + cmd: 'sync', enabled: hasRepo, action: () => handlers().sync(), }), - await item({ id: 'pull', text: 'Pull', enabled: hasRepo, action: () => handlers().pull() }), - await item({ id: 'push', text: 'Push', enabled: hasRepo, action: () => handlers().push() }), + await item({ id: 'pull', text: 'Pull', cmd: 'pull', enabled: hasRepo, action: () => handlers().pull() }), + await item({ id: 'push', text: 'Push', cmd: 'push', enabled: hasRepo, action: () => handlers().push() }), await sep(), await item({ id: 'open-editor', diff --git a/ui/src/lib/treeOrder.test.ts b/ui/src/lib/treeOrder.test.ts new file mode 100644 index 0000000..c7e56cd --- /dev/null +++ b/ui/src/lib/treeOrder.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; + +import { compareTreePaths, treeFileOrder } from './treeOrder'; + +describe('treeFileOrder', () => { + it('puts files in nested directories before sibling files (dirs-first)', () => { + // This is the exact case that made j/k "dive into folders": a flat path + // sort yields [app.ts, lib/keys.ts, lib/menu.ts, zebra.ts], but the tree + // shows the lib/ folder (and its files) before app.ts and zebra.ts. + const input = ['src/app.ts', 'src/lib/keys.ts', 'src/lib/menu.ts', 'src/zebra.ts']; + expect(treeFileOrder(input)).toEqual([ + 'src/lib/keys.ts', + 'src/lib/menu.ts', + 'src/app.ts', + 'src/zebra.ts', + ]); + }); + + it('orders top-level directories before top-level files', () => { + expect(treeFileOrder(['readme.md', 'src/a.ts', 'LICENSE'])).toEqual([ + 'src/a.ts', + 'LICENSE', + 'readme.md', + ]); + }); + + it('sorts case-insensitively', () => { + expect(treeFileOrder(['src/Zebra.ts', 'src/apple.ts'])).toEqual([ + 'src/apple.ts', + 'src/Zebra.ts', + ]); + }); + + it('sorts numbers naturally (a2 before a10)', () => { + expect(treeFileOrder(['f10.ts', 'f2.ts', 'f1.ts'])).toEqual(['f1.ts', 'f2.ts', 'f10.ts']); + }); + + it('keeps a shorter (prefix) path before a deeper sibling chain', () => { + // Within src/: the `core` directory sorts before the `core.ts` file. + const input = ['src/core.ts', 'src/core/mod.ts']; + expect(treeFileOrder(input)).toEqual(['src/core/mod.ts', 'src/core.ts']); + }); + + it('is a pure sort (does not mutate input)', () => { + const input = ['b.ts', 'a.ts']; + const copy = [...input]; + treeFileOrder(input); + expect(input).toEqual(copy); + }); +}); + +describe('compareTreePaths', () => { + it('returns 0 for identical paths', () => { + expect(compareTreePaths('src/a.ts', 'src/a.ts')).toBe(0); + }); + + it('is sign-consistent when swapped', () => { + const a = 'src/lib/x.ts'; + const b = 'src/y.ts'; + expect(Math.sign(compareTreePaths(a, b))).toBe(-Math.sign(compareTreePaths(b, a))); + }); +}); diff --git a/ui/src/lib/treeOrder.ts b/ui/src/lib/treeOrder.ts new file mode 100644 index 0000000..a74851c --- /dev/null +++ b/ui/src/lib/treeOrder.ts @@ -0,0 +1,101 @@ +/** + * File ordering that matches how `@pierre/trees` lays a tree out top-to-bottom. + * + * The Pierre file tree sorts **directories before files** at each path level, + * then by a **case-insensitive natural** comparison of the segment name (so + * `a2` < `a10`). A flat full-path string sort (what `repo` diff lists use) + * interleaves nested files with their siblings differently, which makes `j`/`k` + * appear to "dive into folders" instead of moving straight down the list the + * user sees. Sorting the file paths with this comparator reproduces the tree's + * visible *file* order (folder rows aside) so keyboard nav matches the arrows. + * + * This is a faithful re-implementation of Pierre's internal + * `comparePreparedPaths` (`@pierre/trees/dist/path-store/src/sort.js`); kept in + * `lib/` and unit-tested so the two can be checked against each other. It + * considers only the *file* order — folder flattening and collapsed-folder + * visibility don't affect the relative order of files, so they're ignored. + */ + +type NaturalToken = string | number; + +function isDigit(code: number): boolean { + return code >= 48 && code <= 57; +} + +/** Split a lowercased string into alternating text / number tokens, so numeric + * runs compare by value (`a2` before `a10`). Mirrors Pierre's tokenizer. */ +function splitIntoNaturalTokens(value: string): NaturalToken[] { + const tokens: NaturalToken[] = []; + let tokenStart = 0; + let i = 0; + while (i < value.length) { + while (i < value.length && !isDigit(value.charCodeAt(i))) i += 1; + if (i >= value.length) break; + if (i > tokenStart) tokens.push(value.slice(tokenStart, i)); + let num = 0; + while (i < value.length && isDigit(value.charCodeAt(i))) { + num = num * 10 + (value.charCodeAt(i) - 48); + i += 1; + } + tokens.push(num); + tokenStart = i; + } + if (tokenStart < value.length || tokens.length === 0) tokens.push(value.slice(tokenStart)); + return tokens; +} + +function compareNaturalTokens(left: NaturalToken[], right: NaturalToken[]): number { + const n = Math.min(left.length, right.length); + for (let i = 0; i < n; i++) { + const l = left[i]; + const r = right[i]; + if (l === r) continue; + if (typeof l === 'number' && typeof r === 'number') return l < r ? -1 : 1; + const ls = String(l); + const rs = String(r); + if (ls !== rs) return ls < rs ? -1 : 1; + } + if (left.length !== right.length) return left.length < right.length ? -1 : 1; + return 0; +} + +/** Compare two path segments case-insensitively + naturally, with the raw + * value as the final tiebreak (so case differences are stable). */ +function compareSegments(left: string, right: string): number { + const ll = left.toLowerCase(); + const rl = right.toLowerCase(); + const tokenCmp = compareNaturalTokens(splitIntoNaturalTokens(ll), splitIntoNaturalTokens(rl)); + if (tokenCmp !== 0) return tokenCmp; + if (ll !== rl) return ll < rl ? -1 : 1; + if (left === right) return 0; + return left < right ? -1 : 1; +} + +const segs = (p: string): string[] => p.split('/').filter(Boolean); + +/** + * Compare two *file* paths in Pierre tree display order: directories before + * files at each level, then natural segment order. A path that is a prefix of + * the other sorts first. + */ +export function compareTreePaths(a: string, b: string): number { + const la = segs(a); + const lb = segs(b); + const shared = Math.min(la.length, lb.length); + for (let depth = 0; depth < shared; depth++) { + if (la[depth] === lb[depth]) continue; + // A segment is a directory unless it's the path's last segment (these are + // all file paths, so only the final segment is a file). + const aIsDir = depth !== la.length - 1; + const bIsDir = depth !== lb.length - 1; + if (aIsDir !== bIsDir) return aIsDir ? -1 : 1; + return compareSegments(la[depth], lb[depth]); + } + if (la.length !== lb.length) return la.length < lb.length ? -1 : 1; + return 0; +} + +/** Return `paths` in tree display order (does not mutate the input). */ +export function treeFileOrder(paths: readonly string[]): string[] { + return [...paths].sort(compareTreePaths); +} diff --git a/ui/src/stores/settings.ts b/ui/src/stores/settings.ts index fdd1665..3e56b95 100644 --- a/ui/src/stores/settings.ts +++ b/ui/src/stores/settings.ts @@ -1,6 +1,8 @@ import { create } from 'zustand'; import { persist } from 'zustand/middleware'; +import type { KeyOverrides } from '../lib/keys'; + /** A concrete theme that maps to a `[data-theme]` token set in tokens.css. */ export type Theme = 'dark' | 'light'; /** The user's stored preference: a concrete theme, or `system` to follow the @@ -94,7 +96,16 @@ export interface SettingsState { terminalTool: ExternalTool; updateAutoCheck: boolean; updateAutoInstall: boolean; + /** Per-command keyboard-shortcut overrides. A missing key uses the command's + * default from `lib/keys.ts`; a `null` value means the command is unbound. + * Resolved by `resolveBindings`. */ + keybindings: KeyOverrides; set: (key: K, value: SettingsState[K]) => void; + /** Set (or, with `null`, unbind / with `undefined`, reset to default) the + * binding for one command. */ + setKeybinding: (id: keyof KeyOverrides, binding: string | null | undefined) => void; + /** Clear every override, restoring all commands to their defaults. */ + resetKeybindings: () => void; } export const FONTS = { @@ -160,7 +171,18 @@ export const useSettings = create()( terminalTool: null, updateAutoCheck: true, updateAutoInstall: false, + keybindings: {}, set: (key, value) => set({ [key]: value } as Partial), + setKeybinding: (id, binding) => + set((s) => { + const next = { ...s.keybindings }; + // `undefined` removes the override (back to default); anything else + // (a binding string or `null` to unbind) is stored explicitly. + if (binding === undefined) delete next[id]; + else next[id] = binding; + return { keybindings: next }; + }), + resetKeybindings: () => set({ keybindings: {} }), }), { name: 'strand.settings', diff --git a/ui/src/styles/features.css b/ui/src/styles/features.css index 4c2d5de..1787095 100644 --- a/ui/src/styles/features.css +++ b/ui/src/styles/features.css @@ -3401,6 +3401,78 @@ textarea.clone-input { color: var(--text-2); } +/* Keyboard settings — rebindable shortcut rows + context reference. */ +.kb-head { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; +} +.kb-reset-all { + flex: none; + height: 26px; + padding: 0 10px; + font-size: 11.5px; +} +.kb-controls { + display: flex; + align-items: center; + gap: 6px; + flex: none; +} +.kb-chip { + min-width: 64px; + height: 26px; + padding: 0 10px; + border-radius: 6px; + background: var(--bg-base); + box-shadow: 0 0 0 0.5px var(--border) inset; + font-family: var(--font-mono); + font-size: 12px; + color: var(--text); + cursor: pointer; + transition: box-shadow 0.12s ease, color 0.12s ease; +} +.kb-chip:hover { + box-shadow: 0 0 0 0.5px var(--border-strong, var(--text-dim)) inset; +} +.kb-chip.recording { + color: var(--accent-fg, var(--accent)); + box-shadow: 0 0 0 1px var(--accent) inset; +} +.kb-chip.clash { + box-shadow: 0 0 0 1px var(--del) inset; +} +.kb-chip.static { + cursor: default; + color: var(--text-2); +} +.kb-chip.static:hover { + box-shadow: 0 0 0 0.5px var(--border) inset; +} +.kb-warn { + color: var(--del); +} +.kb-icon-btn { + display: inline-flex; + align-items: center; + justify-content: center; + width: 24px; + height: 24px; + border-radius: 6px; + color: var(--text-dim); + cursor: pointer; + transition: background 0.12s ease, color 0.12s ease; +} +.kb-icon-btn:hover:not(:disabled) { + background: var(--bg-hover, var(--bg-elev)); + color: var(--text); +} +.kb-icon-btn:disabled { + opacity: 0.35; + cursor: default; +} + /* Merge dialog — strategy radio list (reuses the clone-dialog shell). */ .merge-modes { display: flex; diff --git a/ui/src/views/LocalChanges.tsx b/ui/src/views/LocalChanges.tsx index 2760332..81f8bfd 100644 --- a/ui/src/views/LocalChanges.tsx +++ b/ui/src/views/LocalChanges.tsx @@ -17,6 +17,7 @@ import { ignorePatterns } from '../lib/ignore'; import { concatPatches, patchesToMarkdown } from '../lib/patchExport'; import { gitErrorHint } from '../lib/tauri'; import { sliceChangeBlock, type SliceDirection } from '../lib/patch'; +import { treeFileOrder } from '../lib/treeOrder'; import type { LocalSelection } from '../stores/repo'; import { useRepo } from '../stores/repo'; import { useSettings } from '../stores/settings'; @@ -197,10 +198,12 @@ export function LocalChanges() { const state = useRepo.getState(); const sel = state.localSelection; - // Flat nav order: unstaged → staged. + // Nav order: unstaged → staged, each in the tree's *display* order + // (folders first, natural sort) so j/k step straight down the list the + // user sees instead of the diff list's flat path order. const nav: { file: string; staged: boolean }[] = [ - ...state.unstagedDiffs.map((d) => ({ file: d.path, staged: false })), - ...state.stagedDiffs.map((d) => ({ file: d.path, staged: true })), + ...treeFileOrder(state.unstagedDiffs.map((d) => d.path)).map((file) => ({ file, staged: false })), + ...treeFileOrder(state.stagedDiffs.map((d) => d.path)).map((file) => ({ file, staged: true })), ]; const curIdx = sel && !sel.all @@ -229,6 +232,13 @@ export function LocalChanges() { stepChangeBlock(e.key === 'n' ? 1 : -1); break; } + // Shift+J / Shift+K scroll the diff pane itself (j/k stay file nav). + case 'J': + case 'K': { + e.preventDefault(); + scrollDiff(e.key === 'J' ? 1 : -1); + break; + } case 's': { if (!sel || sel.all) return; e.preventDefault(); @@ -395,6 +405,17 @@ export function stepChangeBlock(dir: 1 | -1, hostSelector = '.lc-diff-scroll'): if (target != null) host.scrollTo({ top: target, behavior: 'smooth' }); } +/** + * Smoothly scroll the diff pane down (`dir: 1`) or up (`dir: -1`) by most of a + * viewport — powers Shift+J / Shift+K. Like {@link stepChangeBlock}, the host + * selector defaults to Local Changes' scroller; the Review view passes its own. + */ +export function scrollDiff(dir: 1 | -1, hostSelector = '.lc-diff-scroll'): void { + const host = document.querySelector(hostSelector); + if (!host) return; + host.scrollBy({ top: dir * host.clientHeight * 0.85, behavior: 'smooth' }); +} + /** * Strip above the staging workspace listing the unmerged files during a * merge / rebase / cherry-pick. Clicking one shows it in the in-pane conflict diff --git a/ui/src/views/Review.tsx b/ui/src/views/Review.tsx index 4e704e4..057f6a7 100644 --- a/ui/src/views/Review.tsx +++ b/ui/src/views/Review.tsx @@ -22,7 +22,8 @@ import { gitErrorHint } from '../lib/tauri'; import type { FileDiff } from '../lib/types'; import { useRepo } from '../stores/repo'; import { useSettings } from '../stores/settings'; -import { HunkAnnotatedDiff, stepChangeBlock } from './LocalChanges'; +import { treeFileOrder } from '../lib/treeOrder'; +import { HunkAnnotatedDiff, scrollDiff, stepChangeBlock } from './LocalChanges'; /** * The Review view — the surface for reviewing an AI agent's changes, built @@ -163,14 +164,18 @@ export function Review() { return () => window.clearTimeout(t); }, [workerPool, pool, selected]); + // Walk the queue in the same top-to-bottom order the tree *shows* (folders + // first, natural sort) — not the diff list's flat path order, which made j/k + // appear to jump into nested folders out of sequence. Matches the ↑/↓ arrows. + const navOrder = useMemo(() => treeFileOrder(pool.map((d) => d.path)), [pool]); const step = useCallback( (dir: 1 | -1) => { - if (pool.length === 0) return; - const idx = pool.findIndex((d) => d.path === selected); - const next = pool[Math.max(0, Math.min(pool.length - 1, idx === -1 ? 0 : idx + dir))]; - if (next) selectReviewFile(next.path); + if (navOrder.length === 0) return; + const idx = selected ? navOrder.indexOf(selected) : -1; + const next = navOrder[Math.max(0, Math.min(navOrder.length - 1, idx === -1 ? 0 : idx + dir))]; + if (next) selectReviewFile(next); }, - [pool, selected, selectReviewFile], + [navOrder, selected, selectReviewFile], ); /** Toggle the current file's reviewed mark — and stay on the file, so the @@ -432,6 +437,12 @@ export function Review() { e.preventDefault(); stepChangeBlock(e.key === 'n' ? 1 : -1, '.rv-diff-scroll'); break; + // Shift+J / Shift+K scroll the diff pane (j/k stay queue nav). + case 'J': + case 'K': + e.preventDefault(); + scrollDiff(e.key === 'J' ? 1 : -1, '.rv-diff-scroll'); + break; case ' ': e.preventDefault(); markReviewed(); diff --git a/ui/src/views/SettingsDialog.tsx b/ui/src/views/SettingsDialog.tsx index 4214423..69bb782 100644 --- a/ui/src/views/SettingsDialog.tsx +++ b/ui/src/views/SettingsDialog.tsx @@ -5,6 +5,7 @@ import { AppearanceSection } from './settings/AppearanceSection'; import { DiffSection } from './settings/DiffSection'; import { GitSection } from './settings/GitSection'; import { IntegrationsSection } from './settings/IntegrationsSection'; +import { KeyboardSection } from './settings/KeyboardSection'; import { UpdatesSection } from './settings/UpdatesSection'; /** @@ -21,11 +22,12 @@ import { UpdatesSection } from './settings/UpdatesSection'; * Reachable from the status-bar gear, ⌘, and the command palette. */ -export type SettingsSectionId = 'appearance' | 'diff' | 'git' | 'integrations' | 'updates'; +export type SettingsSectionId = 'appearance' | 'diff' | 'keyboard' | 'git' | 'integrations' | 'updates'; const SECTIONS: { id: SettingsSectionId; label: string; icon: IconName }[] = [ { id: 'appearance', label: 'Appearance', icon: 'eye' }, { id: 'diff', label: 'Diff', icon: 'compare' }, + { id: 'keyboard', label: 'Keyboard', icon: 'command' }, { id: 'git', label: 'Git', icon: 'branch' }, { id: 'integrations', label: 'Integrations', icon: 'external' }, { id: 'updates', label: 'Updates', icon: 'sync' }, @@ -154,6 +156,7 @@ export function SettingsDialog({ > {section === 'appearance' && } {section === 'diff' && } + {section === 'keyboard' && } {section === 'git' && } {section === 'integrations' && } {section === 'updates' && } diff --git a/ui/src/views/settings/KeyboardSection.tsx b/ui/src/views/settings/KeyboardSection.tsx new file mode 100644 index 0000000..7249a73 --- /dev/null +++ b/ui/src/views/settings/KeyboardSection.tsx @@ -0,0 +1,184 @@ +import { useEffect, useMemo, useState } from 'react'; + +import { Icon } from '../../components/Icon'; +import { + CATEGORY_ORDER, + COMMANDS, + conflictingCommands, + eventToBinding, + formatBinding, + resolveBindings, + type CommandCategory, + type CommandId, +} from '../../lib/keys'; +import { useSettings } from '../../stores/settings'; + +/** + * Keyboard — view and rebind every global app shortcut. Bindings are resolved + * from `lib/keys.ts` (defaults) overlaid with the user's overrides + * (`settings.keybindings`); changes apply live (the window keydown handler and + * the native menu both read the same resolved map). + * + * Each row: the command, its current binding chip, a "record" toggle (press a + * combo to capture it), and — when overridden — a reset-to-default control. + * A binding shared by two commands is flagged so it's clear which one wins. + * + * The lower "Context shortcuts" card documents the surface-local keys (commit, + * in-diff search, review queue, palette navigation) that live with their + * focused views and aren't rebindable here. + */ + +const CATEGORY_HINT: Partial> = { + Git: 'Push, pull, fetch and sync only act on the open repository.', +}; + +/** Surface-local shortcuts, documented for discoverability (not rebindable). */ +const CONTEXT_SHORTCUTS: { keys: string; plain?: boolean; label: string }[] = [ + { keys: 'Mod+Enter', label: 'Commit (from the message box)' }, + { keys: 'Mod+F', label: 'Search within the current diff' }, + { keys: '/', plain: true, label: 'Search commits (All Commits view)' }, + { keys: 'j / k', plain: true, label: 'Next / previous file (Review queue · Local Changes)' }, + { keys: 'n / p', plain: true, label: 'Next / previous change block in the diff' }, + { keys: 'Shift+J / Shift+K', plain: true, label: 'Scroll the diff pane down / up' }, + { keys: '↑ ↓ · ↵ · ⇥ · Esc', plain: true, label: 'Palette: navigate · run · change scope · close' }, +]; + +export function KeyboardSection() { + const keybindings = useSettings((s) => s.keybindings); + const platform = useSettings((s) => s.platform); + const setKeybinding = useSettings((s) => s.setKeybinding); + const resetKeybindings = useSettings((s) => s.resetKeybindings); + + const [recording, setRecording] = useState(null); + + const resolved = useMemo(() => resolveBindings(keybindings), [keybindings]); + const conflicts = useMemo(() => conflictingCommands(resolved.byCommand), [resolved]); + + // While recording, grab the next keystroke (capture phase, so it lands before + // the app's own keydown handler) and store it as this command's binding. + // Escape cancels; a lone modifier press is ignored until a real key arrives. + useEffect(() => { + if (!recording) return; + const onKey = (e: KeyboardEvent) => { + e.preventDefault(); + e.stopPropagation(); + if (e.key === 'Escape') { setRecording(null); return; } + const binding = eventToBinding(e); + if (!binding) return; // waiting for a non-modifier key + setKeybinding(recording, binding); + setRecording(null); + }; + window.addEventListener('keydown', onKey, true); + return () => window.removeEventListener('keydown', onKey, true); + }, [recording, setKeybinding]); + + const overridden = (id: CommandId) => + Object.prototype.hasOwnProperty.call(keybindings, id); + const anyOverride = Object.keys(keybindings).length > 0; + + return ( +
+
+
+ Shortcuts + +
+

+ Click a shortcut, then press the keys you want. Use Esc to + cancel. +

+
+ + {CATEGORY_ORDER.map((cat) => { + const cmds = COMMANDS.filter((c) => c.category === cat); + if (cmds.length === 0) return null; + return ( +
+ {cat} + {CATEGORY_HINT[cat] &&

{CATEGORY_HINT[cat]}

} +
+ {cmds.map((c) => { + const binding = resolved.byCommand.get(c.id) ?? null; + const isRec = recording === c.id; + const clash = conflicts.has(c.id); + return ( +
+ + {c.label} + {clash && ( + + Shared with another command + + )} + +
+ + + +
+
+ ); + })} +
+
+ ); + })} + +
+ Context shortcuts +

+ These act on the focused surface and aren't rebindable. +

+
+ {CONTEXT_SHORTCUTS.map((s) => ( +
+ + {s.label} + + + {s.plain ? s.keys : formatBinding(s.keys, platform)} + +
+ ))} +
+
+
+ ); +}