diff --git a/architecture/seek.md b/architecture/seek.md index dafa596..b3dd835 100644 --- a/architecture/seek.md +++ b/architecture/seek.md @@ -22,8 +22,8 @@ Goal: a step of about 15 seconds forward and backward, from every client. Stated explicitly and settled by reading the code rather than by asking: - **A1 — Within the current track only.** A backward seek at 3 s lands at - 0, it does not step into the previous track; `Ctrl-p` is the - track-level control and stays that way. Crossing tracks would need the + 0, it does not step into the previous track; `<` / `Ctrl-p` are the + track-level controls and stay that way. Crossing tracks would need the queue lock and the previous track's duration, for a gesture nobody expects to do that. - **A2 — The server owns the position.** Clients render `TrackPosition` @@ -49,8 +49,8 @@ position. received, adds ±15 s, clamps against the duration it was told, and sends an absolute target. -- The wire is a plain absolute seek, which a click-on-the-progress-bar - gesture also wants. +- The wire is a plain absolute seek, which a click on the progress bar + maps onto directly. (That turned out not to need it either — see D10.) - But the base is **stale**: positions are broadcast on a 250 ms tick and then cross the network. The step is 15 s, so 250 ms of drift is not the problem — **repetition** is. Press `.` three times quickly and all @@ -76,8 +76,8 @@ position, clamps, and seeks. - Clamping policy lives in one place, next to the duration the engine already tracks. - Clients get simpler, not more complex: a constant and one RPC call. -- Cost: absolute seek is not on the wire. It is a compatible proto3 - addition when a click-to-seek gauge wants it (deferred, below). +- Cost: absolute seek is not on the wire. A compatible proto3 addition if + something ever needs it; click-to-seek did not (D10). **Decision: Option B.** The composition argument decides it — Option A is wrong in the ordinary case of pressing the key twice. @@ -124,23 +124,47 @@ wrong in the ordinary case of pressing the key twice. they never moved it (A2). Consistent with how `pause` and `set_volume` failures are handled — no new status codes, no new client-side error path. -- **D8 — Bindings: `Ctrl-b` back, `Ctrl-f` forward**, in the **global** - scope of both the TUI and the web client (the user's choice; an earlier - round used `,`/`.`). They join the existing control-chord family for - playback and movement — `Ctrl-n`/`Ctrl-p` for tracks, `Ctrl-d`/`Ctrl-u` - for paging — and read as vim's back/forward-a-screen pair. Neither chord - was bound in any scope of either client, and plain `f` (spectrum) is - untouched because the TUI's `lookup` compares every modifier except - `SHIFT` exactly. In the browser, `Ctrl-f` would otherwise open the find - bar; the web keydown handler already calls `prevent_default` on any - chord that resolves to an action, so binding it is enough to claim it. - The web transport bar also gets `⏪`/`⏩` buttons, and the CLI gets - `cbd global seek `, which accepts negatives. +- **D8 — Bindings: two physical keys carry all four moves.** `,`/`.` seek + 15 seconds back/forward and their shifted forms `<`/`>` skip a whole + track, in the **global** scope of both clients. The pair is + self-teaching (same key, shift = bigger jump), `<`/`>` are the marks + engraved on those keys, and they match mpv's playlist controls. Settled + after two rounds: `,`/`.`, then `Ctrl-b`/`Ctrl-f` at the user's request, + then back once `<`/`>` earned their place for a separate reason (below). + + The deciding property is that these are **plain printable characters**, + so they collide with nothing a browser reserves. `Ctrl-n` — the + long-standing "next track" — cannot be claimed in a browser at all: + Chrome and Firefox handle it as "new window" above the page, where + `preventDefault` cannot reach, unlike `Ctrl-f`, `Ctrl-p` or `Ctrl-b`. + Reaching for the reserved-chord list is a trap the alphabetic keys + avoid entirely. `Ctrl-n`/`Ctrl-p` stay bound as the terminal's primary + chords (and `Ctrl-p` works in the browser too); the web help documents + `<`/`>`, the ones that always work. + - **D9 — No feature flag.** Seek is a few lines in the engine and one RPC arm; it carries no dependency of its own, so by the rule in `architecture/build-features.md` ("a feature must pay for itself in dependencies") it does not earn one. +- **D10 — The web progress bar is clickable, still as an offset.** A + click maps the pointer's x within the gauge to a fraction of the + duration and sends `target - position`. Relative is *right* here even + though the gesture is absolute: the position it subtracts is the one + drawn on the bar the user just aimed at, and it is at most one 250 ms + tick stale — far under one pixel of the bar for any track worth seeking + in. (The same staleness is fatal for a repeated *key*, which is why + keys send a fixed step; see the Options section.) So click-to-seek + needs no absolute field on the wire after all. The arithmetic lives in + `state.rs` as a pure function, so it is unit-tested on the native + target rather than only in a browser; geometry comes from + `current_target` because the click may land on the fill rather than the + track; a duration of 0 declines the click, since a bar with no scale + has no position to click at. + + The web transport bar also gets `⏪`/`⏩` buttons, and the CLI gets + `cbd global seek `, which accepts negatives. + ## Structure ```d2 @@ -215,9 +239,10 @@ the only place that knows the live sink position and the track duration. Recorded, not dropped: -- **Absolute seek** (`position_millis`) and a click-to-seek progress - gauge in the web client. The gauge is already rendered; only the wire - field and a click handler are missing. A compatible proto3 addition. +- **Absolute seek** (`position_millis`). Click-to-seek shipped without it + (D10), so the only remaining use would be a client that wants to name a + position without knowing the current one. A compatible proto3 addition + if that ever appears. - **Configurable step size** in the client configs, and a larger step on `<`/`>` (same physical keys, shifted). Client-only once wanted. - **Chapter-aware seek** for podcasts and audiobooks. No provider exposes diff --git a/cbd-tui/src/app/bindings.rs b/cbd-tui/src/app/bindings.rs index e20994e..ddd101b 100644 --- a/cbd-tui/src/app/bindings.rs +++ b/cbd-tui/src/app/bindings.rs @@ -237,20 +237,39 @@ pub const BINDINGS: &[Binding] = &[ action: Action::PrevTrack, description: "Previous track", }, + // Two physical keys carry all four moves: unshifted seeks 15 seconds, + // shifted skips a whole track, and `<`/`>` are the marks engraved on them. + // They also give the browser client working track-skip keys, which it + // otherwise lacks — Chrome and Firefox reserve `Ctrl-n` for "new window" + // at a level `preventDefault` cannot reach. Binding { scope: Scope::Global, - mods: KeyModifiers::CONTROL, - code: KeyCode::Char('b'), + mods: KeyModifiers::NONE, + code: KeyCode::Char(','), action: Action::SeekBackward, description: "Seek back 15 seconds", }, Binding { scope: Scope::Global, - mods: KeyModifiers::CONTROL, - code: KeyCode::Char('f'), + mods: KeyModifiers::NONE, + code: KeyCode::Char('.'), action: Action::SeekForward, description: "Seek forward 15 seconds", }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('<'), + action: Action::PrevTrack, + description: "Previous track", + }, + Binding { + scope: Scope::Global, + mods: KeyModifiers::SHIFT, + code: KeyCode::Char('>'), + action: Action::NextTrack, + description: "Next track", + }, Binding { scope: Scope::Global, mods: KeyModifiers::NONE, @@ -880,28 +899,34 @@ mod tests { ); } - /// Seek rides the control chords in every focus, and taking `Ctrl-f` must - /// not disturb plain `f` — the modifier comparison is exact for anything - /// but `SHIFT`. + /// Two physical keys carry all four moves in every focus: unshifted seeks + /// 15 seconds, shifted skips a whole track. The shifted forms must resolve + /// whether or not the terminal reports `SHIFT` (they are `Char` codes). #[test] - fn seek_is_on_the_control_chords_in_any_focus() { + fn seek_and_skip_share_two_keys_in_any_focus() { for focus in [UiFocus::Library, UiFocus::Queue] { assert_eq!( - lookup(focus, false, key(KeyCode::Char('f'), KeyModifiers::CONTROL)), - Some(Action::SeekForward) - ); - assert_eq!( - lookup(focus, false, key(KeyCode::Char('b'), KeyModifiers::CONTROL)), + lookup(focus, false, key(KeyCode::Char(','), KeyModifiers::NONE)), Some(Action::SeekBackward) ); - // Unmodified, these keep their old meanings (or none at all). assert_eq!( - lookup(focus, false, key(KeyCode::Char('f'), KeyModifiers::NONE)), - Some(Action::ToggleSpectrum) + lookup(focus, false, key(KeyCode::Char('.'), KeyModifiers::NONE)), + Some(Action::SeekForward) ); + for mods in [KeyModifiers::NONE, KeyModifiers::SHIFT] { + assert_eq!( + lookup(focus, false, key(KeyCode::Char('<'), mods)), + Some(Action::PrevTrack) + ); + assert_eq!( + lookup(focus, false, key(KeyCode::Char('>'), mods)), + Some(Action::NextTrack) + ); + } + // The long-standing control chords still work in a terminal. assert_eq!( - lookup(focus, false, key(KeyCode::Char('b'), KeyModifiers::NONE)), - None + lookup(focus, false, key(KeyCode::Char('n'), KeyModifiers::CONTROL)), + Some(Action::NextTrack) ); } // And the help modal still swallows them. @@ -909,7 +934,7 @@ mod tests { lookup( UiFocus::Library, true, - key(KeyCode::Char('f'), KeyModifiers::CONTROL) + key(KeyCode::Char('.'), KeyModifiers::NONE) ), None ); diff --git a/cbd-web/Cargo.toml b/cbd-web/Cargo.toml index 9c18394..9bd08c5 100644 --- a/cbd-web/Cargo.toml +++ b/cbd-web/Cargo.toml @@ -20,6 +20,10 @@ wasm-bindgen.workspace = true wasm-bindgen-futures.workspace = true web-sys = { workspace = true, features = [ "Document", + # `DomRect` is what `Element::get_bounding_client_rect` returns; the web-sys + # method only exists with the feature on. Click-to-seek needs the gauge's + # geometry to turn a click into a position. + "DomRect", "Element", "HtmlInputElement", "KeyboardEvent", diff --git a/cbd-web/src/app.rs b/cbd-web/src/app.rs index ec5ac0e..075120c 100644 --- a/cbd-web/src/app.rs +++ b/cbd-web/src/app.rs @@ -15,8 +15,8 @@ use leptos::task::spawn_local; use crate::keymap::{self, Action}; use crate::rpc::Rpc; use crate::state::{ - format_seconds, is_cacheable, track_label, CaptureBoard, Dialog, Focus, LibraryPane, - NamePurpose, QueueCursor, Register, UiItemKind, + format_seconds, is_cacheable, seek_offset_for_fraction, track_label, CaptureBoard, Dialog, + Focus, LibraryPane, NamePurpose, QueueCursor, Register, UiItemKind, }; const VOLUME_STEP: f32 = 0.1; @@ -1151,6 +1151,32 @@ fn Transport(store: Store) -> impl IntoView { PlayState::Loading => "…", _ => "▶", }; + // Click-to-seek on the progress bar. The click may land on the fill rather + // than the track, so the geometry comes from `current_target` — always the + // element the listener is on — not from the event target. + let on_seek = move |ev: leptos::ev::MouseEvent| { + use wasm_bindgen::JsCast; + let Some(gauge) = ev + .current_target() + .and_then(|target| target.dyn_into::().ok()) + else { + return; + }; + let rect = gauge.get_bounding_client_rect(); + if rect.width() <= 0.0 { + return; + } + let fraction = (f64::from(ev.client_x()) - rect.left()) / rect.width(); + let position = store.position.get_untracked(); + let Some(delta) = seek_offset_for_fraction(position.position, position.duration, fraction) + else { + // No duration: the bar has no scale, so the click has no meaning. + return; + }; + if delta != 0 { + store.call(async move |mut rpc: Rpc| rpc.seek(delta).await); + } + }; let on_volume = move |ev: leptos::ev::Event| { if let Ok(target) = event_target_value(&ev).parse::() { let delta = target - store.volume.get_untracked(); @@ -1160,15 +1186,15 @@ fn Transport(store: Store) -> impl IntoView { view! {