From e5a30abb4169a1d9966fa6e96b547d542fdde22d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 04:40:32 +0000 Subject: [PATCH 1/2] desktop: declare the menu bar, and fold its chords into the keyboard registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tauri.conf.json` set no menu, so Tauri installed `Menu::default()` and a dozen chords came with it — ⌘Q ⌘W ⌘M ⌘H ⌥⌘H ⌘Z ⇧⌘Z ⌘X ⌘C ⌘V ⌘A ⌃⌘F. They were live in the window and invisible twice over. Nothing enumerates that default, so no documentation could list them; and AppKit dispatches a menu key equivalent inside `NSApplication.sendEvent`, before the key window's responder chain, so they never reach the webview's keydown and the keyboard registry could not observe them either. Since #118 every *binding* is discoverable by construction — `shortcuts.test.ts` fails on one no row documents — but these are not bindings, so they sat outside that guarantee entirely. K1 promises a keyboard path that is findable. So the menu is B2's own data now. `crates/b2-desktop/src/menu.rs` holds one table — sections, items, and the chord macOS gives each — with two readers: `build`, which is what the window gets, and `chords`, which the new `menu_chords` command hands the UI. The items stay `PredefinedMenuItem`s deliberately: the Edit menu is load-bearing rather than decorative, since those native items are what route cut/copy/paste into the webview. The consequence is that B2 doesn't *choose* these accelerators — muda assigns them and exposes no getter — so the table restates them, and says so. Two departures from the default, neither touching a chord: its Window menu repeats Close Window (⌘W), which already lives in File, and its Help menu is empty on macOS. On the UI side `ui/src/menukeys.ts` is the third keyboard, beside bindings.ts (B2's own) and editorkeys.ts (CodeMirror's). It mirrors the host's declaration for the two jobs a runtime fetch can't do — the suite's gate runs in node with no host to ask, and the sheet has to paint before the first `invoke` resolves — and the mirror is checked against the host at every boot (`menuDrift`), the "change them together" posture `WRITE_CONFLICT_MESSAGE` and `VAULT_CHANGED_EVENT` already use across this seam. What the reader *sees* comes from the host: render.ts passes `state.menuChords` into `shortcuts()`, so the sheet's new "The menu bar" group is the menu the app installed, not the UI's copy of it. The gate is `menuOverlaps`, not more rows in `conflicts()`, and the difference is the point. `conflicts()` asks a same-scope question, because scope is how an inner surface legitimately answers first — the rename field's Esc before the overlay cascade, the Settings rail's ⌃Tab before the Tab trap. Against the menu that move buys nothing: the keystroke is taken before the webview is consulted, so an editor-scoped ⌘Z is not "nearer the user", it is dead. So the comparison ignores scope entirely. Nothing collides today; `menukeys.test.ts` proves the check can fail rather than only that it passes. One thing the new enumeration surfaces, pinned as its own case: the menu takes ⌘Z, ⇧⌘Z and ⌘A from CodeMirror, which binds all three. The note editor's undo, redo and select-all are therefore the webview's native ones rather than CodeMirror's history and selection commands. That was already true; there was nowhere to write it down. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015SJgCnKnUTKNHVsSxrXq7h --- CLAUDE.md | 10 +- crates/b2-desktop/CLAUDE.md | 22 +- crates/b2-desktop/src/commands.rs | 13 + crates/b2-desktop/src/main.rs | 11 + crates/b2-desktop/src/menu.rs | 437 ++++++++++++++++++++++++++++++ docs/design/invariants.md | 8 +- ui/src/api.ts | 10 + ui/src/bindings.ts | 8 + ui/src/main.ts | 29 ++ ui/src/menukeys.test.ts | 203 ++++++++++++++ ui/src/menukeys.ts | 116 ++++++++ ui/src/render.test.ts | 18 ++ ui/src/render.ts | 17 +- ui/src/shortcuts.test.ts | 24 +- ui/src/shortcuts.ts | 48 +++- ui/src/state.ts | 9 + ui/src/types.ts | 15 + 17 files changed, 962 insertions(+), 36 deletions(-) create mode 100644 crates/b2-desktop/src/menu.rs create mode 100644 ui/src/menukeys.test.ts create mode 100644 ui/src/menukeys.ts diff --git a/CLAUDE.md b/CLAUDE.md index 36a4f8c..11153c1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -247,7 +247,8 @@ if/when one lands — `index-engine.md` §5.)* - **`b2-desktop`** — the Tauri host: the *second* dumb adapter, the GUI sibling of `b2-cli`. Each `#[tauri::command]` is deserialize → one `Vault` call → serialize, reusing the CLI's `--json` view types as the IPC contract; it also owns host-only infrastructure (the async cancellable reindex task, - the fs-watch `vault-changed` pulse, the OS folder dialog). Has its own `CLAUDE.md` with the + the fs-watch `vault-changed` pulse, the OS folder dialog, and the **declared menu bar** — `menu.rs`, + GH #119). Has its own `CLAUDE.md` with the thin-adapter rules — read it before touching this crate. - **`ui/`** (not a crate) — the desktop frontend: Vite + vanilla TS + CodeMirror 6, a separate npm toolchain talking to the host over Tauri IPC (`ui/src/api.ts` is the seam). Rendering a note is a @@ -269,8 +270,11 @@ if/when one lands — `index-engine.md` §5.)* (`ui/src/shortcuts.ts`) that `?` jumps straight to. Chords themselves are declared once in `ui/src/bindings.ts` — the keyboard registry — and the dispatcher, the editor's keymap and that sheet all derive from it, so none of the three can drift; `conflicts()` fails the suite on two - commands sharing a keystroke in one scope, and `ui/src/editorkeys.ts` checks B2's chords against - CodeMirror's own ~100 stock bindings so an upgrade can't quietly take one. The four obligations a new surface owes are in + commands sharing a keystroke in one scope, `ui/src/editorkeys.ts` checks B2's chords against + CodeMirror's own ~100 stock bindings so an upgrade can't quietly take one, and `ui/src/menukeys.ts` + checks them against the **macOS menu bar's** — declared in `crates/b2-desktop/src/menu.rs` rather than + inherited from Tauri's default, since AppKit dispatches a menu accelerator before the webview sees the + key at all, which made it the one clash nothing could detect (GH #119). The four obligations a new surface owes are in [`crates/b2-desktop/CLAUDE.md`](crates/b2-desktop/CLAUDE.md). ### The `Vault` façade (`b2-core/src/vault.rs`) diff --git a/crates/b2-desktop/CLAUDE.md b/crates/b2-desktop/CLAUDE.md index 55a28b1..4040b2a 100644 --- a/crates/b2-desktop/CLAUDE.md +++ b/crates/b2-desktop/CLAUDE.md @@ -136,11 +136,13 @@ Every new surface owes all four. They are cheap while you're building it and exp action lives in a menu, beside the menu item, which is where a keyboard user learns the shortcut that lets them skip the menu next time. - Two things the registry will tell you before a user does. `conflicts()` fails the suite if your + Three things the registry will tell you before a user does. `conflicts()` fails the suite if your chord already means something else in the same scope, so pick the scope honestly — it's what - separates "⏎ commits *this* dialog" from a clash. And `editorkeys.test.ts` compares B2's chords + separates "⏎ commits *this* dialog" from a clash. `editorkeys.test.ts` compares B2's chords against CodeMirror's ~100 stock bindings, so if your chord needs to work while the note is being - edited, that check is what proves the editor isn't already using it. + edited, that check is what proves the editor isn't already using it. And `menukeys.test.ts` + compares them against the **menu bar's** (below) — the one clash no scope and no ordering can + win, because the keystroke never reaches the webview. ### Where the pieces live @@ -161,6 +163,20 @@ Every new surface owes all four. They are cheap while you're building it and exp - **`ui/src/shortcuts.ts`** — the one chord table, rendered as Settings' **Keyboard** section (`?` opens the dialog there). Modifiers as macOS glyphs (⌘ ⇧ ⌫ ⏎); keys macOS spells out in its own menus stay words (Esc, Tab, Space, Home/End). +- **`src/menu.rs` + `ui/src/menukeys.ts`** — the **menu bar**, and the app's third keyboard + ([#119](https://github.com/AlteredCraft/B2/issues/119)). Set no menu and Tauri installs + `Menu::default()`, whose dozen accelerators (⌘Q ⌘W ⌘M ⌘H ⌥⌘H ⌘Z ⇧⌘Z ⌘X ⌘C ⌘V ⌘A ⌃⌘F) are live in + the window and enumerable by nobody — and AppKit dispatches a menu key equivalent inside + `NSApplication.sendEvent`, *before* the key window's responder chain, so they never reach the + webview's `keydown` and the registry cannot observe them either. So `menu.rs` declares the menu as + a table (the items stay `PredefinedMenuItem`s: the Edit menu is load-bearing — it is what routes + cut/copy/paste into the webview), the `menu_chords` command exports it, and `menukeys.ts` mirrors + it for the two jobs a runtime fetch can't do — the suite's gate, and the sheet's first paint. The + mirror is checked against the host at every boot (`menuDrift`), the way `WRITE_CONFLICT_MESSAGE` + and `VAULT_CHANGED_EVENT` are pinned across the same seam: **change the two together.** Note what + the gate is *not*: `conflicts()` asks a same-scope question, and scope buys nothing here — a menu + accelerator is taken before the webview is consulted, so `menuOverlaps` compares across every + scope. - **`ui/src/settingstabs.ts`** — the Settings dialog's rail: the section list and its ARIA `tabs` moves (↑↓ with wrap, Home/End; ⌃Tab cycles from anywhere in the dialog). Its own module for treenav.ts's reason — the paint and the arrows must agree on order, so the order is defined once diff --git a/crates/b2-desktop/src/commands.rs b/crates/b2-desktop/src/commands.rs index c9656ff..9f11c71 100644 --- a/crates/b2-desktop/src/commands.rs +++ b/crates/b2-desktop/src/commands.rs @@ -456,6 +456,19 @@ pub fn embed_stats() -> Vec { .collect() } +/// Every chord the app's **menu bar** takes, in menu order (`menu.rs`, #119). The UI +/// folds these into its keyboard registry as reserved chords: the reference sheet lists +/// them, and the collision check can finally see the one set of chords it was blind to +/// — AppKit dispatches a menu key equivalent before the webview receives the key at all, +/// so no amount of watching `keydown` would have found them. +/// +/// Static data, so infallible and vault-free — the shape of [`embed_device`], not of a +/// façade call. +#[tauri::command] +pub fn menu_chords() -> Vec { + crate::menu::chords() +} + /// Releases the single-in-flight reindex slot on drop, so it is freed on **every** /// exit path — normal return, an early `?` (e.g. model-not-provisioned), or a panic. struct ReindexGuard<'a>(&'a AppState); diff --git a/crates/b2-desktop/src/main.rs b/crates/b2-desktop/src/main.rs index af0bff9..28eb8d9 100644 --- a/crates/b2-desktop/src/main.rs +++ b/crates/b2-desktop/src/main.rs @@ -20,6 +20,10 @@ //! `project` — the model-free half of a reindex (index-engine.md) — opens the fake, //! so the first tree paint never waits on a model load. //! `B2_EMBEDDER=fake` forces the fake everywhere (offline/dev mode). +//! +//! And one it hands off: the **menu bar** is declared in [`menu`] rather than inherited +//! from `Menu::default()`, so its chords are B2's own data and the UI can list them +//! ([#119](https://github.com/AlteredCraft/B2/issues/119)). // This binary is desktop-only (no mobile entry point), so a plain `main` suffices. #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] @@ -27,6 +31,7 @@ mod commands; mod error; mod logging; +mod menu; mod stats; mod watch; @@ -282,6 +287,11 @@ fn main() { let _guard = logging::init_logging(); let state = AppState::new(resolve_root()); tauri::Builder::default() + // The menu bar, declared (#119). Without this call Tauri installs + // `Menu::default()`, whose dozen accelerators nothing in the app can enumerate + // — and AppKit dispatches them before the webview sees a key, so the keyboard + // registry can't observe them either. `menu::MENU` is that list, made data. + .menu(menu::build) // The dialog plugin backs the native folder picker for `choose_vault`. It is // driven host-side only; the webview gets no dialog permission (capabilities/ // default.json), so it can never open a dialog itself. @@ -343,6 +353,7 @@ fn main() { commands::models_dir, commands::embed_device, commands::embed_stats, + commands::menu_chords, ]) .run(tauri::generate_context!()) .expect("error while running the B2 desktop app"); diff --git a/crates/b2-desktop/src/menu.rs b/crates/b2-desktop/src/menu.rs new file mode 100644 index 0000000..9fdb537 --- /dev/null +++ b/crates/b2-desktop/src/menu.rs @@ -0,0 +1,437 @@ +//! The app's macOS menu bar — **declared**, not inherited +//! ([#119](https://github.com/AlteredCraft/B2/issues/119)). +//! +//! Tauri applies `Menu::default()` to an app that sets none, and a dozen chords ride +//! in with it: ⌘Q, ⌘W, ⌘M, ⌘H, ⌥⌘H, ⌘Z, ⇧⌘Z, ⌘X, ⌘C, ⌘V, ⌘A, ⌃⌘F. Those chords are +//! live in the window, and they used to be invisible twice over. Nothing enumerates +//! the default, so no documentation could list them; and AppKit dispatches a menu key +//! equivalent inside `NSApplication.sendEvent` *before* the key window's responder +//! chain, so they never reach the webview's keydown handler and the keyboard registry +//! (`ui/src/bindings.ts`) could not see them either. Invariant **K1** +//! (docs/design/invariants.md) promises a keyboard path that is *findable*; an +//! inherited chord sits outside that promise entirely — you cannot document what you +//! cannot enumerate. +//! +//! So the menu is B2's own data now. [`MENU`] is the whole of it — the sections, their +//! items, and the chord macOS gives each one — and it has exactly two readers: +//! [`build`], which is what the window actually gets, and [`chords`], which the +//! `menu_chords` command hands the UI so the reference sheet can list them and the +//! registry's collision check can see them (`ui/src/menukeys.ts`). +//! +//! **The items stay predefined on purpose.** The Edit menu is load-bearing rather than +//! decorative — Cut/Copy/Paste/Select All work in the webview *because* the native +//! items route the standard selectors to it, so these remain [`PredefinedMenuItem`]s +//! rather than custom items B2 would then have to implement itself. The consequence is +//! that B2 does not *choose* these accelerators: muda assigns them and exposes no +//! getter for them, so the `keys` column below restates them. This is the one place to +//! fix if a muda release ever moves one. +//! +//! **Two departures from `Menu::default()`**, neither of which touches a chord: its +//! Window menu repeats Close Window (⌘W), which already lives in File, and its Help +//! menu is empty on macOS. Neither survives here. + +use serde::Serialize; +use tauri::menu::{AboutMetadata, Menu, PredefinedMenuItem, Submenu}; +use tauri::{AppHandle, Runtime}; + +/// The native behavior an item delegates to — one variant per [`PredefinedMenuItem`] +/// constructor B2 uses. An enum rather than a function pointer so [`MENU`] stays a +/// plain, readable table that the tests below can walk. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Item { + About, + Services, + Hide, + HideOthers, + Quit, + CloseWindow, + Undo, + Redo, + Cut, + Copy, + Paste, + SelectAll, + Fullscreen, + Minimize, + /// macOS's name for `maximize` — the label the platform itself uses. + Zoom, + Separator, +} + +/// One line of the menu. +#[derive(Debug, Clone, Copy)] +struct ItemSpec { + /// Stable id, and the join key the UI mirrors this row by (`edit.copy`). + /// Deliberately *not* prefixed `menu.`: the registry already spells the + /// right-click menu's own commands that way (`menu.open`, `menu.item.next`). + id: &'static str, + item: Item, + /// What the menu shows — and, since the reference sheet renders these verbatim, + /// what the keyboard reference calls the action. One string, both places. + label: &'static str, + /// The chord macOS gives this item, spelled in the chord syntax of + /// `ui/src/bindings.ts` (which is CodeMirror's) so the UI can parse it with the + /// same parser it uses for B2's own chords. `None` for an item with no + /// accelerator — those are real menu items, but they are not keyboard surface. + keys: Option<&'static str>, +} + +/// A section of the menu bar. +#[derive(Debug, Clone, Copy)] +struct SectionSpec { + title: &'static str, + items: &'static [ItemSpec], +} + +const SEPARATOR: ItemSpec = ItemSpec { + id: "separator", + item: Item::Separator, + label: "", + keys: None, +}; + +/// B2's menu bar, in the order it is drawn. +const MENU: &[SectionSpec] = &[ + // The application menu. macOS draws this one from the bundle, and `Menu::default` + // passes the package name here — which is this same string (tauri.conf.json's + // `productName`). + SectionSpec { + title: "B2", + items: &[ + ItemSpec { + id: "app.about", + item: Item::About, + label: "About B2", + keys: None, + }, + SEPARATOR, + ItemSpec { + id: "app.services", + item: Item::Services, + label: "Services", + keys: None, + }, + SEPARATOR, + ItemSpec { + id: "app.hide", + item: Item::Hide, + label: "Hide B2", + keys: Some("Mod-h"), + }, + ItemSpec { + id: "app.hide-others", + item: Item::HideOthers, + label: "Hide Others", + keys: Some("Mod-Alt-h"), + }, + SEPARATOR, + ItemSpec { + id: "app.quit", + item: Item::Quit, + label: "Quit B2", + keys: Some("Mod-q"), + }, + ], + }, + SectionSpec { + title: "File", + items: &[ItemSpec { + id: "file.close-window", + item: Item::CloseWindow, + label: "Close Window", + keys: Some("Mod-w"), + }], + }, + // The load-bearing one: these route the platform's editing selectors into the + // webview, which is how copy and paste work at all inside the note editor. + SectionSpec { + title: "Edit", + items: &[ + ItemSpec { + id: "edit.undo", + item: Item::Undo, + label: "Undo", + keys: Some("Mod-z"), + }, + ItemSpec { + id: "edit.redo", + item: Item::Redo, + label: "Redo", + keys: Some("Mod-Shift-z"), + }, + SEPARATOR, + ItemSpec { + id: "edit.cut", + item: Item::Cut, + label: "Cut", + keys: Some("Mod-x"), + }, + ItemSpec { + id: "edit.copy", + item: Item::Copy, + label: "Copy", + keys: Some("Mod-c"), + }, + ItemSpec { + id: "edit.paste", + item: Item::Paste, + label: "Paste", + keys: Some("Mod-v"), + }, + ItemSpec { + id: "edit.select-all", + item: Item::SelectAll, + label: "Select All", + keys: Some("Mod-a"), + }, + ], + }, + SectionSpec { + title: "View", + items: &[ItemSpec { + id: "view.fullscreen", + item: Item::Fullscreen, + label: "Toggle Full Screen", + keys: Some("Mod-Ctrl-f"), + }], + }, + SectionSpec { + title: "Window", + items: &[ + ItemSpec { + id: "window.minimize", + item: Item::Minimize, + label: "Minimize", + keys: Some("Mod-m"), + }, + ItemSpec { + id: "window.zoom", + item: Item::Zoom, + label: "Zoom", + keys: None, + }, + ], + }, +]; + +/// One menu item that carries a chord — the host's half of the app's keyboard +/// contract, serialized to the UI by the `menu_chords` command. +/// +/// Borrowed rather than owned because [`MENU`] is static: there is nothing to build, +/// only something to hand over. +#[derive(Debug, Clone, Copy, Serialize)] +pub struct MenuChord { + pub id: &'static str, + pub label: &'static str, + /// The chord, in `ui/src/bindings.ts`'s syntax (`Mod-Shift-z`). + pub keys: &'static str, +} + +/// Every chord the menu bar takes, in menu order. +/// +/// Items with no accelerator are skipped: this is the keyboard surface, not an +/// inventory of the menu. `ui/src/menukeys.ts` mirrors the result — see the pin in +/// this module's tests. +pub fn chords() -> Vec { + MENU.iter() + .flat_map(|section| section.items) + .filter_map(|spec| { + spec.keys.map(|keys| MenuChord { + id: spec.id, + label: spec.label, + keys, + }) + }) + .collect() +} + +/// Build the menu [`MENU`] describes — what `tauri::Builder::menu` installs. +pub fn build(app: &AppHandle) -> tauri::Result> { + let about = about_metadata(app); + let menu = Menu::new(app)?; + for section in MENU { + let submenu = Submenu::new(app, section.title, true)?; + for spec in section.items { + submenu.append(&predefined(app, spec, &about)?)?; + } + menu.append(&submenu)?; + } + Ok(menu) +} + +/// The About panel's contents, from the same sources `Menu::default` reads: the +/// package info and the bundle config. +/// +/// `'static` because the only borrowed field is the panel's `icon`, which B2 leaves +/// unset — eliding it here would tie the metadata to the handle it was read from for +/// no reason. +fn about_metadata(app: &AppHandle) -> AboutMetadata<'static> { + let pkg = app.package_info(); + let bundle = &app.config().bundle; + AboutMetadata { + name: Some(pkg.name.clone()), + version: Some(pkg.version.to_string()), + copyright: bundle.copyright.clone(), + authors: bundle.publisher.clone().map(|p| vec![p]), + ..Default::default() + } +} + +/// One [`ItemSpec`] as the native item it delegates to. Every item passes its own +/// `label`, so what the menu shows and what the keyboard reference prints are the +/// same string rather than two that agree today. +fn predefined( + app: &AppHandle, + spec: &ItemSpec, + about: &AboutMetadata<'static>, +) -> tauri::Result> { + let text = Some(spec.label); + match spec.item { + Item::About => PredefinedMenuItem::about(app, text, Some(about.clone())), + Item::Services => PredefinedMenuItem::services(app, text), + Item::Hide => PredefinedMenuItem::hide(app, text), + Item::HideOthers => PredefinedMenuItem::hide_others(app, text), + Item::Quit => PredefinedMenuItem::quit(app, text), + Item::CloseWindow => PredefinedMenuItem::close_window(app, text), + Item::Undo => PredefinedMenuItem::undo(app, text), + Item::Redo => PredefinedMenuItem::redo(app, text), + Item::Cut => PredefinedMenuItem::cut(app, text), + Item::Copy => PredefinedMenuItem::copy(app, text), + Item::Paste => PredefinedMenuItem::paste(app, text), + Item::SelectAll => PredefinedMenuItem::select_all(app, text), + Item::Fullscreen => PredefinedMenuItem::fullscreen(app, text), + Item::Minimize => PredefinedMenuItem::minimize(app, text), + Item::Zoom => PredefinedMenuItem::maximize(app, text), + Item::Separator => PredefinedMenuItem::separator(app), + } +} + +#[cfg(test)] +mod tests { + //! The menu as *data*. [`build`] needs a running app and is left to the app to + //! exercise; the table it reads is what has to hold together, and every check here + //! is a claim the UI relies on — a chord it can parse, an id it can join by, and a + //! list that matches its mirror. + + use super::*; + use std::collections::HashSet; + + /// Every item, separators included. + fn all_items() -> impl Iterator { + MENU.iter().flat_map(|section| section.items) + } + + /// Is this spelled the way `ui/src/bindings.ts`'s `parseChord` reads a chord? + /// + /// A deliberately small check, not a second parser: it exists to catch a chord + /// written in the *platform's* spelling (`CmdOrCtrl+C`, which is what Tauri's own + /// accelerator syntax would want) leaking into a table the UI parses with + /// CodeMirror's. `menukeys.test.ts` runs the real parser over the mirror. + fn is_registry_chord(spec: &str) -> bool { + let mut parts = spec.split('-').collect::>(); + let Some(key) = parts.pop() else { + return false; + }; + let key_ok = key.len() == 1 + && key + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit()); + key_ok + && parts + .iter() + .all(|m| matches!(*m, "Mod" | "Ctrl" | "Shift" | "Alt")) + } + + #[test] + fn every_item_has_a_unique_id_and_a_label() { + // The id is what the UI's mirror joins on, so a duplicate would make one of the + // two rows unaddressable; the label is what both the menu and the keyboard + // reference print, so an empty one is a blank row in the sheet. + let mut seen = HashSet::new(); + for spec in all_items() { + if spec.item == Item::Separator { + continue; + } + assert!(seen.insert(spec.id), "duplicate menu item id: {}", spec.id); + assert!( + !spec.label.is_empty(), + "menu item with no label: {}", + spec.id + ); + } + } + + #[test] + fn a_separator_is_never_keyboard_surface() { + for spec in all_items().filter(|s| s.item == Item::Separator) { + assert!(spec.keys.is_none(), "a separator with a chord"); + assert!(spec.label.is_empty(), "a separator with a label"); + } + } + + #[test] + fn no_two_items_answer_to_the_same_chord() { + // The sheet lists one action per chord, so a menu that binds ⌘W twice — which + // `Menu::default` does, with Close Window in both File and Window — would print + // two rows the reader can't choose between. Dropping that duplicate is one of + // this module's two departures from the default. + let mut seen = HashSet::new(); + for c in chords() { + assert!( + seen.insert(c.keys), + "two menu items on {}: {}", + c.keys, + c.id + ); + } + } + + #[test] + fn every_chord_is_spelled_the_way_the_ui_registry_reads_one() { + for c in chords() { + assert!( + is_registry_chord(c.keys), + "{} is spelled {:?}, which ui/src/bindings.ts cannot parse", + c.id, + c.keys + ); + } + // And the guard has teeth: the spelling this is here to keep out. + assert!(!is_registry_chord("CmdOrCtrl+C")); + assert!(!is_registry_chord("Mod-Meh-c")); + } + + #[test] + fn the_exported_chords_are_what_the_ui_mirrors() { + // `ui/src/menukeys.ts` carries this same list — it is the UI's only offline + // knowledge of what the menu takes, and what its collision gate reads. **Change + // the two together**: the app compares them at startup (`menuDrift`, called from + // the frontend's boot) and reports a mismatch, the same posture as + // `WRITE_CONFLICT_MESSAGE` and `VAULT_CHANGED_EVENT`. + // + // A row moving in or out of this list is a change to what the app reserves from + // its own keyboard, which is exactly the kind of thing that used to happen + // invisibly — hence a pin rather than a count. + let exported: Vec = chords() + .iter() + .map(|c| format!("{} {} {}", c.id, c.keys, c.label)) + .collect(); + assert_eq!( + exported, + [ + "app.hide Mod-h Hide B2", + "app.hide-others Mod-Alt-h Hide Others", + "app.quit Mod-q Quit B2", + "file.close-window Mod-w Close Window", + "edit.undo Mod-z Undo", + "edit.redo Mod-Shift-z Redo", + "edit.cut Mod-x Cut", + "edit.copy Mod-c Copy", + "edit.paste Mod-v Paste", + "edit.select-all Mod-a Select All", + "view.fullscreen Mod-Ctrl-f Toggle Full Screen", + "window.minimize Mod-m Minimize", + ] + ); + } +} diff --git a/docs/design/invariants.md b/docs/design/invariants.md index 9661d9c..f5d1332 100644 --- a/docs/design/invariants.md +++ b/docs/design/invariants.md @@ -184,6 +184,10 @@ tomorrow's model* — made mechanical. open/create/rename/move/delete, global search and find-in-note, entering/leaving edit mode (⌘E) and every in-editor formatting chord, connection discovery and linking, the graph, and each menu/modal (`Escape` dismisses, `Enter` confirms, focus is trapped while an overlay is open and restored on - close). Focus is always visible and follows platform/ARIA conventions. The `b2` CLI satisfies this by + close). Focus is always visible and follows platform/ARIA conventions. **A chord that is live in the + app is B2's to document, whoever authored it**: the macOS menu bar's accelerators are declared rather + than inherited from Tauri's default (`b2-desktop/src/menu.rs`), so the reference sheet can list them + and the collision gate can see them — a chord nothing enumerates cannot be found, and the app cannot + warn about landing on it. The `b2` CLI satisfies this by nature; K1 governs the GUI adapter. ([crates/b2-desktop/CLAUDE.md](../../crates/b2-desktop/CLAUDE.md), - GH #78) + GH #78, #119) diff --git a/ui/src/api.ts b/ui/src/api.ts index 6c44d39..a40f60e 100644 --- a/ui/src/api.ts +++ b/ui/src/api.ts @@ -16,6 +16,7 @@ import type { EmbedStat, ExplainView, LinkReport, + MenuChord, ModelChoice, MoveReport, NeighborView, @@ -251,6 +252,15 @@ export const api = { /** Compute device the embedder runs on for this build — "Metal" or "CPU" (Settings badge). */ embedDevice: (): Promise => invoke("embed_device"), + /** + * Every chord the app's **menu bar** takes (#119) — the host declares the menu + * (b2-desktop `menu.rs`), so this is the authority on chords the webview never sees: + * AppKit dispatches a menu key equivalent before the key window's responder chain. + * The keyboard reference lists them from here, and `menukeys.ts` holds the mirror this + * is checked against at boot. + */ + menuChords: (): Promise => invoke("menu_chords"), + /** * Subscribe to the host's debounced filesystem-watch pulse (#14). `handler` fires once * per burst of external Markdown changes; the returned promise resolves to an unlisten diff --git a/ui/src/bindings.ts b/ui/src/bindings.ts index cb5eaba..eaafb67 100644 --- a/ui/src/bindings.ts +++ b/ui/src/bindings.ts @@ -26,6 +26,14 @@ // (`sideArrowMove`), settingstabs.ts (`tabMove`). Copying their keys in would recreate // exactly the two-sources-of-truth problem this module exists to end, so the sheet // carries them as literal rows instead and shortcuts.ts says why. +// - Not here either: the app menu bar's chords — ⌘Q ⌘W ⌘M ⌘H ⌥⌘H ⌘Z ⇧⌘Z ⌘X ⌘C ⌘V ⌘A +// ⌃⌘F. Those aren't key → action mappings this file could own at all: the host +// declares the menu (crates/b2-desktop/src/menu.rs) and AppKit dispatches its +// accelerators before the key window's responder chain, so the webview never gets a +// keydown for them. What they are to *this* table is a list of keystrokes a new +// binding may not be spelled with, and menukeys.ts is where that gate lives — a +// separate check because `conflicts()` asks a same-scope question and these are taken +// from every scope at once (#119). // // The chord syntax is CodeMirror's (`Mod-Shift-v`) on purpose: the editor's own bindings // come out of this same table and are handed to `keymap.of` verbatim, so there is one diff --git a/ui/src/main.ts b/ui/src/main.ts index 293b8c3..38a5e96 100644 --- a/ui/src/main.ts +++ b/ui/src/main.ts @@ -51,6 +51,7 @@ import { wikiCandidates, wikiInsertion, wikiQueryAt } from "./wikicomplete"; import { FORMATS, insertTable, toggleInline, type InlineFormat } from "./format"; import { canonicalKey, chordFor, displayKeys, isBound } from "./bindings"; import { STOCK_EDITOR_KEYMAP } from "./editorkeys"; +import { menuDrift } from "./menukeys"; import { markdownForPaste } from "./paste"; import { activeAfter, countLabel, FIND_CAP, findMatches, locate, stepActive, type Match } from "./findbar"; import { BOUNDS, initPanes } from "./panes"; @@ -4087,6 +4088,33 @@ function wireEvents(): void { // --- boot ----------------------------------------------------------------------- +/** + * The app menu bar's chords, from the host that declares them (#119) — the last group of + * the keyboard reference, and the one set of chords the webview never sees a keydown for. + * + * It doubles as the mirror's only check. `menukeys.ts` carries an offline copy — the + * suite's collision gate reads it, and the sheet paints from it until this resolves — and + * a copy free to fall behind the menu is precisely what #119 set out to end. A difference + * goes to the console, not to the user: it means someone edited `menu.rs` without editing + * the mirror, which is a developer's bug, and the sheet has already switched to the host's + * own list by the time anyone can open Settings to read it. Nothing here blocks the paint, + * and a failure costs only the switch from mirror to host. + */ +async function loadMenuChords(): Promise { + try { + const chords = await api.menuChords(); + state.menuChords = chords; + const drift = menuDrift(chords); + if (drift.length > 0) { + console.error( + `[b2] ui/src/menukeys.ts no longer matches the host's menu:\n ${drift.join("\n ")}`, + ); + } + } catch (e) { + console.error(`[b2] could not read the menu bar's chords: ${errText(e)}`); + } +} + async function boot(): Promise { loadTheme(); // stamp the saved appearance onto before the first paint loadEmbedReminderPref(); // honor a persisted "don't remind me" before the banner can paint @@ -4097,6 +4125,7 @@ async function boot(): Promise { // host only pulses when the *watched* vault's Markdown changes, and re-points the watch // on a vault switch, so this single subscription always tracks the active vault. void api.onVaultChanged(() => void onVaultChanged()); + void loadMenuChords(); // the keyboard reference's last group — never blocks the paint try { const info = await api.vaultInfo(); state.vaultRoot = info.root; diff --git a/ui/src/menukeys.test.ts b/ui/src/menukeys.test.ts new file mode 100644 index 0000000..0ab2902 --- /dev/null +++ b/ui/src/menukeys.test.ts @@ -0,0 +1,203 @@ +// Where B2's keyboard meets the app menu's (menukeys.ts), pinned — and the reserved-chord +// gate itself. +// +// The gate is `menuOverlaps()` being empty: no B2 binding may be spelled with a chord the +// menu bar already takes. It matters more than the ordinary collision check, because it is +// the one clash a user reports as "that shortcut does nothing" — the keystroke never +// reaches the webview, so no handler runs, nothing logs, and there is no ordering trick +// that would let B2 have it back. Before #119 nothing could see these chords at all: they +// came from Tauri's `Menu::default()`, which the app inherited without enumerating. +// +// It imports the real @codemirror keymaps for the last check, the way editorkeys.test.ts +// does — a claim about what the menu takes *from the editor* is worthless against a +// hand-written guess at what CodeMirror binds. +import { type Binding, displayChord, keystrokes, parseChord } from "./bindings.ts"; +import { editorChords } from "./editorkeys.ts"; +import { MENU_CHORDS, menuDrift, menuOverlaps } from "./menukeys.ts"; +import { sheet } from "./shortcuts.ts"; + +let passed = 0; + +function assert(cond: boolean, msg: string): void { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} +function assertEq(actual: unknown, expected: unknown, msg: string): void { + const [a, b] = [JSON.stringify(actual, null, 1), JSON.stringify(expected, null, 1)]; + if (a !== b) throw new Error(`assertion failed: ${msg}\n actual: ${a}\n expected: ${b}`); +} +function check(name: string, fn: () => void): void { + fn(); + passed++; + console.log(` ok ${name}`); +} + +// --- the mirror itself ---------------------------------------------------------------- + +check("every menu chord parses into the registry's model", () => { + // The host spells these in the registry's syntax precisely so this works; a row it + // can't read is a row the gate below is blind to, and would pass by being ignored. + for (const c of MENU_CHORDS) parseChord(c.keys); +}); + +check("menu items are unique, by id and by chord", () => { + // The chord half is what keeps the sheet honest: it prints one action per chord, so two + // items claiming ⌘W (which `Menu::default` does — Close Window in both File and Window) + // would print two rows the reader can't choose between. menu.rs drops that duplicate. + const ids = new Set(); + const forms = new Set(); + for (const c of MENU_CHORDS) { + assert(!ids.has(c.id), `duplicate menu item id: ${c.id}`); + assert(!forms.has(c.keys), `two menu items on ${c.keys}`); + assert(c.label.trim() !== "", `menu item with no label: ${c.id}`); + ids.add(c.id); + forms.add(c.keys); + } +}); + +// --- the gate ------------------------------------------------------------------------- + +check("no B2 chord lands on one the menu bar takes", () => { + const found = menuOverlaps(); + assertEq(found, [], `${found.length} B2 chord(s) the menu would eat`); +}); + +check("the gate fails on a chord the menu already has", () => { + // The check above proves nothing on its own — this is what proves it can fail. ⌘M is + // Minimize; a B2 command spelled that way would simply never run. + const table: Binding[] = [{ id: "pane.minimap", keys: ["Mod-m"], scope: "global" }]; + assertEq( + menuOverlaps(table).map((o) => `${o.id} ${o.chord} → ${o.item} (${o.form})`), + ["pane.minimap Mod-m → window.minimize (⌘m)"], + "the clash", + ); +}); + +check("a menu chord is taken from every scope, not just the global one", () => { + // The reason this is its own function rather than more rows in BINDINGS run through + // `conflicts()`. Scope is how B2 lets an inner surface answer first — the rename field's + // Esc before the overlay cascade, the Settings rail's ⌃Tab before the Tab trap — and it + // is exactly the move that does *not* work here: AppKit dispatches a menu key equivalent + // inside `NSApplication.sendEvent`, before the key window's responder chain, so the + // webview is never asked. An editor-scoped ⌘Z is not "nearer the user"; it is dead. + const table: Binding[] = [ + { id: "editor.history", keys: ["Mod-z"], scope: "editor" }, + { id: "link.select-all", keys: ["Mod-a"], scope: "overlay:link" }, + ]; + assertEq( + menuOverlaps(table).map((o) => `${o.id} (${o.scope}) → ${o.item}`), + ["editor.history (editor) → edit.undo", "link.select-all (overlay:link) → edit.select-all"], + "scope buys nothing against the menu", + ); +}); + +check("an Any- chord meets every menu chord over its key", () => { + // `Any-Escape` claims every way of holding Escape, so an `Any-` chord over a letter the + // menu uses would claim the menu's form too. Nothing binds one today; this is what + // notices if something does. + const table: Binding[] = [{ id: "panic", keys: ["Any-q"], scope: "global" }]; + assertEq( + menuOverlaps(table).map((o) => o.form), + ["⌘q"], + "Any-q swallows ⌘Q", + ); +}); + +// --- what the menu takes from the editor ---------------------------------------------- + +check("the menu takes exactly these chords from CodeMirror", () => { + // Not a B2-vs-menu clash — a menu-vs-dependency one, and the reason it's worth pinning + // is that it is invisible from inside the editor. CodeMirror binds all three; none of + // them ever reaches it, because the menu item is dispatched first. So the note editor's + // undo, redo and select-all are the *webview's* native ones, not CodeMirror's history + // and selection commands, and a bug report about undo behaving oddly in the editor + // starts here rather than in @codemirror/commands. + // + // This is what "you can't document what you can't enumerate" cost: the fact was true + // before #119 too, and there was nowhere to write it down. + const stock = editorChords().map((c) => ({ ...c, forms: new Set(keystrokes(c.spec)) })); + const taken: string[] = []; + for (const c of MENU_CHORDS) { + const forms = keystrokes(c.keys); + for (const s of stock) { + if (forms.some((f) => s.forms.has(f))) { + taken.push(`${c.id} ${c.keys} — ${s.source}: ${s.command}`); + } + } + } + // The history pair reads "(anonymous)" because CodeMirror builds those two commands as + // closures; the keymap they came from is what names them, which is why the row carries + // it — the same shape editorkeys.test.ts pins its overlaps in. + assertEq( + taken, + [ + "edit.undo Mod-z — historyKeymap: (anonymous)", + "edit.redo Mod-Shift-z — historyKeymap: (anonymous)", + "edit.select-all Mod-a — defaultKeymap: selectAll", + ], + "the chords the menu takes from the editor", + ); +}); + +// --- the sheet ------------------------------------------------------------------------ + +check("every menu chord has a row in the keyboard reference", () => { + // The K1 half of #119: a chord that is live in the app and documented nowhere is the + // failure that matters, and an inherited chord is no more findable than an undocumented + // one. The group is a projection of MENU_CHORDS, so this holds by construction — it is + // asserted so that hand-writing the rows instead would fail rather than quietly drop one. + const rows = sheet().flatMap((g) => g.rows); + for (const c of MENU_CHORDS) { + const want = displayChord(c.keys); + assert( + rows.some((r) => !("ids" in r) && r.keys === want && r.action === c.label), + `${c.id} (${want} — ${c.label}) is in the menu but not in the sheet`, + ); + } +}); + +check("the sheet lists the host's menu when it has one, not the mirror", () => { + // render.ts passes `state.menuChords` once the boot fetch lands. If the sheet ignored + // it, the mirror could drift and the reader would never know — which is the same + // undocumented-chord failure one level up. + const host = [{ id: "app.panic", label: "Panic", keys: "Mod-Shift-p" }]; + const rows = sheet(host).flatMap((g) => g.rows); + assert( + rows.some((r) => !("ids" in r) && r.keys === "⇧⌘P" && r.action === "Panic"), + "the host's row", + ); + assert( + !rows.some((r) => !("ids" in r) && r.action === "Quit B2"), + "and only the host's rows", + ); +}); + +// --- drift ---------------------------------------------------------------------------- + +check("a mirror that matches the host drifts by nothing", () => { + assertEq(menuDrift([...MENU_CHORDS]), [], "no drift"); +}); + +check("drift names what changed, in both directions", () => { + // The three ways menu.rs and menukeys.ts come apart: an item added, an item removed, an + // item whose chord or label moved. Each line is meant to be read in a console by someone + // who has just edited one of the two files. + const mirror = [ + { id: "app.quit", label: "Quit B2", keys: "Mod-q" }, + { id: "edit.copy", label: "Copy", keys: "Mod-c" }, + ]; + const host = [ + { id: "app.quit", label: "Quit B2", keys: "Mod-Shift-q" }, + { id: "view.fullscreen", label: "Toggle Full Screen", keys: "Mod-Ctrl-f" }, + ]; + assertEq( + menuDrift(host, mirror), + [ + "the host declares app.quit Mod-Shift-q (Quit B2); the mirror says app.quit Mod-q (Quit B2)", + "the host declares view.fullscreen Mod-Ctrl-f (Toggle Full Screen); the mirror doesn't have it", + "the mirror has edit.copy Mod-c (Copy); the host doesn't declare it", + ], + "the drift report", + ); +}); + +console.log(`menukeys: ${passed} checks passed`); diff --git a/ui/src/menukeys.ts b/ui/src/menukeys.ts new file mode 100644 index 0000000..d281d02 --- /dev/null +++ b/ui/src/menukeys.ts @@ -0,0 +1,116 @@ +// The *third* keyboard in the app: macOS's menu bar. +// +// bindings.ts owns the chords B2 answers to in the webview, editorkeys.ts owns the ones +// CodeMirror answers to inside the editor, and a dozen more belong to neither — ⌘Q, ⌘W, +// ⌘M, ⌘H, ⌥⌘H, ⌘Z, ⇧⌘Z, ⌘X, ⌘C, ⌘V, ⌘A, ⌃⌘F are the app menu's. Until #119 they came +// from Tauri's `Menu::default()`, which is to say from nowhere anyone could point at: +// the registry couldn't see them, the reference sheet couldn't list them, and +// `conflicts()` couldn't warn about landing a new B2 chord on one. +// +// Why the registry can't just watch for them. AppKit dispatches a menu key equivalent +// inside `NSApplication.sendEvent`, *before* the key window's responder chain — so these +// keystrokes never reach the webview's keydown handler at all. No amount of listening +// finds them; the menu has to be declared and then read. It is, in +// `crates/b2-desktop/src/menu.rs`, and the host hands the list over through the +// `menu_chords` command. +// +// What's here is the **mirror** of that declaration. The host is the authority — it is +// the one that builds the menu — and the UI holds a copy for the two jobs a runtime +// fetch can't do: the collision gate below runs in node, in the suite, with no host to +// ask; and the sheet has to paint before the first `invoke` resolves. The copy is +// checked against the host on every launch (`menuDrift`, called from main.ts's boot), +// which is the same "change them together, and the app says so if you didn't" posture as +// WRITE_CONFLICT_MESSAGE and VAULT_CHANGED_EVENT in api.ts. +// +// The check itself is deliberately *not* `conflicts()`. That function asks "do two +// commands claim one keystroke in one scope?", and scope is the wrong question here: a +// menu accelerator is taken before the webview is consulted, so it cannot be shadowed by +// an inner surface the way a global B2 chord can be by the editor or a modal. ⌘Z is the +// menu's in the note editor, in the find bar, in a modal, everywhere. So `menuOverlaps` +// compares keystrokes across *every* scope, and that difference is the whole point of it +// being its own function rather than more rows in BINDINGS. +import { type Binding, BINDINGS, allKeys, keystrokes } from "./bindings.ts"; +import type { MenuChord } from "./types.ts"; + +/** The menu bar as `crates/b2-desktop/src/menu.rs` declares it — every item that carries + * a chord, in menu order. Mirrors the host's `menu_chords`; change them together (the + * Rust side pins this same list, and `menuDrift` catches it at runtime if you don't). */ +export const MENU_CHORDS = [ + { id: "app.hide", label: "Hide B2", keys: "Mod-h" }, + { id: "app.hide-others", label: "Hide Others", keys: "Mod-Alt-h" }, + { id: "app.quit", label: "Quit B2", keys: "Mod-q" }, + { id: "file.close-window", label: "Close Window", keys: "Mod-w" }, + { id: "edit.undo", label: "Undo", keys: "Mod-z" }, + { id: "edit.redo", label: "Redo", keys: "Mod-Shift-z" }, + { id: "edit.cut", label: "Cut", keys: "Mod-x" }, + { id: "edit.copy", label: "Copy", keys: "Mod-c" }, + { id: "edit.paste", label: "Paste", keys: "Mod-v" }, + { id: "edit.select-all", label: "Select All", keys: "Mod-a" }, + { id: "view.fullscreen", label: "Toggle Full Screen", keys: "Mod-Ctrl-f" }, + { id: "window.minimize", label: "Minimize", keys: "Mod-m" }, +] as const satisfies readonly MenuChord[]; + +/** A B2 chord the menu bar takes first. */ +export interface MenuOverlap { + /** The B2 command that would never run. */ + id: string; + /** Its chord, as the registry spells it. */ + chord: string; + /** The scope it was declared in — reported because it explains nothing away: the + * menu wins in every one of them. */ + scope: string; + /** The menu item that takes it. */ + item: string; + /** The keystroke they meet on, e.g. "⌘z". */ + form: string; +} + +/** Every B2 binding whose keystroke the menu bar claims — in any scope, for the reason + * in the module header. Empty for BINDINGS, and kept that way by menukeys.test.ts. */ +export function menuOverlaps( + bindings: readonly Binding[] = BINDINGS, + menu: readonly MenuChord[] = MENU_CHORDS, +): MenuOverlap[] { + const reserved = menu.map((c) => ({ item: c.id, forms: new Set(keystrokes(c.keys)) })); + const out: MenuOverlap[] = []; + for (const b of bindings) { + for (const spec of allKeys(b)) { + for (const form of keystrokes(spec)) { + for (const r of reserved) { + if (r.forms.has(form)) { + out.push({ id: b.id, chord: spec, scope: b.scope, item: r.item, form }); + } + } + } + } + } + return out; +} + +/** How the host's live menu differs from the mirror above, one line per difference. + * + * Empty is the only healthy answer, and the only one a shipped build should ever + * produce — a difference means someone changed `menu.rs` without changing this file, so + * the sheet the *suite* checks and the gate it runs are both describing a menu the app + * no longer has. Reported rather than thrown: a stale mirror is a developer's problem, + * and it must not take the window down over a keyboard reference. */ +export function menuDrift( + host: readonly MenuChord[], + mirror: readonly MenuChord[] = MENU_CHORDS, +): string[] { + const line = (c: MenuChord): string => `${c.id} ${c.keys} (${c.label})`; + const here = new Map(mirror.map((c) => [c.id, c])); + const out: string[] = []; + for (const c of host) { + const mine = here.get(c.id); + if (!mine) out.push(`the host declares ${line(c)}; the mirror doesn't have it`); + else if (mine.keys !== c.keys || mine.label !== c.label) { + out.push(`the host declares ${line(c)}; the mirror says ${line(mine)}`); + } + } + const theirs = new Set(host.map((c) => c.id)); + for (const c of mirror) { + if (!theirs.has(c.id)) out.push(`the mirror has ${line(c)}; the host doesn't declare it`); + } + return out; +} diff --git a/ui/src/render.test.ts b/ui/src/render.test.ts index ed45d06..d0d15f8 100644 --- a/ui/src/render.test.ts +++ b/ui/src/render.test.ts @@ -283,4 +283,22 @@ check("the panel distinguishes 'no pass yet' from 'the pass found nothing'", () assert(clean.includes("found none"), "a pass that ran and found nothing still says so"); }); +// The Keyboard panel is the K1 promise's *findable* half, and #119 extended it to chords +// B2 doesn't own — the app menu's, which the host declares and hands over at boot. This is +// the one line of wiring that carries them into the paint (`shortcuts(state.menuChords)`), +// and dropping it is invisible: the panel would keep rendering menukeys.ts's mirror and +// look right until the two came apart. +check("the Keyboard panel lists the menu bar the host declared, not the mirror", () => { + const html = modalHtml( + app({ + settingsOpen: true, + settingsTab: "keyboard", + menuChords: [{ id: "app.panic", label: "Panic", keys: "Mod-Shift-p" }], + }), + ); + assert(html.includes("The menu bar"), "the group is in the sheet"); + assert(html.includes("⇧⌘P"), "with the host's chord"); + assert(!html.includes("Quit B2"), "and none of the mirror's rows"); +}); + console.log(`render: ${passed} checks passed`); diff --git a/ui/src/render.ts b/ui/src/render.ts index ddb83d3..c225398 100644 --- a/ui/src/render.ts +++ b/ui/src/render.ts @@ -22,7 +22,7 @@ import { anomalyRows, type AnomalyPath, type AnomalyRow } from "./anomalies.ts"; import { RELATION_VERBS, type AppState, type SideSection } from "./state.ts"; import { allDirs, canMoveInto, renamePrefill } from "./move.ts"; import { shouldPromptEmbedInstall } from "./embedreminder.ts"; -import { SHORTCUTS } from "./shortcuts.ts"; +import { shortcuts } from "./shortcuts.ts"; import { SETTINGS_TABS, type SettingsTabId } from "./settingstabs.ts"; import { buildTree, @@ -1134,7 +1134,7 @@ function settingsPanelHtml(state: AppState): string { case "embedding": return embeddingPanelHtml(state); case "keyboard": - return keyboardPanelHtml(); + return keyboardPanelHtml(state); } } @@ -1228,15 +1228,18 @@ function embeddingPanelHtml(state: AppState): string { // Keyboard — the discoverable half of invariant K1, and now its home rather than a sheet // stacked over this dialog: one surface for the table, reached by `?` from anywhere or by // walking the rail. The table itself is `shortcuts.ts` (GH #78). -function keyboardPanelHtml(): string { +function keyboardPanelHtml(state: AppState): string { return `
Keyboard shortcuts

