From 85861940968c57b22b03657efc7258eabbe3079b Mon Sep 17 00:00:00 2001 From: Test User Date: Mon, 20 Jul 2026 17:27:12 +0200 Subject: [PATCH] Add a help modal to cbd-tui behind a declarative binding table Pressing ? opens an overlay listing usage notes and every key binding. The bindings now live in one declarative table (app/bindings.rs) that both key dispatch and the help modal render from, so the help can never drift from the real bindings. Includes the dev-flow design artifacts. Co-Authored-By: Claude Fable 5 --- architecture/help-modal.md | 162 +++++++++ cbd-tui/src/app/bindings.rs | 661 ++++++++++++++++++++++++++++++++++++ cbd-tui/src/app/help.rs | 227 +++++++++++++ cbd-tui/src/app/mod.rs | 176 +++++++++- cbd-tui/src/main.rs | 115 +------ plan/help-modal.md | 70 ++++ plan/summary.md | 28 ++ quality/help-modal.md | 57 ++++ 8 files changed, 1385 insertions(+), 111 deletions(-) create mode 100644 architecture/help-modal.md create mode 100644 cbd-tui/src/app/bindings.rs create mode 100644 cbd-tui/src/app/help.rs create mode 100644 plan/help-modal.md create mode 100644 plan/summary.md create mode 100644 quality/help-modal.md diff --git a/architecture/help-modal.md b/architecture/help-modal.md new file mode 100644 index 0000000..5a89239 --- /dev/null +++ b/architecture/help-modal.md @@ -0,0 +1,162 @@ +# Help modal for cbd-tui + +## Context and problem statement + +`cbd-tui` is the ratatui/crossterm terminal client for crabidy. All keyboard +handling lives in a single `match (app.focus, key.modifiers, key.code)` in +`main.rs` (`run_ui`), covering global bindings plus per-pane bindings for the +two focusable panes (`UiFocus::Library`, `UiFocus::Queue`). None of this is +discoverable from inside the app: a new user has to read the source to learn +that `Tab` cycles panes or that `a` appends the selected library node to the +queue. + +Goal: pressing `?` opens a help modal that explains basic usage (panes, focus +cycling) and lists all keyboard shortcuts; a key press closes it again. + +## Assumptions (confirmed) + +- The help content must not be able to drift from the real bindings — the + binding table becomes the single source of truth for both dispatch and help + rendering (confirmed with user; see Options). +- The modal is read-only and modal in the strict sense: while it is open, all + other bindings are inert. `?`, `Esc`, and `q` close it (`q` therefore does + **not** quit the app while help is open). +- Bindings stay hardcoded for now. User-configurable keymaps are out of scope, + but the table design must not preclude them later. +- The modal shows **all** scopes (Global, Library, Queue) grouped, not just the + bindings of the currently focused pane — the point is discovery. +- No new dependencies; ratatui's `Clear` widget plus a centered `Rect` is + enough for the overlay. + +## Options considered + +### Option A — static help text, dispatch untouched + +A display-only `const HELP: &[(&str, &str, &str)]` table rendered by the +modal; the existing `match` in `main.rs` stays as-is. + +- **Pros**: smallest diff; zero refactor risk. +- **Cons**: two parallel encodings of the same facts; every binding change now + has a silently skippable second edit site. Historically this is exactly the + kind of table that rots. + +### Option B — declarative binding table (chosen) + +Introduce `app/bindings.rs`: + +- `Scope` — `Global | Library | Queue`, mirroring `UiFocus` plus a global tier. +- `Action` — one variant per user-visible operation (`Quit`, `TogglePlay`, + `VolumeUp`, `LibraryDown`, `QueueRemoveTrack`, …). +- `Binding { scope, mods, code, action, description }` with + `const BINDINGS: &[Binding]`. +- `lookup(focus: UiFocus, key: KeyEvent) -> Option` — scope-aware + table scan (global entries match in any focus; pane entries only when that + pane is focused). +- A `key_label(mods, code) -> String` formatter so the help modal derives the + displayed key from the same data dispatch uses (no hand-written "Ctrl+d" + strings). + +The event loop shrinks to: translate `KeyEvent` → `Action` via `lookup`, then +one `match action` executes it (`App::dispatch`). The help modal renders +`BINDINGS` grouped by `Scope`. + +- **Pros**: single source of truth; help cannot drift; the loop's 30-arm match + becomes data; natural seam for configurable keymaps later. +- **Cons**: moderate refactor of `run_ui`; `Action` execution needs access to + both `&mut App` and the `Sender` (solved by giving `App` its + own `tx`, which it already receives in `App::new`). + +**Decision: Option B**, confirmed with the user. + +## Structure + +```d2 +direction: right + +main: main.rs run_ui loop { + poll: crossterm event poll +} + +app: app module { + bindings: bindings.rs { + table: "BINDINGS: &[Binding]" + lookup: "lookup(focus, key) -> Option" + label: "key_label(mods, code)" + } + state: App { + focus: "focus: UiFocus" + help: "show_help: bool" + dispatch: "dispatch(action)" + } + help_modal: help.rs { + render: "render_help(frame)" + } +} + +server: crabidy-server (gRPC) + +main.poll -> app.bindings.lookup: KeyEvent +app.bindings.lookup -> app.state.dispatch: Action +app.bindings.table -> app.bindings.lookup: dispatch reads +app.bindings.table -> app.help_modal.render: help reads same table +app.state.dispatch -> server: MessageFromUi via tx +app.state.help -> app.help_modal.render: gates overlay +``` + +## Key-press flow + +```d2 +shape: sequence_diagram + +user: User +loop: run_ui loop +bindings: bindings::lookup +app: App + +user -> loop: presses "?" +loop -> bindings: lookup(focus, key) +bindings -> loop: "Some(Action::ToggleHelp)" +loop -> app: "dispatch(ToggleHelp)" +app -> app: "show_help = true" +loop -> app: render() +app -> app: draw panes, then help overlay (Clear + centered popup) + +user -> loop: presses any bound key while help open +loop -> bindings: lookup sees help-open state +bindings -> loop: "only Close actions match (?, Esc, q)" +``` + +## Boundaries and interfaces + +- **`app/bindings.rs`** owns the vocabulary: `Scope`, `Action`, `Binding`, + `BINDINGS`, `lookup`, `key_label`. Pure data + pure functions; no I/O, no + ratatui types — unit-testable without a terminal. +- **`App`** gains `show_help: bool`, a stored `tx: Sender`, and + `dispatch(&mut self, action: Action)`. `run_ui` keeps ownership of the loop + and terminal; quitting stays a loop-level concern (`dispatch` returns a + signal or `Action::Quit` is handled in the loop — decided in api-design). +- **`app/help.rs`** renders the overlay: short usage paragraph (panes, `Tab` + to switch focus) followed by the binding table grouped by scope. Reads + `BINDINGS` only. +- Modal gating lives in one place: when `show_help` is true, `lookup` (or the + loop) only admits close actions. No other component needs to know the modal + exists. + +## Risks + +- **`?` and modifier reporting**: terminals differ on whether `?` arrives with + `SHIFT` set. Match `KeyCode::Char('?')` regardless of the shift modifier + (as the existing `J`/`K`/`G` arms already do for shifted letters). +- **Small terminals**: the full binding list may not fit. Initial version + clamps the popup to the frame and truncates; scrolling is an explicit + non-goal for now (open question below). +- **Refactor regressions**: converting ~30 match arms to table entries risks + transposition mistakes. Mitigated by unit tests asserting `lookup` results + for every current binding (quality-gates stage). + +## Open questions + +- Should the help modal scroll when the terminal is too small, or is + truncation with a "…" indicator acceptable? (Default: truncate.) +- Mouse support is enabled (`EnableMouseCapture`) but unused; clicking outside + the modal to close it is a possible later nicety, not in scope. diff --git a/cbd-tui/src/app/bindings.rs b/cbd-tui/src/app/bindings.rs new file mode 100644 index 0000000..c1a9d54 --- /dev/null +++ b/cbd-tui/src/app/bindings.rs @@ -0,0 +1,661 @@ +//! Declarative keyboard bindings: the single source of truth for both key +//! dispatch (`lookup`) and the help modal (which renders [`BINDINGS`]). +//! +//! Pure data and pure functions — no I/O, no ratatui types — so the whole +//! table is unit-testable without a terminal. + +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + +use super::UiFocus; + +/// Where a binding applies. +/// +/// `Global` entries match regardless of which pane is focused. `Library` and +/// `Queue` entries match only while that pane is focused. `Help` entries +/// match only while the help modal is open — while it is open, *no* other +/// scope matches (the modal is strictly modal). +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum Scope { + Global, + Library, + Queue, + Help, +} + +/// Every user-visible operation a key press can trigger. +/// +/// Pane-specific variants are prefixed with their pane so the enum stays +/// collision-free as panes grow. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Action { + // Global + /// Open the help modal (closing it is `CloseHelp`, scope `Help`). + OpenHelp, + Quit, + CycleFocus, + TogglePlay, + RestartTrack, + VolumeUp, + VolumeDown, + ToggleMute, + ToggleShuffle, + ToggleRepeat, + NextTrack, + PrevTrack, + // Library pane + LibraryFirst, + LibraryLast, + LibraryNext, + LibraryPrev, + LibraryJumpDown, + LibraryJumpUp, + LibraryAscend, + LibraryDive, + LibraryQueueNext, + LibraryQueueAppend, + LibraryQueueReplace, + LibraryToggleMark, + // Queue pane + QueueInsertHere, + QueueFirst, + QueueLast, + QueueNext, + QueuePrev, + QueueJumpDown, + QueueJumpUp, + QueueSelectCurrent, + QueuePlaySelected, + QueueRemoveTrack, + QueueClearKeepCurrent, + QueueClearAll, + // Help modal + CloseHelp, +} + +/// One key binding: a key chord, the scope it applies in, the action it +/// triggers, and the human-readable description the help modal shows. +pub struct Binding { + pub scope: Scope, + pub mods: KeyModifiers, + pub code: KeyCode, + pub action: Action, + /// Shown verbatim in the help modal. Imperative mood, no trailing period. + pub description: &'static str, +} + +/// All bindings, in help-modal display order (grouped by scope; `Global` +/// first, then `Library`, `Queue`, `Help`). +/// +/// Invariant: within one scope, each `(mods, code)` chord appears at most +/// once, except that shifted characters may carry either `SHIFT` or `NONE` +/// (terminals disagree); `lookup` must treat those as equal for `Char` codes. +pub const BINDINGS: &[Binding] = &[ + // -- Global ---------------------------------------------------------- + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char('?'), + action: Action::OpenHelp, + description: "Show this help", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char('q'), + action: Action::Quit, + description: "Quit", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Tab, + action: Action::CycleFocus, + description: "Switch between library and queue", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char(' '), + action: Action::TogglePlay, + description: "Play/pause", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char('r'), + action: Action::RestartTrack, + description: "Restart current track", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('K'), + action: Action::VolumeUp, + description: "Volume up", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('J'), + action: Action::VolumeDown, + description: "Volume down", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char('m'), + action: Action::ToggleMute, + description: "Toggle mute", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char('z'), + action: Action::ToggleShuffle, + description: "Toggle shuffle", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::NONE, + code: KeyCode::Char('x'), + action: Action::ToggleRepeat, + description: "Toggle repeat", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::CONTROL, + code: KeyCode::Char('n'), + action: Action::NextTrack, + description: "Next track", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::CONTROL, + code: KeyCode::Char('p'), + action: Action::PrevTrack, + description: "Previous track", + }, + // -- Library --------------------------------------------------------- + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('j'), + action: Action::LibraryNext, + description: "Select next item", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('k'), + action: Action::LibraryPrev, + description: "Select previous item", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('g'), + action: Action::LibraryFirst, + description: "Select first item", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('G'), + action: Action::LibraryLast, + description: "Select last item", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::CONTROL, + code: KeyCode::Char('d'), + action: Action::LibraryJumpDown, + description: "Jump 15 items down", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::CONTROL, + code: KeyCode::Char('u'), + action: Action::LibraryJumpUp, + description: "Jump 15 items up", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('h'), + action: Action::LibraryAscend, + description: "Go to parent folder", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('l'), + action: Action::LibraryDive, + description: "Enter selected folder", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('s'), + action: Action::LibraryToggleMark, + description: "Mark/unmark selection", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Char('a'), + action: Action::LibraryQueueAppend, + description: "Append selection to queue", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('L'), + action: Action::LibraryQueueNext, + description: "Queue selection after current track", + }, + Binding { + scope: Scope::Library, + mods: KeyModifiers::NONE, + code: KeyCode::Enter, + action: Action::LibraryQueueReplace, + description: "Replace queue with selection", + }, + // -- Queue ----------------------------------------------------------- + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('j'), + action: Action::QueueNext, + description: "Select next track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('k'), + action: Action::QueuePrev, + description: "Select previous track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('g'), + action: Action::QueueFirst, + description: "Select first track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('G'), + action: Action::QueueLast, + description: "Select last track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::CONTROL, + code: KeyCode::Char('d'), + action: Action::QueueJumpDown, + description: "Jump 15 tracks down", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::CONTROL, + code: KeyCode::Char('u'), + action: Action::QueueJumpUp, + description: "Jump 15 tracks up", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('o'), + action: Action::QueueSelectCurrent, + description: "Select the playing track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Enter, + action: Action::QueuePlaySelected, + description: "Play selected track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('p'), + action: Action::QueueInsertHere, + description: "Insert library selection after this track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('d'), + action: Action::QueueRemoveTrack, + description: "Remove selected track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::NONE, + code: KeyCode::Char('c'), + action: Action::QueueClearKeepCurrent, + description: "Clear queue except current track", + }, + Binding { + scope: Scope::Queue, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('C'), + action: Action::QueueClearAll, + description: "Clear entire queue", + }, + // -- Help modal ------------------------------------------------------ + Binding { + scope: Scope::Help, + mods: KeyModifiers::NONE, + code: KeyCode::Char('?'), + action: Action::CloseHelp, + description: "Close help", + }, + Binding { + scope: Scope::Help, + mods: KeyModifiers::NONE, + code: KeyCode::Esc, + action: Action::CloseHelp, + description: "Close help", + }, + Binding { + scope: Scope::Help, + mods: KeyModifiers::NONE, + code: KeyCode::Char('q'), + action: Action::CloseHelp, + description: "Close help", + }, +]; + +/// Translate a key event into an action, honoring modality and focus. +/// +/// While `help_open` is true only `Scope::Help` entries can match — every +/// other key press is swallowed. Otherwise `Scope::Global` entries match in +/// any focus and pane entries match only their own `UiFocus`. +/// +/// For `KeyCode::Char` codes the `SHIFT` modifier is ignored during +/// comparison (terminals disagree on whether shifted characters like `?` or +/// `G` report `SHIFT`); all other modifiers must match exactly. +pub fn lookup(focus: UiFocus, help_open: bool, key: KeyEvent) -> Option { + let scope_matches = |scope: Scope| match (help_open, scope) { + (true, s) => s == Scope::Help, + (false, Scope::Help) => false, + (false, Scope::Global) => true, + (false, Scope::Library) => matches!(focus, UiFocus::Library), + (false, Scope::Queue) => matches!(focus, UiFocus::Queue), + }; + let chord_matches = |b: &Binding| { + if b.code != key.code { + return false; + } + if matches!(b.code, KeyCode::Char(_)) { + b.mods.difference(KeyModifiers::SHIFT) == key.modifiers.difference(KeyModifiers::SHIFT) + } else { + b.mods == key.modifiers + } + }; + BINDINGS + .iter() + .find(|b| scope_matches(b.scope) && chord_matches(b)) + .map(|b| b.action) +} + +/// Human-readable label for a binding's key chord, e.g. `"Space"`, +/// `"Ctrl+d"`, `"?"`. Used by the help modal so displayed keys are derived +/// from the same data dispatch uses. +pub fn key_label(mods: KeyModifiers, code: KeyCode) -> String { + let key = match code { + KeyCode::Char(' ') => "Space".to_string(), + KeyCode::Char(c) => c.to_string(), + KeyCode::Tab => "Tab".to_string(), + KeyCode::Enter => "Enter".to_string(), + KeyCode::Esc => "Esc".to_string(), + other => format!("{other:?}"), + }; + if mods.contains(KeyModifiers::CONTROL) { + format!("Ctrl+{key}") + } else { + // SHIFT is already visible in the character itself ('K', '?', …). + key + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent::new(code, mods) + } + + #[test] + fn global_bindings_match_in_any_focus() { + for focus in [UiFocus::Library, UiFocus::Queue] { + assert_eq!( + lookup(focus, false, key(KeyCode::Char('?'), KeyModifiers::NONE)), + Some(Action::OpenHelp) + ); + assert_eq!( + lookup(focus, false, key(KeyCode::Char('q'), KeyModifiers::NONE)), + Some(Action::Quit) + ); + assert_eq!( + lookup(focus, false, key(KeyCode::Tab, KeyModifiers::NONE)), + Some(Action::CycleFocus) + ); + assert_eq!( + lookup(focus, false, key(KeyCode::Char(' '), KeyModifiers::NONE)), + Some(Action::TogglePlay) + ); + } + } + + #[test] + fn pane_bindings_only_match_their_own_pane() { + // 'h' ascends in the library but is unbound in the queue. + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Char('h'), KeyModifiers::NONE) + ), + Some(Action::LibraryAscend) + ); + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Char('h'), KeyModifiers::NONE) + ), + None + ); + // 'd' removes a track in the queue but is unbound in the library. + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Char('d'), KeyModifiers::NONE) + ), + Some(Action::QueueRemoveTrack) + ); + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Char('d'), KeyModifiers::NONE) + ), + None + ); + } + + #[test] + fn same_chord_resolves_per_pane() { + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Char('j'), KeyModifiers::NONE) + ), + Some(Action::LibraryNext) + ); + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Char('j'), KeyModifiers::NONE) + ), + Some(Action::QueueNext) + ); + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Enter, KeyModifiers::NONE) + ), + Some(Action::LibraryQueueReplace) + ); + assert_eq!( + lookup( + UiFocus::Queue, + false, + key(KeyCode::Enter, KeyModifiers::NONE) + ), + Some(Action::QueuePlaySelected) + ); + } + + #[test] + fn shift_is_ignored_for_char_codes() { + // Terminals disagree on whether shifted characters report SHIFT. + for mods in [KeyModifiers::NONE, KeyModifiers::SHIFT] { + assert_eq!( + lookup(UiFocus::Library, false, key(KeyCode::Char('?'), mods)), + Some(Action::OpenHelp) + ); + assert_eq!( + lookup(UiFocus::Library, false, key(KeyCode::Char('G'), mods)), + Some(Action::LibraryLast) + ); + assert_eq!( + lookup(UiFocus::Queue, false, key(KeyCode::Char('K'), mods)), + Some(Action::VolumeUp) + ); + } + } + + #[test] + fn control_must_match_exactly() { + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Char('n'), KeyModifiers::CONTROL) + ), + Some(Action::NextTrack) + ); + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Char('n'), KeyModifiers::NONE) + ), + None + ); + assert_eq!( + lookup( + UiFocus::Library, + false, + key(KeyCode::Char('d'), KeyModifiers::CONTROL) + ), + Some(Action::LibraryJumpDown) + ); + } + + #[test] + fn open_help_swallows_everything_but_close() { + for focus in [UiFocus::Library, UiFocus::Queue] { + // The three close chords work... + for code in [KeyCode::Char('?'), KeyCode::Esc, KeyCode::Char('q')] { + assert_eq!( + lookup(focus, true, key(code, KeyModifiers::NONE)), + Some(Action::CloseHelp) + ); + } + // ...and every other binding is inert while help is open. + assert_eq!( + lookup(focus, true, key(KeyCode::Char(' '), KeyModifiers::NONE)), + None + ); + assert_eq!( + lookup(focus, true, key(KeyCode::Char('j'), KeyModifiers::NONE)), + None + ); + assert_eq!( + lookup(focus, true, key(KeyCode::Tab, KeyModifiers::NONE)), + None + ); + } + } + + #[test] + fn help_scope_never_matches_while_help_is_closed() { + // Esc is only bound inside the modal. + for focus in [UiFocus::Library, UiFocus::Queue] { + assert_eq!( + lookup(focus, false, key(KeyCode::Esc, KeyModifiers::NONE)), + None + ); + } + } + + #[test] + fn chords_are_unique_within_scope() { + // Normalize SHIFT away for Char codes, mirroring lookup's comparison. + fn norm(b: &Binding) -> (Scope, KeyModifiers, KeyCode) { + let mods = if matches!(b.code, KeyCode::Char(_)) { + b.mods.difference(KeyModifiers::SHIFT) + } else { + b.mods + }; + (b.scope, mods, b.code) + } + let mut seen = std::collections::HashSet::new(); + for b in BINDINGS { + assert!( + seen.insert(norm(b)), + "duplicate chord in scope {:?}: {:?}+{:?}", + b.scope, + b.mods, + b.code + ); + } + } + + #[test] + fn every_binding_has_a_description() { + for b in BINDINGS { + assert!( + !b.description.trim().is_empty(), + "empty description for {:?}", + b.action + ); + } + } + + #[test] + fn key_labels_are_human_readable() { + assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Char('q')), "q"); + assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Char('?')), "?"); + assert_eq!(key_label(KeyModifiers::SHIFT, KeyCode::Char('K')), "K"); + assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Char(' ')), "Space"); + assert_eq!( + key_label(KeyModifiers::CONTROL, KeyCode::Char('d')), + "Ctrl+d" + ); + assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Tab), "Tab"); + assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Enter), "Enter"); + assert_eq!(key_label(KeyModifiers::NONE, KeyCode::Esc), "Esc"); + } +} diff --git a/cbd-tui/src/app/help.rs b/cbd-tui/src/app/help.rs new file mode 100644 index 0000000..0ca9f00 --- /dev/null +++ b/cbd-tui/src/app/help.rs @@ -0,0 +1,227 @@ +//! The help modal: a centered overlay listing usage notes and all key +//! bindings, rendered entirely from [`super::bindings::BINDINGS`]. + +use ratatui::{ + layout::{Constraint, Direction, Layout, Rect}, + style::{Modifier, Style}, + text::{Line, Span}, + widgets::{Block, BorderType, Borders, Clear, Paragraph}, + Frame, +}; + +use super::bindings::{key_label, Scope, BINDINGS}; +use super::{COLOR_PRIMARY, COLOR_SECONDARY}; + +/// The modal's content: a usage blurb, two binding columns (Global left, +/// Library + Queue right, so the whole table fits a typical frame), and a +/// close-keys footer derived from the `Scope::Help` bindings. +struct HelpContent { + usage: Vec>, + left: Vec>, + right: Vec>, + footer: Line<'static>, +} + +impl HelpContent { + fn build() -> Self { + let usage = vec![ + Line::from("Browse the library (left pane) and manage the play queue (right pane)."), + Line::from("Press Tab to switch focus; keys apply globally or to the focused pane."), + Line::from(""), + ]; + + let left = group(Scope::Global, "Global"); + + let mut right = group(Scope::Library, "Library"); + right.push(Line::from("")); + right.extend(group(Scope::Queue, "Queue")); + + // All Help-scope chords close the modal; derive their labels instead + // of hardcoding key names. + let close_keys = BINDINGS + .iter() + .filter(|b| b.scope == Scope::Help) + .map(|b| key_label(b.mods, b.code)) + .collect::>() + .join(", "); + let footer = Line::from(Span::styled( + format!("Close help: {close_keys}"), + Style::default().fg(COLOR_SECONDARY), + )); + + Self { + usage, + left, + right, + footer, + } + } + + fn left_width(&self) -> u16 { + max_width(&self.left) + } + + /// Content size excluding the popup borders. + fn size(&self) -> (u16, u16) { + let columns = self.left_width() + COLUMN_GAP + max_width(&self.right); + let width = columns + .max(max_width(&self.usage)) + .max(self.footer.width() as u16); + let height = self.usage.len() as u16 + + (self.left.len().max(self.right.len()) as u16) + + 2 // blank line + footer + ; + (width, height) + } +} + +const COLUMN_GAP: u16 = 2; + +fn max_width(lines: &[Line<'_>]) -> u16 { + lines.iter().map(|l| l.width() as u16).max().unwrap_or(0) +} + +/// One scope's bindings as a styled header plus `key description` rows, +/// with key labels right-aligned to the group's widest label. +fn group(scope: Scope, title: &'static str) -> Vec> { + let entries: Vec<_> = BINDINGS.iter().filter(|b| b.scope == scope).collect(); + let key_width = entries + .iter() + .map(|b| key_label(b.mods, b.code).chars().count()) + .max() + .unwrap_or(0); + + let mut lines = vec![Line::from(Span::styled( + title, + Style::default() + .fg(COLOR_SECONDARY) + .add_modifier(Modifier::BOLD), + ))]; + lines.extend(entries.iter().map(|b| { + Line::from(vec![ + Span::styled( + format!("{:>key_width$}", key_label(b.mods, b.code)), + Style::default().fg(COLOR_PRIMARY), + ), + Span::from(format!(" {}", b.description)), + ]) + })); + lines +} + +/// Render the help modal over the current frame. +/// +/// Draws a `Clear`-backed, centered popup on top of whatever is already in +/// the frame. If the frame is smaller than the content, the popup is clamped +/// to the frame and overflowing lines are truncated (no scrolling — see +/// architecture/help-modal.md, open questions). +pub fn render(f: &mut Frame) { + let content = HelpContent::build(); + let area = popup_area(f.area()); + + f.render_widget(Clear, area); + let block = Block::default() + .borders(Borders::ALL) + .border_type(BorderType::Rounded) + .border_style(Style::default().fg(COLOR_PRIMARY)) + .title("Help"); + let inner = block.inner(area); + f.render_widget(block, area); + + let rows = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(content.usage.len() as u16), + Constraint::Min(0), + Constraint::Length(1), + ]) + .split(inner); + + let columns = Layout::default() + .direction(Direction::Horizontal) + .constraints([ + Constraint::Length(content.left_width() + COLUMN_GAP), + Constraint::Min(0), + ]) + .split(rows[1]); + + f.render_widget(Paragraph::new(content.usage.clone()), rows[0]); + f.render_widget(Paragraph::new(content.left.clone()), columns[0]); + f.render_widget(Paragraph::new(content.right.clone()), columns[1]); + f.render_widget(Paragraph::new(vec![content.footer.clone()]), rows[2]); +} + +/// The centered popup rectangle: sized to the help content but clamped to +/// `frame`, never exceeding it. +fn popup_area(frame: Rect) -> Rect { + let (content_w, content_h) = HelpContent::build().size(); + let width = content_w.saturating_add(2).min(frame.width); + let height = content_h.saturating_add(2).min(frame.height); + Rect::new( + frame.x + (frame.width - width) / 2, + frame.y + (frame.height - height) / 2, + width, + height, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use ratatui::{backend::TestBackend, Terminal}; + + fn render_to_buffer(width: u16, height: u16) -> ratatui::buffer::Buffer { + let backend = TestBackend::new(width, height); + let mut terminal = Terminal::new(backend).expect("test terminal"); + terminal.draw(render).expect("draw help"); + terminal.backend().buffer().clone() + } + + fn buffer_text(buf: &ratatui::buffer::Buffer) -> String { + let mut text = String::new(); + for y in 0..buf.area.height { + for x in 0..buf.area.width { + text.push_str(buf[(x, y)].symbol()); + } + text.push('\n'); + } + text + } + + #[test] + fn help_lists_bindings_from_the_table() { + let text = buffer_text(&render_to_buffer(100, 40)); + // Spot-check one entry per scope, by description from BINDINGS. + assert!(text.contains("Quit")); + assert!(text.contains("Enter selected folder")); + assert!(text.contains("Remove selected track")); + assert!(text.contains("Close help")); + } + + #[test] + fn help_explains_basic_usage() { + let text = buffer_text(&render_to_buffer(100, 40)); + // The usage blurb must mention the panes and how to switch focus. + assert!(text.contains("Tab")); + assert!(text.to_lowercase().contains("library")); + assert!(text.to_lowercase().contains("queue")); + } + + #[test] + fn help_survives_tiny_terminals() { + // Truncation, not panic, on frames smaller than the content. + for (w, h) in [(10, 5), (20, 10), (1, 1)] { + let _ = render_to_buffer(w, h); + } + } + + #[test] + fn popup_never_exceeds_the_frame() { + for (w, h) in [(100, 40), (30, 12), (5, 3)] { + let frame = Rect::new(0, 0, w, h); + let popup = popup_area(frame); + assert!(popup.right() <= frame.right()); + assert!(popup.bottom() <= frame.bottom()); + } + } +} diff --git a/cbd-tui/src/app/mod.rs b/cbd-tui/src/app/mod.rs index 7edeb80..014d2fb 100644 --- a/cbd-tui/src/app/mod.rs +++ b/cbd-tui/src/app/mod.rs @@ -1,3 +1,5 @@ +pub mod bindings; +mod help; mod library; mod list; mod now_playing; @@ -16,6 +18,7 @@ use crabidy_core::proto::crabidy::{ pub use list::StatefulList; +use bindings::Action; use library::Library; use now_playing::NowPlaying; use queue::Queue; @@ -76,26 +79,112 @@ pub enum MessageFromUi { ToggleRepeat, } +/// What the event loop should do after dispatching an action. +/// +/// Quitting stays a loop-level concern: `App::dispatch` never tears down the +/// terminal itself, it only reports that the loop should end. +#[must_use] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DispatchResult { + Continue, + Quit, +} + pub struct App { pub focus: UiFocus, + /// Whether the help modal is open. While true, `bindings::lookup` only + /// admits `Scope::Help` actions and `render` draws the overlay last. + pub show_help: bool, pub library: Library, pub now_playing: NowPlaying, pub queue: Queue, + tx: Sender, } impl App { pub fn new(tx: Sender) -> App { let library = Library::new(tx.clone()); - let queue = Queue::new(tx); + let queue = Queue::new(tx.clone()); let now_playing = NowPlaying::default(); App { focus: UiFocus::Library, + show_help: false, library, now_playing, queue, + tx, } } + /// Execute one [`Action`] against the app: mutate UI state (focus, help + /// modal, list selections) or send the matching [`MessageFromUi`] via + /// `tx`. Send failures are ignored like everywhere else in this module — + /// the orchestrator side owns error reporting. + pub fn dispatch(&mut self, action: Action) -> DispatchResult { + match action { + Action::Quit => return DispatchResult::Quit, + Action::OpenHelp => self.show_help = true, + Action::CloseHelp => self.show_help = false, + Action::CycleFocus => self.cycle_active(), + Action::TogglePlay => { + let _ = self.tx.send(MessageFromUi::TogglePlay); + } + Action::RestartTrack => { + let _ = self.tx.send(MessageFromUi::RestartTrack); + } + Action::VolumeUp => { + let _ = self.tx.send(MessageFromUi::ChangeVolume(0.1)); + } + Action::VolumeDown => { + let _ = self.tx.send(MessageFromUi::ChangeVolume(-0.1)); + } + Action::ToggleMute => { + let _ = self.tx.send(MessageFromUi::ToggleMute); + } + Action::ToggleShuffle => { + let _ = self.tx.send(MessageFromUi::ToggleShuffle); + } + Action::ToggleRepeat => { + let _ = self.tx.send(MessageFromUi::ToggleRepeat); + } + Action::NextTrack => self.queue.play_next(), + Action::PrevTrack => self.queue.play_prev(), + Action::LibraryFirst => self.library.first(), + Action::LibraryLast => self.library.last(), + Action::LibraryNext => self.library.next(), + Action::LibraryPrev => self.library.prev(), + Action::LibraryJumpDown => self.library.down(), + Action::LibraryJumpUp => self.library.up(), + Action::LibraryAscend => self.library.ascend(), + Action::LibraryDive => self.library.dive(), + Action::LibraryQueueNext => self.library.queue_queue(), + Action::LibraryQueueAppend => self.library.queue_append(), + Action::LibraryQueueReplace => self.library.queue_replace(), + Action::LibraryToggleMark => self.library.toggle_mark(), + Action::QueueInsertHere => { + if let Some(selected) = self.queue.selected() { + self.library.queue_insert(selected); + } + } + Action::QueueFirst => self.queue.first(), + Action::QueueLast => self.queue.last(), + Action::QueueNext => self.queue.next(), + Action::QueuePrev => self.queue.prev(), + Action::QueueJumpDown => self.queue.down(), + Action::QueueJumpUp => self.queue.up(), + Action::QueueSelectCurrent => self.queue.select_current(), + Action::QueuePlaySelected => self.queue.play_selected(), + Action::QueueRemoveTrack => self.queue.remove_track(), + Action::QueueClearKeepCurrent => { + let _ = self.tx.send(MessageFromUi::ClearQueue(true)); + } + Action::QueueClearAll => { + let _ = self.tx.send(MessageFromUi::ClearQueue(false)); + } + } + DispatchResult::Continue + } + pub fn cycle_active(&mut self) { self.focus = match (self.focus, self.queue.is_empty()) { (UiFocus::Library, false) => UiFocus::Queue, @@ -124,5 +213,90 @@ impl App { self.queue.render(f, right_side[0], queue_focused); self.now_playing.render(f, right_side[1]); + + // The help modal renders last so it overlays every pane. + if self.show_help { + help::render(f); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use flume::Receiver; + + fn app() -> (App, Receiver) { + let (tx, rx) = flume::unbounded(); + (App::new(tx), rx) + } + + #[test] + fn quit_is_reported_to_the_loop_not_executed() { + let (mut app, _rx) = app(); + assert_eq!(app.dispatch(Action::Quit), DispatchResult::Quit); + } + + #[test] + fn open_and_close_help_toggle_the_flag() { + let (mut app, _rx) = app(); + assert!(!app.show_help); + assert_eq!(app.dispatch(Action::OpenHelp), DispatchResult::Continue); + assert!(app.show_help); + assert_eq!(app.dispatch(Action::CloseHelp), DispatchResult::Continue); + assert!(!app.show_help); + } + + #[test] + fn cycle_focus_respects_empty_queue() { + let (mut app, _rx) = app(); + // The queue starts empty, so focus must stay on the library. + assert_eq!(app.dispatch(Action::CycleFocus), DispatchResult::Continue); + assert!(matches!(app.focus, UiFocus::Library)); + } + + #[test] + fn playback_actions_send_the_matching_message() { + let (mut app, rx) = app(); + + let _ = app.dispatch(Action::TogglePlay); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::TogglePlay))); + + let _ = app.dispatch(Action::RestartTrack); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::RestartTrack))); + + let _ = app.dispatch(Action::ToggleMute); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ToggleMute))); + + let _ = app.dispatch(Action::ToggleShuffle); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ToggleShuffle))); + + let _ = app.dispatch(Action::ToggleRepeat); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ToggleRepeat))); + } + + #[test] + fn volume_actions_send_signed_deltas() { + let (mut app, rx) = app(); + + let _ = app.dispatch(Action::VolumeUp); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ChangeVolume(d)) if d > 0.0)); + + let _ = app.dispatch(Action::VolumeDown); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ChangeVolume(d)) if d < 0.0)); + } + + #[test] + fn queue_clear_actions_carry_the_keep_current_flag() { + let (mut app, rx) = app(); + + let _ = app.dispatch(Action::QueueClearKeepCurrent); + assert!(matches!(rx.try_recv(), Ok(MessageFromUi::ClearQueue(true)))); + + let _ = app.dispatch(Action::QueueClearAll); + assert!(matches!( + rx.try_recv(), + Ok(MessageFromUi::ClearQueue(false)) + )); } } diff --git a/cbd-tui/src/main.rs b/cbd-tui/src/main.rs index 381410e..81ac853 100644 --- a/cbd-tui/src/main.rs +++ b/cbd-tui/src/main.rs @@ -12,9 +12,7 @@ use std::{ use crabidy_core::proto::crabidy::{get_update_stream_response::Update as StreamUpdate, PlayState}; use crossterm::{ - event::{ - self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEventKind, KeyModifiers, - }, + event::{self, DisableMouseCapture, EnableMouseCapture, Event, KeyEventKind}, execute, terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen}, }; @@ -24,7 +22,7 @@ use ratatui::{backend::CrosstermBackend, Terminal}; use tokio::select; use tokio_stream::StreamExt; -use app::{App, MessageFromUi, MessageToUi, StatefulList, UiFocus}; +use app::{bindings, App, DispatchResult, MessageFromUi, MessageToUi}; use config::Config; use rpc::RpcClient; use tracing::{error, info, warn}; @@ -193,7 +191,7 @@ fn run_ui(tx: Sender, rx: Receiver) { let mut terminal = Terminal::new(backend).unwrap(); // create app and run it - let mut app = App::new(tx.clone()); + let mut app = App::new(tx); let tick_rate = Duration::from_millis(100); let mut last_tick = Instant::now(); @@ -253,113 +251,10 @@ fn run_ui(tx: Sender, rx: Receiver) { if event::poll(timeout).unwrap() { if let Event::Key(key) = event::read().unwrap() { if key.kind == KeyEventKind::Press { - match (app.focus, key.modifiers, key.code) { - (_, KeyModifiers::NONE, KeyCode::Char('q')) => { + if let Some(action) = bindings::lookup(app.focus, app.show_help, key) { + if app.dispatch(action) == DispatchResult::Quit { break; } - (_, KeyModifiers::NONE, KeyCode::Tab) => app.cycle_active(), - (_, KeyModifiers::NONE, KeyCode::Char(' ')) => { - let _ = tx.send(MessageFromUi::TogglePlay); - } - (_, KeyModifiers::NONE, KeyCode::Char('r')) => { - let _ = tx.send(MessageFromUi::RestartTrack); - } - (_, KeyModifiers::SHIFT, KeyCode::Char('J')) => { - let _ = tx.send(MessageFromUi::ChangeVolume(-0.1)); - } - (_, KeyModifiers::SHIFT, KeyCode::Char('K')) => { - let _ = tx.send(MessageFromUi::ChangeVolume(0.1)); - } - (_, KeyModifiers::NONE, KeyCode::Char('m')) => { - let _ = tx.send(MessageFromUi::ToggleMute); - } - (_, KeyModifiers::NONE, KeyCode::Char('z')) => { - let _ = tx.send(MessageFromUi::ToggleShuffle); - } - (_, KeyModifiers::NONE, KeyCode::Char('x')) => { - let _ = tx.send(MessageFromUi::ToggleRepeat); - } - (_, KeyModifiers::CONTROL, KeyCode::Char('n')) => { - app.queue.play_next(); - } - (_, KeyModifiers::CONTROL, KeyCode::Char('p')) => { - app.queue.play_prev(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('g')) => { - app.library.first(); - } - (UiFocus::Library, KeyModifiers::SHIFT, KeyCode::Char('G')) => { - app.library.last(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('j')) => { - app.library.next(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('k')) => { - app.library.prev(); - } - (UiFocus::Library, KeyModifiers::CONTROL, KeyCode::Char('d')) => { - app.library.down(); - } - (UiFocus::Library, KeyModifiers::CONTROL, KeyCode::Char('u')) => { - app.library.up(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('h')) => { - app.library.ascend(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('l')) => { - app.library.dive(); - } - (UiFocus::Library, KeyModifiers::SHIFT, KeyCode::Char('L')) => { - app.library.queue_queue(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('a')) => { - app.library.queue_append(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Enter) => { - app.library.queue_replace(); - } - (UiFocus::Library, KeyModifiers::NONE, KeyCode::Char('s')) => { - app.library.toggle_mark(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('p')) => { - if let Some(selected) = app.queue.selected() { - app.library.queue_insert(selected); - } - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('g')) => { - app.queue.first(); - } - (UiFocus::Queue, KeyModifiers::SHIFT, KeyCode::Char('G')) => { - app.queue.last(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('j')) => { - app.queue.next(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('k')) => { - app.queue.prev(); - } - (UiFocus::Queue, KeyModifiers::CONTROL, KeyCode::Char('d')) => { - app.queue.down(); - } - (UiFocus::Queue, KeyModifiers::CONTROL, KeyCode::Char('u')) => { - app.queue.up(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('o')) => { - app.queue.select_current(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Enter) => { - app.queue.play_selected(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('d')) => { - app.queue.remove_track(); - } - (UiFocus::Queue, KeyModifiers::NONE, KeyCode::Char('c')) => { - let _ = tx.send(MessageFromUi::ClearQueue(true)); - } - (UiFocus::Queue, KeyModifiers::SHIFT, KeyCode::Char('C')) => { - let _ = tx.send(MessageFromUi::ClearQueue(false)); - } - _ => {} } } } diff --git a/plan/help-modal.md b/plan/help-modal.md new file mode 100644 index 0000000..ed040a6 --- /dev/null +++ b/plan/help-modal.md @@ -0,0 +1,70 @@ +# Plan — help modal (cbd-tui) + +Ordered tasks for the `implement` stage. Inputs: `architecture/help-modal.md`, +stubs in `cbd-tui/src/app/{bindings,help,mod}.rs`, gates in +`quality/help-modal.md`. Run tests with `devenv shell -- cargo test -p cbd-tui`. + +## 1. Binding lookup + +- [x] Implement `bindings::lookup`: scan `BINDINGS` in order; when `help_open` + only `Scope::Help` entries are eligible, otherwise `Scope::Global` plus + the scope matching `focus`. Compare chords with `SHIFT` ignored for + `KeyCode::Char` codes, exact modifiers otherwise. Return the first match. + **Verify**: all `app::bindings::tests::*lookup*`-related tests pass + (`global_bindings_match_in_any_focus`, + `pane_bindings_only_match_their_own_pane`, `same_chord_resolves_per_pane`, + `shift_is_ignored_for_char_codes`, `control_must_match_exactly`, + `open_help_swallows_everything_but_close`, + `help_scope_never_matches_while_help_is_closed`). +- [x] Implement `bindings::key_label`: `Char(' ')` → `"Space"`, other chars → + the char itself, `Tab`/`Enter`/`Esc` named, `CONTROL` prefix `"Ctrl+"`; + no panic on any input (fall back to `Debug`-ish formatting for unbound + codes). **Verify**: `key_labels_are_human_readable` passes; gate "no + panics on user input". + +## 2. Action dispatch + +- [x] Implement `App::dispatch` as one `match action` reproducing, arm for + arm, the behavior of the old key match in `main.rs` (send via `self.tx` + with `let _ =`, or call `self.library`/`self.queue` methods). + `OpenHelp`/`CloseHelp` set `show_help`; `Quit` returns + `DispatchResult::Quit`; everything else `Continue`. + **Verify**: all `app::tests::*` pass; gate "behavior parity" (cross-check + against the pre-change `main.rs` match arm by arm). + +## 3. Wire the event loop + +- [x] Replace the key match in `main.rs run_ui` with: on key press, + `bindings::lookup(app.focus, app.show_help, key)` then + `app.dispatch(action)`, breaking the loop on `DispatchResult::Quit`. + Keep the `KeyEventKind::Press` filter. Remove the now-dead imports + (`KeyCode`, `KeyModifiers`, `UiFocus`, `MessageFromUi` uses that move + into `dispatch`). **Verify**: `cargo check` clean, no key handling left + in `main.rs` (gate "single source of truth"); manual smoke via `?`. + +## 4. Help modal rendering + +- [x] Implement `help::popup_area`: content-sized centered `Rect` clamped to + the frame. **Verify**: `popup_never_exceeds_the_frame`. +- [x] Implement `help::render`: `Clear` the popup area, draw a bordered block + (style matching the panes: rounded borders, `COLOR_PRIMARY`), a short + usage paragraph naming the Library and Queue panes and `Tab` to switch, + then `BINDINGS` grouped by scope in table order with labels from + `key_label`. Truncate lines that don't fit; never panic on tiny frames. + **Verify**: `help_lists_bindings_from_the_table`, + `help_explains_basic_usage`, `help_survives_tiny_terminals` pass; gates + "modality" (overlay drawn last, `Clear` used) and "no panics". + +## 5. Polish and gates + +- [x] Resolve the `TODO(api-design)` on `LibraryQueueNext`: check what + `MessageFromUi::QueueTracks` does in `crabidy-server` and fix the + description text if needed. **Verify**: gate "TODO resolved". +- [x] Sweep: doc comments still accurate, no dead-code warnings left for + `bindings`/`help`, no new dependencies. + **Verify**: `devenv shell -- cargo fmt --check`, + `devenv shell -- cargo clippy -p cbd-tui` (no new warnings), + `devenv shell -- cargo test -p cbd-tui` all green; every box in + `quality/help-modal.md` checked. +- [x] Write `plan/summary.md` (or a `help-modal` section in it) recording any + deviations from this plan. diff --git a/plan/summary.md b/plan/summary.md new file mode 100644 index 0000000..a08a4db --- /dev/null +++ b/plan/summary.md @@ -0,0 +1,28 @@ +# Implementation summaries + +## help-modal (2026-07-20) + +Built per `plan/help-modal.md`: `app/bindings.rs` (declarative +`BINDINGS` table + `lookup` + `key_label`), `app/help.rs` (overlay), the +`App::dispatch`/`DispatchResult` seam, and the rewired event loop in +`main.rs`. All 20 tests pass; every gate in `quality/help-modal.md` checked. + +### Deviations from plan / architecture + +- **Two-column modal layout.** The architecture assumed a single-column list; + the full table is ~50 rows and would not fit even a 100×40 frame. The modal + renders Global in the left column and Library + Queue stacked in the right + column, with the close keys as a footer line (`Close help: ?, Esc, q`) + derived from the `Scope::Help` bindings instead of a fourth listed group. + The open question "scroll vs truncate" stays resolved as truncate — but + after the column split the content fits ~34×94, so truncation only kicks in + on genuinely small terminals. +- **`Scope` derives `Hash`** (not in the stub) so the chord-uniqueness test + can use a `HashSet`. +- **`QueueInsertHere` description reworded** to "Insert library selection + after this track": `crabidy-server`'s `insert_tracks` splices at + `position + 1`. Same check confirmed the planned "Queue selection after + current track" wording for `LibraryQueueNext`. +- **`main.rs`** passes `tx` to `App::new` without the now-unneeded clone; the + `KeyCode`/`KeyModifiers`/`UiFocus`/`StatefulList` imports moved out with the + old match. diff --git a/quality/help-modal.md b/quality/help-modal.md new file mode 100644 index 0000000..e36f5b0 --- /dev/null +++ b/quality/help-modal.md @@ -0,0 +1,57 @@ +# Quality gates — help modal (cbd-tui) + +Checklist for the `implement` stage. Each gate is pass/fail and verified by +reading the code (the automatic tests live in +`cbd-tui/src/app/{bindings,help,mod}.rs` `#[cfg(test)]` modules and must pass +via `devenv shell -- cargo test -p cbd-tui`). + +## Single source of truth + +- [x] `bindings::BINDINGS` is the only encoding of key → action → description. + `help.rs` contains no hardcoded key names or binding descriptions other + than the usage paragraph; key labels come from `bindings::key_label`. +- [x] The old `match (app.focus, key.modifiers, key.code)` in `main.rs` is + fully replaced by `bindings::lookup` + `App::dispatch`. No key handling + remains in `main.rs` besides translating `KeyEvent` → `Action` and + honoring `DispatchResult::Quit`. +- [x] Behavior parity: every binding that existed before the refactor + (main.rs match arms at the pre-change commit) maps to a `BINDINGS` entry + triggering the same underlying call. Cross-check arm by arm. + +## Modality + +- [x] While `show_help` is true, only `Scope::Help` entries dispatch; `q` + closes the modal and does not quit the app. +- [x] `Scope::Help` entries never dispatch while the modal is closed. +- [x] The overlay is drawn last in `App::render` and uses ratatui's `Clear` + before drawing the popup, so pane content never bleeds through. + +## No panics on user input (AGENTS.md hard rule) + +- [x] All `todo!()` stubs from api-design are gone from `bindings.rs`, + `help.rs`, and `App::dispatch`. +- [x] `lookup`, `key_label`, `dispatch`, and `help::render` cannot panic for + any `KeyEvent` or any frame size (including 1×1); no `unwrap`/`expect` + /indexing on user-driven paths. Unknown keys are ignored (`None`), not + errors. +- [x] Channel send failures in `dispatch` are ignored (`let _ =`), matching + the existing module convention — no `unwrap` on `tx.send`. + +## Robust key matching + +- [x] `lookup` ignores `SHIFT` when comparing `KeyCode::Char` chords and + requires exact matches for all other modifiers (`Ctrl+n` ≠ `n`). +- [x] Key events with `kind != KeyEventKind::Press` are still filtered out + before lookup (existing behavior preserved). + +## Code quality + +- [x] Public items in `bindings.rs` and `help.rs` keep doc comments that match + the implemented behavior (update them if implementation details shift). +- [x] No new external dependencies in `cbd-tui/Cargo.toml`. +- [x] `devenv shell -- cargo fmt --check`, `cargo clippy` (no new warnings, + no dead-code warnings remaining for `bindings`/`help`), and + `cargo test -p cbd-tui` all pass. +- [x] The `TODO(api-design)` marker on the `LibraryQueueNext` description is + resolved: the wording matches what `MessageFromUi::QueueTracks` actually + does server-side.