refactor: centralize shortcut routing metadata

This commit is contained in:
lucaronin
2026-04-11 12:59:11 +02:00
parent 69e520b5aa
commit 4b60b9539d
10 changed files with 639 additions and 349 deletions

View File

@@ -729,6 +729,7 @@ Selection-dependent note actions are wired through both the command palette and
Shortcut ownership is explicit:
- `appCommandCatalog.ts` is the shared shortcut manifest for command IDs, modifier rules, and ownership
- Renderer-owned shortcuts flow through `useAppKeyboard` into `appCommandDispatcher.ts`
- Native-owned shortcuts flow through `menu.rs` into `useMenuEvents`, then into `appCommandDispatcher.ts`
- Deterministic QA uses `trigger_menu_command` in Tauri and `window.__laputaTest.triggerMenuCommand()` in browser runs to exercise the native-menu command path without flaky macOS key synthesis

View File

@@ -97,6 +97,7 @@ laputa-app/
│ │ ├── useCommandRegistry.ts # Command palette registry
│ │ ├── useAppCommands.ts # App-level commands
│ │ ├── useAppKeyboard.ts # Keyboard shortcuts
│ │ ├── appCommandCatalog.ts # Shortcut ownership + command metadata
│ │ ├── appCommandDispatcher.ts # Shared shortcut/menu command IDs + dispatch
│ │ ├── useSettings.ts # App settings
│ │ ├── useOnboarding.ts # First-launch flow
@@ -281,7 +282,7 @@ type SidebarSelection =
### Command Registry
`useCommandRegistry` + `useAppCommands` build a centralized command registry. Commands are registered with labels, shortcuts, and handlers. The `CommandPalette` (Cmd+K) fuzzy-searches this registry. Renderer shortcuts (`useAppKeyboard`) and native menu events (`useMenuEvents`) both route through `appCommandDispatcher.ts`, so shortcut behavior is owned by shared command IDs instead of duplicate handler paths.
`useCommandRegistry` + `useAppCommands` build a centralized command registry. Commands are registered with labels, shortcuts, and handlers. The `CommandPalette` (Cmd+K) fuzzy-searches this registry. Shortcut ownership now lives in `appCommandCatalog.ts`: renderer shortcuts (`useAppKeyboard`) resolve commands from that catalog, native menu events (`useMenuEvents`) emit the same command IDs, and `appCommandDispatcher.ts` owns execution rather than duplicate shortcut facts.
Commands whose availability depends on the current note or Git state must also flow through `update_menu_state` so the native menu stays in sync with the command palette. The deleted-note restore action in Changes view is the reference example: the row opens a deleted diff preview, the command palette exposes "Restore Deleted Note", and the Note menu enables the same action only while that preview is active.
@@ -340,7 +341,7 @@ BASE_URL="http://localhost:5173" npx playwright test tests/smoke/<slug>.spec.ts
1. Register the command in `useAppCommands.ts` via the command registry
2. Add a corresponding menu bar item in `menu.rs` for discoverability
3. If it has a keyboard shortcut, map it to the canonical command ID in `appCommandDispatcher.ts` and register ownership in `useAppKeyboard.ts`
3. If it has a keyboard shortcut, register it in `appCommandCatalog.ts` with the canonical command ID, ownership (`renderer` vs `native-menu`), and modifier rule
4. If its enabled state depends on runtime selection (active note, deleted preview, Git status, etc.), thread that flag through `useMenuEvents.ts` and `update_menu_state` so the native menu enables/disables correctly
### Modify styling

View File

@@ -0,0 +1,31 @@
---
type: ADR
id: "0051"
title: "Shared shortcut manifest for testable routing"
status: active
date: 2026-04-11
---
## Context
ADR 0050 moved renderer shortcuts and native menu events onto the same command dispatcher, but shortcut ownership still drifted across multiple places: `appKeyboardShortcuts.ts`, `appCommandDispatcher.ts`, command-palette metadata, and `menu.rs`. That made shortcut regressions easy to reintroduce because the same facts had to be updated manually in several files.
The riskiest failures were exactly the native-owned commands that matter most in a keyboard-first app: `Cmd+\` for raw editor, `Cmd+Shift+I` for properties, and `Cmd+Shift+L` for the AI panel. We need one declarative place that says which command owns which shortcut, whether the shortcut is renderer-owned or native-menu-owned, and how tests should trigger it.
## Decision
**Shortcut-capable app commands are now defined in a shared frontend manifest that owns command IDs, routing semantics, and shortcut ownership. Renderer keyboard handling resolves commands from that manifest, native menu routing dispatches the same command IDs, and deterministic QA for native-owned shortcuts targets those IDs rather than duplicating shortcut facts in ad hoc code paths.**
## Options considered
- **Option A** (chosen): Shared shortcut manifest plus shared dispatcher and deterministic menu-command QA. This reduces drift, improves CodeScene on the command router, and makes native-owned shortcuts provable without flaky macOS key synthesis. Downside: one more manifest to maintain.
- **Option B**: Keep the shared dispatcher from ADR 0050 but continue storing shortcut ownership in separate key maps and menu lists. Lower churn, but it keeps the exact source of the regressions we reopened.
- **Option C**: Move all shortcuts into renderer-only handlers. Easier to test, but weaker macOS menu-bar parity and worse native desktop UX.
## Consequences
- `appCommandCatalog.ts` is now the frontend source of truth for shortcut-capable command IDs, ownership, modifier rules, and dispatch kind.
- `appCommandDispatcher.ts` is reduced to route execution instead of carrying a large switch plus duplicated ownership metadata.
- `useAppKeyboard.ts` resolves shortcuts from the shared manifest, including the distinction between `Cmd+Shift+L` (macOS-only) and `CmdOrCtrl+Shift+I/F/O`.
- Native-menu smoke tests should use `window.__laputaTest.triggerMenuCommand()` or the Tauri `trigger_menu_command` bridge to prove the native command path. Renderer-only commands may still be proven with direct keyboard events.
- This ADR supersedes ADR 0050 by replacing “shared command IDs are enough” with “shared command IDs plus shared shortcut ownership metadata are required.”

View File

@@ -105,4 +105,5 @@ proposed → active → superseded
| [0047](0047-regex-mode-for-view-filter-conditions.md) | Regex mode for view filter conditions | active |
| [0048](0048-relative-date-expressions-in-view-filters.md) | Relative date expressions in view filter conditions | active |
| [0049](0049-per-note-icon-property.md) | Per-note icon property (_icon on individual notes) | active |
| [0050](0050-deterministic-shortcut-command-routing.md) | Deterministic shortcut command routing | active |
| [0050](0050-deterministic-shortcut-command-routing.md) | Deterministic shortcut command routing | superseded → [0051](0051-shared-shortcut-manifest-for-testable-routing.md) |
| [0051](0051-shared-shortcut-manifest-for-testable-routing.md) | Shared shortcut manifest for testable routing | active |