B2 is fully operable from the keyboard — the mouse is an accelerator, never a requirement.

- ${shortcutsGridHtml()}`; + ${shortcutsGridHtml(state)}`; } -/** Every chord B2 answers to, grouped, from the one table in shortcuts.ts. */ -function shortcutsGridHtml(): string { - const groups = SHORTCUTS.map( +/** Every chord the app answers to, grouped, from the one table in shortcuts.ts — plus + * the menu bar's, which are the host's declaration (`state.menuChords`, #119) rather + * than B2 bindings. `null` until the boot fetch lands, and the sheet falls back to + * menukeys.ts's mirror for that window. */ +function shortcutsGridHtml(state: AppState): string { + const groups = shortcuts(state.menuChords ?? undefined).map( (g) => `

${escapeHtml(g.title)}

${g.items diff --git a/ui/src/shortcuts.test.ts b/ui/src/shortcuts.test.ts index c91ca03..54511a0 100644 --- a/ui/src/shortcuts.test.ts +++ b/ui/src/shortcuts.test.ts @@ -5,8 +5,8 @@ // keyboard registry, one old worry is gone and a better one has replaced it. Gone: a row // spelling a chord the wiring doesn't answer to, because rows no longer spell chords — // they name commands, and `displayKeys` renders whatever bindings.ts says. Merely -// importing this module proves every id in it resolves, since SHORTCUTS is built at load -// and the lookup throws on a miss. +// calling `shortcuts()` proves every id in it resolves, since building a row resolves its +// ids and the lookup throws on a miss. // // The new worry is the other direction: a chord that exists and is documented *nowhere*. // That's the K1 failure that matters (docs/design/invariants.md, GH #78) — an action @@ -18,7 +18,7 @@ // meaning two different things, an entry spelled "Cmd+N" among a page of ⌘N. Those are // the drifts that make a reference stop reading as authoritative. import { BINDINGS, allKeys } from "./bindings.ts"; -import { SHEET, SHORTCUTS } from "./shortcuts.ts"; +import { sheet, shortcuts } from "./shortcuts.ts"; let passed = 0; @@ -32,15 +32,15 @@ function check(name: string, fn: () => void): void { } check("every group has a title and at least one row", () => { - assert(SHORTCUTS.length > 0, "the sheet is not empty"); - for (const g of SHORTCUTS) { + assert(shortcuts().length > 0, "the sheet is not empty"); + for (const g of shortcuts()) { assert(g.title.trim() !== "", "a group with no title"); assert(g.items.length > 0, `an empty group: ${g.title}`); } }); check("every row is a full pair — a chord and what it does", () => { - for (const g of SHORTCUTS) { + for (const g of shortcuts()) { for (const s of g.items) { assert(s.keys.trim() !== "", `a row with no chord under ${g.title}`); assert(s.action.trim() !== "", `a chord with no action: ${s.keys}`); @@ -52,7 +52,7 @@ check("no chord is listed twice within one group", () => { // Across groups is fine and deliberate — ⇧F10 opens a menu on a tree row *and* on a // discovery card, Esc closes an overlay and closes Settings. Two meanings in one group // is the contradiction: the reader has no way to tell which one applies. - for (const g of SHORTCUTS) { + for (const g of shortcuts()) { const seen = new Set(); for (const s of g.items) { assert(!seen.has(s.keys), `${s.keys} appears twice under ${g.title}`); @@ -69,7 +69,7 @@ check("modifiers are written as macOS glyphs, never spelled out", () => { // Esc / Tab / Space / Home / End out in its own menus, and so does B2's existing // chrome ("Close (Esc)"), so those stay words — shortcuts.ts says why. const spelled = /\b(cmd|command|ctrl|control|shift|alt|option|enter|return|backspace)\b/i; - for (const g of SHORTCUTS) { + for (const g of shortcuts()) { for (const s of g.items) { assert(!spelled.test(s.keys), `${JSON.stringify(s.keys)} spells out a modifier (${g.title})`); assert(!s.keys.includes("+"), `${JSON.stringify(s.keys)} joins with "+" instead of adjacency`); @@ -86,7 +86,7 @@ check("every chord B2 binds is documented somewhere in the sheet", () => { // have to be found. Anything else — including a chord that only applies inside a // modal, or only while the find bar is open — has to earn a row. const documented = new Set(); - for (const group of SHEET) { + for (const group of sheet()) { for (const row of group.rows) { if ("ids" in row) for (const id of row.ids) documented.add(id); } @@ -98,11 +98,11 @@ check("every chord B2 binds is documented somewhere in the sheet", () => { }); check("the sheet documents no command that isn't bound", () => { - // The mirror image, and the cheap half: SHORTCUTS is built at import, and resolving a - // row's ids throws on an unknown one, so a stale row can't survive to be rendered. + // The mirror image, and the cheap half: every check above builds the sheet, and + // resolving a row's ids throws on an unknown one, so a stale row can't survive a paint. // Asserted anyway so the failure names the sheet rather than a module-load stack. const ids = new Set(BINDINGS.map((b) => b.id)); - for (const group of SHEET) { + for (const group of sheet()) { for (const row of group.rows) { if (!("ids" in row)) continue; for (const id of row.ids) assert(ids.has(id), `${id} is documented but not bound`); diff --git a/ui/src/shortcuts.ts b/ui/src/shortcuts.ts index 1e47911..27ae9d0 100644 --- a/ui/src/shortcuts.ts +++ b/ui/src/shortcuts.ts @@ -29,7 +29,16 @@ // ⌘ command, ⇧ shift, ⌫ delete, ⏎ return — while keys macOS itself spells out in menus // stay spelled out (Esc, Tab, Space, Home/End). That's the split the app's existing // tooltips already use ("Close (Esc)"); `displayChord` in bindings.ts now applies it. -import { type BindingId, displayKeys } from "./bindings.ts"; +// +// One group has no ids and no hand-written keys either: the menu bar's (#119). Those +// chords aren't B2's — they're the app menu's, and AppKit takes them before the webview +// sees a key — so the sheet lists them from the *host's* declaration, passed in by +// render.ts (`menukeys.ts` supplies the offline mirror for the first paint and the +// suite). K1's promise is that a keyboard path is findable, and it says nothing about +// who authored it. +import { type BindingId, displayChord, displayKeys } from "./bindings.ts"; +import { MENU_CHORDS } from "./menukeys.ts"; +import type { MenuChord } from "./types.ts"; /** One chord and what it does. `keys` is display text, projected from the registry. */ export interface Shortcut { @@ -53,7 +62,9 @@ export interface SheetGroup { readonly rows: readonly SheetRow[]; } -export const SHEET: readonly SheetGroup[] = [ +/** The groups B2 authors — everything except the menu bar's, which is the host's and is + * appended by `sheet()`. */ +const OWN_SHEET: readonly SheetGroup[] = [ { title: "Getting around", rows: [ @@ -157,11 +168,30 @@ export const SHEET: readonly SheetGroup[] = [ }, ]; +/** The whole sheet, in order. + * + * `menu` is the app menu bar's chords, defaulting to the mirror in `menukeys.ts`; + * render.ts passes the **host's own** list once it has arrived, so what the reader sees + * is the menu the app installed rather than the UI's copy of it. Its rows are literal + * because there are no ids to name: an item's `label` is what the menu itself shows, + * which is exactly what the sheet wants to print. */ +export function sheet(menu: readonly MenuChord[] = MENU_CHORDS): readonly SheetGroup[] { + return [ + ...OWN_SHEET, + { + title: "The menu bar", + rows: menu.map((c) => ({ keys: displayChord(c.keys), action: c.label })), + }, + ]; +} + /** The sheet as render.ts paints it — every row's chords resolved to display text. */ -export const SHORTCUTS: ShortcutGroup[] = SHEET.map((group) => ({ - title: group.title, - items: group.rows.map((row) => ({ - keys: "ids" in row ? displayKeys(row.ids) : row.keys, - action: row.action, - })), -})); +export function shortcuts(menu?: readonly MenuChord[]): ShortcutGroup[] { + return sheet(menu).map((group) => ({ + title: group.title, + items: group.rows.map((row) => ({ + keys: "ids" in row ? displayKeys(row.ids) : row.keys, + action: row.action, + })), + })); +} diff --git a/ui/src/state.ts b/ui/src/state.ts index ffbdc4f..35b8e41 100644 --- a/ui/src/state.ts +++ b/ui/src/state.ts @@ -4,6 +4,7 @@ import type { EmbedStat, + MenuChord, ModelChoice, NeighborView, NoteSummary, @@ -249,6 +250,13 @@ export interface AppState { modelsDir: string | null; /** Compute device the embedder runs on ("Metal"/"CPU") — loaded with Settings, else null. */ embedDevice: string | null; + /** + * The app menu bar's chords, as the **host** declares them (b2-desktop `menu.rs`, #119) + * — the keyboard reference's last group. Null until the boot fetch lands, and the sheet + * falls back to `menukeys.ts`'s mirror for that window. Fetched once: a menu is + * compiled-in data, not something that changes under the app. + */ + menuChords: MenuChord[] | null; /** A slow op is in flight. */ loading: boolean; /** @@ -313,6 +321,7 @@ export const state: AppState = { embedReminderDismissed: false, modelsDir: null, embedDevice: null, + menuChords: null, loading: false, reindexing: false, reindexProgress: null, diff --git a/ui/src/types.ts b/ui/src/types.ts index 49ab7d3..5c5afb6 100644 --- a/ui/src/types.ts +++ b/ui/src/types.ts @@ -18,6 +18,21 @@ export interface VaultInfo { notes_total: number; } +/** + * `menu_chords` — one item of the app's **menu bar** that carries a chord + * (b2-desktop `menu.rs`, #119). Not a `b2-core` view type: the menu is the host's own + * surface, and this is the only shape it exports. `keys` is spelled in the keyboard + * registry's chord syntax (`Mod-Shift-z`), so `bindings.ts` can parse it with the same + * parser it uses for B2's own chords; `label` is the text the menu itself shows, and the + * keyboard reference prints it verbatim. See `menukeys.ts` for the mirror this is + * checked against. + */ +export interface MenuChord { + id: string; + label: string; + keys: string; +} + /** * `list_models` / `set_model` — one embedding model the settings picker offers * (b2-embed `ModelChoice`). `current` is the model B2 is configured to use now; From b7262a2cf76706bbb6bd6a1e0782313ba0d8c587 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 13:02:40 +0000 Subject: [PATCH 2/2] ui: repaint the keyboard panel when the host's menu chords land MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch (PR #123). `boot` fires `loadMenuChords` without awaiting it, and `wireEvents` has already bound ⌘, by then — so Settings can be open before the host answers, and the assignment to `state.menuChords` had no repaint behind it. The reader would sit looking at menukeys.ts's mirror, which is the one thing the host list exists to replace. Guarded on `settingsOpen` rather than unconditional: during boot the answer normally lands *before* the first `render()` (a static-data IPC against a vault read and a note list), and painting there would flash the empty shell ahead of the vault. The guard is false at that point, so the boot flow is untouched. Worth noting what the repaint costs when nothing is wrong: nothing. The mirror and the host agree in the healthy case, so the HTML is identical and `paintModal`'s memo skips the swap entirely — the only case where the DOM actually changes is drift, which is the case worth showing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015SJgCnKnUTKNHVsSxrXq7h --- ui/src/main.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ui/src/main.ts b/ui/src/main.ts index 38a5e96..be13c66 100644 --- a/ui/src/main.ts +++ b/ui/src/main.ts @@ -4104,6 +4104,16 @@ async function loadMenuChords(): Promise { try { const chords = await api.menuChords(); state.menuChords = chords; + // The sheet reads this, so a panel that is already up has to be told. `boot` fires + // this fetch without awaiting it and `wireEvents` has already bound ⌘, by then, so + // "Settings is open before the host answers" is reachable, and without a repaint the + // reader would sit looking at the mirror — the one thing the host list exists to + // replace. Guarded rather than unconditional because during boot the answer normally + // lands *before* the first `render()`, and painting there would flash the empty shell + // ahead of the vault read. In the healthy case the two lists agree, so the HTML is + // identical and `paintModal`'s memo skips the swap; the case where the DOM really + // changes is drift, which is the case worth showing. + if (state.settingsOpen) render(); const drift = menuDrift(chords); if (drift.length > 0) { console.error(