diff --git a/CHANGELOG.md b/CHANGELOG.md index 05c2f3e..ab7bbac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [Unreleased] + +### Added + +- **Click and drag to seek on the playbar**: The progress bar is now interactive. Click anywhere on the gauge to jump to that position, or click and drag to scrub. Control buttons keep priority, the time label stays non-clickable, and seeks reuse the existing native and throttled-API paths ([#157](https://github.com/LargeModGames/spotatui/issues/157)). + +### Fixed + +- **Search box no longer traps focus on submit**: Pressing `Enter` to run a search now always moves focus to the results list, including when re-searching while already on the Search screen (previously focus stayed stuck in the input box) ([#191](https://github.com/LargeModGames/spotatui/issues/191)). +- **Cover-art load failures are non-fatal**: A failed album-image fetch is now logged and ignored instead of surfacing a blocking error and aborting the now-playing update, so playback metadata keeps updating when artwork can't be loaded ([#142](https://github.com/LargeModGames/spotatui/issues/142)). + ## [v0.38.6] 2026-05-28 ### Fixed diff --git a/src/core/app.rs b/src/core/app.rs index ea7daed..06ec654 100644 --- a/src/core/app.rs +++ b/src/core/app.rs @@ -1758,6 +1758,50 @@ impl App { self.queue_api_seek(new_progress); } + /// Seek to an absolute position within the current track (e.g. from clicking or + /// dragging on the playbar progress line). The target is clamped to the track + /// duration. Mirrors the dispatch logic of [`Self::seek_forwards`]. + pub fn seek_to(&mut self, position_ms: u32) { + if let Some(CurrentPlaybackContext { + item: Some(item), .. + }) = &self.current_playback_context + { + let duration_ms = match item { + PlayableItem::Track(track) => track.duration.num_milliseconds() as u32, + PlayableItem::Episode(episode) => episode.duration.num_milliseconds() as u32, + _ => return, + }; + + let new_progress = position_ms.min(duration_ms); + self.seek_ms = Some(new_progress as u128); + + // Use native streaming player for instant control (bypasses event channel latency) + #[cfg(feature = "streaming")] + if self.is_native_streaming_active_for_playback() && self.streaming_player.is_some() { + // Always update UI immediately + self.song_progress_ms = new_progress as u128; + self.seek_ms = None; + + // Throttle actual seeks to avoid overwhelming librespot (max ~20/sec) + const SEEK_THROTTLE_MS: u128 = 50; + let should_seek_now = self + .last_native_seek + .is_none_or(|t| t.elapsed().as_millis() >= SEEK_THROTTLE_MS); + + if should_seek_now { + self.execute_native_seek(new_progress); + } else { + // Queue the seek - will be flushed by tick loop or next seek + self.pending_native_seek = Some(new_progress); + } + return; + } + + // Fallback: API-based seek for external devices (with throttling) + self.queue_api_seek(new_progress); + } + } + /// Queue an API-based seek with throttling (for external device control) fn queue_api_seek(&mut self, position_ms: u32) { // Always update UI immediately diff --git a/src/infra/network/playback.rs b/src/infra/network/playback.rs index b971f66..bb15fd9 100644 --- a/src/infra/network/playback.rs +++ b/src/infra/network/playback.rs @@ -485,10 +485,10 @@ impl PlaybackNetwork for Network { }; if let Some(image) = image { - if let anyhow::Result::Err(err) = app.cover_art.refresh(image).await { - drop(app); - self.handle_error(err).await; - return; + // Cover art is non-essential: a failed image fetch must not surface a + // blocking error or abort the rest of the playback-context update (#142). + if let Err(err) = app.cover_art.refresh(image).await { + log::warn!("ignoring cover art load failure: {err}"); } } } diff --git a/src/tui/handlers/input.rs b/src/tui/handlers/input.rs index 0fd8385..2f78697 100644 --- a/src/tui/handlers/input.rs +++ b/src/tui/handlers/input.rs @@ -116,6 +116,13 @@ fn process_input(app: &mut App, input: String) { // Default fallback behavior: treat the input as a raw search phrase. app.dispatch(IoEvent::GetSearchResults(input, app.get_user_country())); app.push_navigation_stack(RouteId::Search, ActiveBlock::SearchResultBlock); + // push_navigation_stack is a no-op when the Search route is already on top, which + // otherwise leaves focus trapped in the input box. Force focus onto the results so + // submitting a search always escapes the search box (#191). + app.set_current_route_state( + Some(ActiveBlock::SearchResultBlock), + Some(ActiveBlock::SearchResultBlock), + ); } fn process_playlist_track_search(app: &mut App, input: String) { @@ -389,6 +396,34 @@ mod tests { assert!(rx.try_recv().is_err()); } + #[test] + fn search_enter_on_search_route_moves_focus_to_results() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut app = App::new( + tx, + crate::core::user_config::UserConfig::new(), + std::time::SystemTime::now(), + ); + + // Already on the Search route with focus in the input box. This is the case + // that previously left focus trapped in the search box on submit (#191). + app.push_navigation_stack(RouteId::Search, ActiveBlock::Input); + app.input = str_to_vec_char("daft punk"); + app.input_idx = app.input.len(); + app.input_cursor_position = app.input.len() as u16; + + handler(Key::Enter, &mut app); + + match rx.recv().unwrap() { + IoEvent::GetSearchResults(query, _) => assert_eq!(query, "daft punk"), + _ => panic!("unexpected event"), + } + + let route = app.get_current_route(); + assert_eq!(route.active_block, ActiveBlock::SearchResultBlock); + assert_eq!(route.hovered_block, ActiveBlock::SearchResultBlock); + } + #[test] fn test_input_handler_on_enter_text() { let mut app = App::default(); diff --git a/src/tui/handlers/mouse.rs b/src/tui/handlers/mouse.rs index c45a308..8e73ca9 100644 --- a/src/tui/handlers/mouse.rs +++ b/src/tui/handlers/mouse.rs @@ -7,7 +7,7 @@ use crate::core::layout::{ sidebar_constraints, }; use crate::tui::event::Key; -use crate::tui::ui::player::playbar_control_at; +use crate::tui::ui::player::{playbar_control_at, playbar_progress_line}; use crate::tui::ui::tables::table_scroll_offset; use crate::tui::ui::util::{get_main_layout_margin, SMALL_TERMINAL_WIDTH}; use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; @@ -227,16 +227,34 @@ fn handle_playbar_mouse( focus_block: ActiveBlock, app: &mut App, ) { - if !matches!(mouse.kind, MouseEventKind::Down(MouseButton::Left)) { - return; - } - - let Some(control) = playbar_control_at(app, playbar_area, mouse.column, mouse.row) else { - return; - }; + match mouse.kind { + MouseEventKind::Down(MouseButton::Left) => { + // Control buttons take precedence; they sit on a different row than the gauge. + if let Some(control) = playbar_control_at(app, playbar_area, mouse.column, mouse.row) { + focus_playbar(focus_block, app); + playbar::handle_control(control, app); + return; + } - focus_playbar(focus_block, app); - playbar::handle_control(control, app); + // Otherwise, a click on the progress line seeks to that position (#157). + if let Some(line) = playbar_progress_line(app, playbar_area) { + if line.contains(mouse.column, mouse.row) { + focus_playbar(focus_block, app); + app.seek_to(line.position_at(mouse.column)); + } + } + } + // Dragging along the progress row scrubs; the column is clamped into the line. + MouseEventKind::Drag(MouseButton::Left) => { + if let Some(line) = playbar_progress_line(app, playbar_area) { + if line.on_row(mouse.row) { + focus_playbar(focus_block, app); + app.seek_to(line.position_at(mouse.column)); + } + } + } + _ => {} + } } fn handle_settings_screen_mouse(mouse: MouseEvent, app: &mut App) { @@ -1265,6 +1283,145 @@ mod tests { assert_eq!(route.hovered_block, ActiveBlock::PlayBar); } + #[test] + fn click_main_layout_playbar_progress_seeks() { + let mut app = App::default(); + app.size = Size { + width: 160, + height: 50, + }; + app.push_navigation_stack(RouteId::Home, ActiveBlock::Home); + with_playbar_context(&mut app); // 180_000 ms track + app.song_progress_ms = 0; + + let areas = main_layout_areas(&app).expect("layout areas"); + let line = playbar_progress_line(&app, areas.playbar).expect("progress line"); + + // A click a quarter of the way along the gauge seeks to ~25% of the track. + let quarter_x = line.start + line.width / 4; + handler( + mouse_event(MouseEventKind::Down(MouseButton::Left), quarter_x, line.row), + &mut app, + ); + let after_quarter = app.song_progress_ms; + assert!( + (35_000..=60_000).contains(&after_quarter), + "quarter click gave {after_quarter}" + ); + + // A click three quarters along seeks further into the track. + let three_quarter_x = line.start + (line.width * 3) / 4; + handler( + mouse_event( + MouseEventKind::Down(MouseButton::Left), + three_quarter_x, + line.row, + ), + &mut app, + ); + assert!( + (120_000..=150_000).contains(&app.song_progress_ms), + "three-quarter click gave {}", + app.song_progress_ms + ); + assert!(app.song_progress_ms > after_quarter); + + assert_eq!(app.get_current_route().active_block, ActiveBlock::PlayBar); + } + + #[test] + fn click_playbar_time_label_does_not_seek() { + let mut app = App::default(); + app.size = Size { + width: 160, + height: 50, + }; + app.push_navigation_stack(RouteId::Home, ActiveBlock::Home); + with_playbar_context(&mut app); + app.song_progress_ms = 12_345; + + let areas = main_layout_areas(&app).expect("layout areas"); + let line = playbar_progress_line(&app, areas.playbar).expect("progress line"); + + // Clicking the time label (left of the gauge line) must not seek. + let label_x = line.start - 2; + handler( + mouse_event(MouseEventKind::Down(MouseButton::Left), label_x, line.row), + &mut app, + ); + assert_eq!(app.song_progress_ms, 12_345); + } + + #[test] + fn drag_playbar_progress_seeks() { + let mut app = App::default(); + app.size = Size { + width: 160, + height: 50, + }; + app.push_navigation_stack(RouteId::Home, ActiveBlock::Home); + with_playbar_context(&mut app); + app.song_progress_ms = 0; + + let areas = main_layout_areas(&app).expect("layout areas"); + let line = playbar_progress_line(&app, areas.playbar).expect("progress line"); + + let mid_x = line.start + line.width / 2; + handler( + mouse_event(MouseEventKind::Drag(MouseButton::Left), mid_x, line.row), + &mut app, + ); + assert!( + (80_000..=100_000).contains(&app.song_progress_ms), + "drag to midpoint gave {}", + app.song_progress_ms + ); + } + + // Non-circular geometry check: the seek tests above derive their click column + // from `playbar_progress_line`, so a wrong `start`/`width` would cancel out. This + // renders the real playbar and asserts the computed line start matches the column + // where the actual gauge symbols begin in the rendered buffer. + #[test] + fn playbar_progress_line_start_matches_rendered_gauge() { + use crate::tui::ui::player::draw_playbar; + use ratatui::{backend::TestBackend, Terminal}; + + let mut app = App::default(); + app.size = Size { + width: 160, + height: 50, + }; + app.push_navigation_stack(RouteId::Home, ActiveBlock::Home); + with_playbar_context(&mut app); + app.song_progress_ms = 60_000; // 1/3 of the 180s track -> a partly-filled gauge + + let areas = main_layout_areas(&app).expect("layout areas"); + let line = playbar_progress_line(&app, areas.playbar).expect("progress line"); + + let mut terminal = Terminal::new(TestBackend::new(160, 50)).unwrap(); + terminal + .draw(|f| draw_playbar(f, &app, areas.playbar)) + .unwrap(); + let buffer = terminal.backend().buffer(); + + // The LineGauge fills with "⣿" and "⣉"; neither appears in the time label, so the + // first such cell on the progress row is the true gauge start column. + let rendered_start = (areas.playbar.x..areas.playbar.x + areas.playbar.width) + .find(|&x| { + matches!( + buffer.cell((x, line.row)).map(|c| c.symbol()), + Some("⣿") | Some("⣉") + ) + }) + .expect("expected rendered gauge cells on the progress row"); + + assert_eq!( + rendered_start, line.start, + "computed gauge start must match the rendered bar" + ); + } + #[test] fn click_lyrics_view_playbar_control_triggers_action() { let mut app = App::default(); diff --git a/src/tui/ui/player.rs b/src/tui/ui/player.rs index 60a0c71..9409011 100644 --- a/src/tui/ui/player.rs +++ b/src/tui/ui/player.rs @@ -15,6 +15,7 @@ use ratatui::{ use rspotify::model::enums::RepeatState; use rspotify::model::PlayableItem; use rspotify::prelude::Id; +use unicode_width::UnicodeWidthStr; use super::util::{ create_artist_string, display_track_progress, get_color, get_track_progress_percentage, @@ -396,6 +397,95 @@ pub(crate) fn playbar_control_at( .find_map(|(control, rect)| rect.contains(Position { x, y }).then_some(control)) } +/// Geometry of the seekable region of the playbar progress line, used to translate +/// a mouse column into an absolute playback position. Mirrors ratatui's `LineGauge` +/// layout: the left-aligned label is drawn first, then the gauge line begins one +/// column after it. +#[derive(Clone, Copy, Debug)] +pub(crate) struct PlaybarProgressLine { + /// Row the progress line is rendered on. + pub(crate) row: u16, + /// First column of the gauge line (the cell after the label + 1-column gap). + pub(crate) start: u16, + /// Number of cells in the gauge line. + pub(crate) width: u16, + /// Track duration the line represents, in milliseconds. + pub(crate) duration_ms: u32, +} + +impl PlaybarProgressLine { + /// True when `(x, y)` lands on the seekable gauge line (excludes the time label). + pub(crate) fn contains(&self, x: u16, y: u16) -> bool { + y == self.row && x >= self.start && x < self.start + self.width + } + + /// True when `y` is on the progress row, regardless of column (used for drags, + /// where the column is clamped into range rather than rejected). + pub(crate) fn on_row(&self, y: u16) -> bool { + y == self.row + } + + /// Map a column to an absolute position in milliseconds, clamped to the line. + /// The far-right cell maps to just under the full duration so a click never + /// overshoots into the next track. + pub(crate) fn position_at(&self, x: u16) -> u32 { + let last = self.start + self.width.saturating_sub(1); + let offset = x.clamp(self.start, last) - self.start; + let fraction = f64::from(offset) / f64::from(self.width.max(1)); + (f64::from(self.duration_ms) * fraction).round() as u32 + } +} + +/// Compute the seekable geometry of the playbar progress line for the current +/// playback, or `None` when nothing is playing or the line is not rendered (e.g. +/// the single-row playbar, or a terminal too narrow to fit the gauge). +pub(crate) fn playbar_progress_line(app: &App, playbar_area: Rect) -> Option { + let item = app + .current_playback_context + .as_ref() + .and_then(|ctx| ctx.item.as_ref())?; + + let progress_area = playbar_layout_areas(app, playbar_area).progress_area; + if progress_area.width == 0 || progress_area.height == 0 { + return None; + } + + // Duration as shown on the playbar (native track info preferred). Mirrors + // draw_playbar's `display_duration_ms`, so keep the two in sync (player.rs ~761). + let duration_ms = if let Some(native_info) = &app.native_track_info { + native_info.duration_ms + } else { + match item { + PlayableItem::Track(track) => track.duration.num_milliseconds() as u32, + PlayableItem::Episode(episode) => episode.duration.num_milliseconds() as u32, + _ => return None, + } + }; + + // Recreate the gauge label exactly as draw_playbar does so the computed line + // `start` matches the rendered bar. The label reflects a pending seek if one is + // in flight (see player.rs ~805), otherwise the current progress. + let progress_ms = app.seek_ms.unwrap_or(app.song_progress_ms); + let duration_std = std::time::Duration::from_millis(u64::from(duration_ms)); + let label = display_track_progress(progress_ms, duration_std); + + // LineGauge writes the label (capped at the area width), then starts the line one + // column later: `start = label_end + 1` (see ratatui-widgets LineGauge::render). + let label_width = (UnicodeWidthStr::width(label.as_str()) as u16).min(progress_area.width); + let start = progress_area.x + label_width + 1; + let right = progress_area.x + progress_area.width; + if start >= right { + return None; + } + + Some(PlaybarProgressLine { + row: progress_area.y, + start, + width: right - start, + duration_ms, + }) +} + fn draw_playbar_controls(f: &mut Frame<'_>, app: &App, controls_area: Rect) { let controls_style = Style::default().fg(app.user_config.theme.playbar_text); for hitbox in playbar_control_hitboxes_in_area(controls_area) {