From af2542b6682c7809d07f8126bccb320017479094 Mon Sep 17 00:00:00 2001 From: Ultradesu Date: Thu, 23 Jul 2026 17:35:11 +0300 Subject: [PATCH] Improved UI. --- README.md | 5 +- src/app/action.rs | 5 +- src/app/popup.rs | 21 ++-- src/app/state.rs | 2 +- src/app/update.rs | 177 ++++++++++++++++++++++++++------- src/config/default_keymap.toml | 9 +- src/config/keymap.rs | 43 ++++++++ src/ui/popup.rs | 39 ++++++-- 8 files changed, 241 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 2ce9ed2..c1c61ed 100644 --- a/README.md +++ b/README.md @@ -93,8 +93,9 @@ Common key bindings: | `x` | Like / unlike | | `a` | Add track next | | `Shift-A` | Add track to the end of the queue | -| `Shift-P` | Add track to a playlist | -| `Shift-D` | Open device picker | +| `Shift-P` | Add selected/current track(s) to a playlist | +| `i`, `Shift-I` | Track info / current track info | +| `Shift-D` | Delete selected item | | `v` | Toggle tile/table view | | `/` | Search | | `:` | Open command line | diff --git a/src/app/action.rs b/src/app/action.rs index 89b3a73..b8df863 100644 --- a/src/app/action.rs +++ b/src/app/action.rs @@ -34,6 +34,7 @@ pub enum Action { ToggleLike, ToggleTrackSelection, OpenTrackInfo, + OpenCurrentTrackInfo, QueueAddNext, QueueAddLast, /// Download the selected federated track(s) into the local library. @@ -105,7 +106,8 @@ impl Action { | Action::NewPlaylist | Action::ToggleLike | Action::ToggleTrackSelection - | Action::OpenTrackInfo => Category::Queue, + | Action::OpenTrackInfo + | Action::OpenCurrentTrackInfo => Category::Queue, Action::MoveUp | Action::MoveDown | Action::MoveLeft @@ -174,6 +176,7 @@ impl Action { Action::ToggleLike => "Like / unlike".into(), Action::ToggleTrackSelection => "Track line selection".into(), Action::OpenTrackInfo => "Track info".into(), + Action::OpenCurrentTrackInfo => "Current track info".into(), Action::QueueAddNext => "Queue: add next".into(), Action::QueueAddLast => "Queue: add to end".into(), Action::DownloadSelected => "Federation: download to library".into(), diff --git a/src/app/popup.rs b/src/app/popup.rs index 292eb49..cd0dc5e 100644 --- a/src/app/popup.rs +++ b/src/app/popup.rs @@ -12,7 +12,8 @@ use crossterm::event::{KeyCode, KeyEvent}; use crate::app::Runtime; use crate::app::event::AppEvent; use crate::app::state::{ - AppState, DeleteTarget, EditField, EditTarget, FedInputField, Popup, addable_playlists, + AppState, DeleteTarget, EditField, EditTarget, FedInputField, Loadable, Popup, + addable_playlists, }; use crate::library::models::{ReleaseEdit, TrackEdit, TrackItem}; @@ -536,7 +537,9 @@ fn handle_picker( cursor: usize, key: KeyEvent, ) { + let waiting = matches!(&state.playlists.list, None | Some(Loadable::Loading)); let options = addable_playlists(state); + let new_index = options.len(); match key.code { KeyCode::Esc => {} KeyCode::Up | KeyCode::Char('k') => { @@ -548,18 +551,23 @@ fn handle_picker( KeyCode::Down | KeyCode::Char('j') => { state.popup = Some(Popup::AddToPlaylist { target, - cursor: (cursor + 1).min(options.len()), + cursor: (cursor + 1).min(new_index), }); } KeyCode::Enter => { - if cursor == 0 { + if waiting { + state.status_message = Some("loading playlists…".into()); + state.popup = Some(Popup::AddToPlaylist { target, cursor }); + } else if cursor < options.len() { + if let Some((id, title)) = options.get(cursor).cloned() { + spawn_add_target(runtime, id, title, target); + } + } else { state.popup = Some(Popup::NewPlaylist { for_target: Some(target), input: crate::app::input::LineEdit::default(), busy: false, }); - } else if let Some((id, title)) = options.get(cursor - 1).cloned() { - spawn_add_target(runtime, id, title, target); } } _ => state.popup = Some(Popup::AddToPlaylist { target, cursor }), @@ -586,7 +594,8 @@ fn handle_name_entry( KeyCode::Esc => { // Reached from the picker → step back to it; otherwise close. if let Some(target) = for_target { - state.popup = Some(Popup::AddToPlaylist { target, cursor: 0 }); + let cursor = addable_playlists(state).len(); + state.popup = Some(Popup::AddToPlaylist { target, cursor }); } } KeyCode::Enter => { diff --git a/src/app/state.rs b/src/app/state.rs index 3e702b6..a2ab02a 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -480,7 +480,7 @@ impl PlaylistAddTarget { /// Modal dialog over the main screen. #[derive(Debug)] pub enum Popup { - /// Pick one of the playlists (row 0 = "create new"); the target is + /// Pick one of the playlists (last row = "create new"); the target is /// added on Enter (federated tracks are downloaded first). AddToPlaylist { target: PlaylistAddTarget, diff --git a/src/app/update.rs b/src/app/update.rs index 5ee6594..6e6f198 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -283,27 +283,11 @@ pub fn update(state: &mut AppState, action: Action) -> Option { } Action::OpenTrackInfo => { let tracks = selected_tracks(state); - if tracks.is_empty() { - state.status_message = Some("no track selected".into()); - None - } else { - let fed_tracks = tracks - .iter() - .filter(|track| track_info_needs_fed_metadata(track)) - .filter_map(|track| track.fed.as_ref().map(|fed| (track.id, fed.clone()))) - .collect::>(); - state.popup = Some(super::state::Popup::TrackInfo { - tracks, - cursor: 0, - scroll: 0, - }); - if fed_tracks.is_empty() { - None - } else { - state.status_message = Some("federation: fetching track metadata…".to_string()); - Some(Effect::FedFetchTrackInfo { tracks: fed_tracks }) - } - } + open_track_info(state, tracks, "no track selected") + } + Action::OpenCurrentTrackInfo => { + let tracks = state.player.current.clone().into_iter().collect(); + open_track_info(state, tracks, "nothing playing") } Action::RemoveFromQueue => remove_selected_from_queue(state), Action::QueueAddNext => queue_add(state, true), @@ -317,19 +301,7 @@ pub fn update(state: &mut AppState, action: Action) -> Option { None } Action::AddToPlaylist => { - let fed = selected_fed_tracks(state); - let target = if !fed.is_empty() { - Some(super::state::PlaylistAddTarget::Fed(fed)) - } else { - let local = selected_tracks(state); - if !local.is_empty() { - Some(super::state::PlaylistAddTarget::Local(local)) - } else { - selected_track(state) - .or_else(|| state.player.current.clone()) - .map(|track| super::state::PlaylistAddTarget::Local(vec![track])) - } - }; + let target = selected_playlist_target(state, true); match target { Some(target) => { state.popup = Some(super::state::Popup::AddToPlaylist { target, cursor: 0 }); @@ -355,11 +327,13 @@ pub fn update(state: &mut AppState, action: Action) -> Option { } } Action::NewPlaylist => { + let for_target = selected_playlist_target(state, false); state.popup = Some(super::state::Popup::NewPlaylist { - for_target: None, + for_target, input: crate::app::input::LineEdit::default(), busy: false, }); + state.track_selection.clear(); None } Action::ClearQueue => { @@ -399,6 +373,54 @@ fn track_info_needs_fed_metadata(track: &TrackItem) -> bool { || track.file_path.is_empty()) } +fn open_track_info( + state: &mut AppState, + tracks: Vec, + empty_message: &'static str, +) -> Option { + if tracks.is_empty() { + state.status_message = Some(empty_message.into()); + return None; + } + + let fed_tracks = tracks + .iter() + .filter(|track| track_info_needs_fed_metadata(track)) + .filter_map(|track| track.fed.as_ref().map(|fed| (track.id, fed.clone()))) + .collect::>(); + state.popup = Some(super::state::Popup::TrackInfo { + tracks, + cursor: 0, + scroll: 0, + }); + if fed_tracks.is_empty() { + None + } else { + state.status_message = Some("federation: fetching track metadata…".to_string()); + Some(Effect::FedFetchTrackInfo { tracks: fed_tracks }) + } +} + +fn selected_playlist_target( + state: &AppState, + include_current: bool, +) -> Option { + let fed = selected_fed_tracks(state); + if !fed.is_empty() { + return Some(super::state::PlaylistAddTarget::Fed(fed)); + } + + let local = selected_tracks(state); + if !local.is_empty() { + return Some(super::state::PlaylistAddTarget::Local(local)); + } + + include_current + .then(|| state.player.current.clone()) + .flatten() + .map(|track| super::state::PlaylistAddTarget::Local(vec![track])) +} + /// `e`: open the metadata edit form for whatever is under the cursor — /// an artist tile, a release, a track or a playlist. fn open_edit_popup(state: &mut AppState) { @@ -2748,6 +2770,91 @@ mod tests { assert!(!state.player.playing); } + #[test] + fn current_track_info_uses_now_playing_track() { + let mut state = AppState { + active_tab: Tab::Queue, + ..AppState::default() + }; + state.player.queue = vec![test_track(1), test_track(2)]; + state.queue_tab.cursor = 0; + state.player.current = Some(test_track(2)); + + assert_eq!(update(&mut state, Action::OpenCurrentTrackInfo), None); + match &state.popup { + Some(crate::app::state::Popup::TrackInfo { tracks, .. }) => { + assert_eq!( + tracks.iter().map(|track| track.id).collect::>(), + vec![2] + ); + } + other => panic!("expected track info popup, got {other:?}"), + } + } + + #[test] + fn add_to_playlist_from_release_carries_selected_track() { + use crate::app::state::{PlaylistAddTarget, Popup}; + use crate::library::models::ReleaseDetail; + + let mut state = AppState::default(); + state + .global + .stack + .push(GlobalView::Release { id: 1, cursor: 1 }); + state.release_views.insert( + 1, + Loadable::Ready(ReleaseDetail { + id: 1, + title: "r".into(), + release_type: "album".into(), + year: None, + cover_path: None, + artists: vec![], + tracks: vec![test_track(1), test_track(2)], + }), + ); + + assert_eq!(update(&mut state, Action::AddToPlaylist), None); + match &state.popup { + Some(Popup::AddToPlaylist { + target: PlaylistAddTarget::Local(tracks), + .. + }) => { + assert_eq!( + tracks.iter().map(|track| track.id).collect::>(), + vec![2] + ); + } + other => panic!("expected add-to-playlist popup with selected track, got {other:?}"), + } + } + + #[test] + fn add_to_playlist_from_non_track_view_uses_current_track() { + use crate::app::state::{PlaylistAddTarget, Popup}; + + let mut state = AppState { + active_tab: Tab::Logs, + ..AppState::default() + }; + state.player.current = Some(test_track(7)); + + assert_eq!(update(&mut state, Action::AddToPlaylist), None); + match &state.popup { + Some(Popup::AddToPlaylist { + target: PlaylistAddTarget::Local(tracks), + .. + }) => { + assert_eq!( + tracks.iter().map(|track| track.id).collect::>(), + vec![7] + ); + } + other => panic!("expected add-to-playlist popup with current track, got {other:?}"), + } + } + #[test] fn visual_selection_removes_queue_range() { let mut state = AppState { diff --git a/src/config/default_keymap.toml b/src/config/default_keymap.toml index b48ea23..da87197 100644 --- a/src/config/default_keymap.toml +++ b/src/config/default_keymap.toml @@ -86,11 +86,6 @@ command = "GoToRelease" key_sequence = "shift-p" command = "AddToPlaylist" -[[keymaps]] -key_sequence = "n" -command = "NewPlaylist" -context = "playlists" - [[keymaps]] key_sequence = "j" command = "MoveDown" @@ -211,6 +206,10 @@ command = "ToggleTrackSelection" key_sequence = "i" command = "OpenTrackInfo" +[[keymaps]] +key_sequence = "shift-i" +command = "OpenCurrentTrackInfo" + [[keymaps]] key_sequence = "e" command = "EditSelected" diff --git a/src/config/keymap.rs b/src/config/keymap.rs index 061e013..4c8e39b 100644 --- a/src/config/keymap.rs +++ b/src/config/keymap.rs @@ -416,6 +416,49 @@ mod tests { ); } + #[test] + fn default_playlist_context_keeps_next_track_on_n() { + let mut km = keymap_from(DEFAULT_KEYMAP); + assert_eq!( + km.resolve(key!(n), KeyContext::Playlists), + KeyResolution::Action(Action::NextTrack) + ); + } + + #[test] + fn default_shift_n_is_unbound() { + let mut km = keymap_from(DEFAULT_KEYMAP); + let shift_n = KeyCombination::new(KeyCode::Char('N'), KeyModifiers::SHIFT); + assert_eq!( + km.resolve(shift_n, KeyContext::Library), + KeyResolution::Unmatched + ); + assert_eq!( + km.resolve(shift_n, KeyContext::Playlists), + KeyResolution::Unmatched + ); + } + + #[test] + fn default_add_to_playlist_key_is_global() { + let mut km = keymap_from(DEFAULT_KEYMAP); + let shift_p = KeyCombination::new(KeyCode::Char('P'), KeyModifiers::SHIFT); + assert_eq!( + km.resolve(shift_p, KeyContext::Library), + KeyResolution::Action(Action::AddToPlaylist) + ); + } + + #[test] + fn default_current_track_info_key_resolves() { + let mut km = keymap_from(DEFAULT_KEYMAP); + let shift_i = KeyCombination::new(KeyCode::Char('I'), KeyModifiers::SHIFT); + assert_eq!( + km.resolve(shift_i, KeyContext::Library), + KeyResolution::Action(Action::OpenCurrentTrackInfo) + ); + } + #[test] fn user_binding_overrides_default() { let mut bindings = parse_bindings(DEFAULT_KEYMAP).unwrap(); diff --git a/src/ui/popup.rs b/src/ui/popup.rs index a67ef4e..bde268e 100644 --- a/src/ui/popup.rs +++ b/src/ui/popup.rs @@ -441,8 +441,14 @@ fn file_size(value: Option) -> String { fn draw_picker(frame: &mut Frame, state: &AppState, track_title: &str, cursor: usize) { let options = addable_playlists(state); - let loading = !matches!(&state.playlists.list, Some(Loadable::Ready(_))); - let rows = options.len() + 1; + let loading = matches!(&state.playlists.list, None | Some(Loadable::Loading)); + let rows = if loading { + 1 + } else if options.is_empty() { + 2 + } else { + options.len() + 1 + }; let height = (rows as u16 + 4) .min(frame.area().height.saturating_sub(2)) .max(6); @@ -463,18 +469,30 @@ fn draw_picker(frame: &mut Frame, state: &AppState, track_title: &str, cursor: u ]) .areas(inner); - let mut lines: Vec = vec![Line::styled("+ New playlist…", theme::accent())]; + let mut lines: Vec = Vec::new(); if loading { lines.push(Line::styled("loading playlists…", theme::dim())); + } else if matches!(&state.playlists.list, Some(Loadable::Failed(_))) { + lines.push(Line::styled("playlist list unavailable", theme::dim())); + lines.push(Line::styled("+ New playlist…", theme::accent())); } else if options.is_empty() { lines.push(Line::styled("no playlists yet", theme::dim())); + lines.push(Line::styled("+ New playlist…", theme::accent())); } else { for (_, title) in &options { lines.push(Line::raw(title.clone())); } + lines.push(Line::styled("+ New playlist…", theme::accent())); } + let selected_line = if loading { + 0 + } else if options.is_empty() { + lines.len().saturating_sub(1) + } else { + cursor.min(options.len()) + }; let visible = usize::from(list_area.height.max(1)); - let first = cursor + let first = selected_line .saturating_sub(visible / 2) .min(lines.len().saturating_sub(visible)); for (index, line) in lines.into_iter().enumerate().skip(first).take(visible) { @@ -485,17 +503,18 @@ fn draw_picker(frame: &mut Frame, state: &AppState, track_title: &str, cursor: u height: 1, }; frame.render_widget(Paragraph::new(line), row); - if index == cursor { + if index == selected_line { frame.buffer_mut().set_style(row, theme::tab_active()); } } + let footer_text = if loading { + format!("♪ {track_title} · loading playlists · esc close") + } else { + format!("♪ {track_title} · enter add/create · esc close") + }; frame.render_widget( - Paragraph::new(Line::styled( - format!("♪ {track_title} · enter add · esc close"), - theme::dim(), - )) - .alignment(Alignment::Center), + Paragraph::new(Line::styled(footer_text, theme::dim())).alignment(Alignment::Center), footer, ); }