Skip to content

Commit df8b55f

Browse files
committed
fix(tui): gate the search and copy keys on what they need
1 parent bf8c593 commit df8b55f

7 files changed

Lines changed: 183 additions & 37 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
### Fixed
1212

13-
- **The sidebar, the help menu and Settings only offer what your source can do**: without a Spotify session the Library panel used to be hidden under every other source, which also hid Friends and Stats even though neither needs Spotify, while under the Spotify scope it kept offering Liked Songs, Albums, Artists, Podcasts, Discover and Recently Played, each ending in the "Spotify not connected" message. The panel is now drawn under every source with the rows that work: Friends and Stats always, Local Files and AI DJ when built in, and the six Spotify rows only with a Spotify session; a mouse click on the panel lands on the row you see. The help menu (`?`) applies the same rule: a rebindable key that needs Spotify or another source stays listed with a `(needs Spotify)` / `(not for Local Files)` / `(Internet Radio only)` suffix, and a fixed key of a screen your source cannot reach is left out. Settings hides the Spotify-only behavior rows (the playback poll, the liked and episode icons, the saved-library sort orders, the community pin) without a session, marks the Spotify-only key rows the same way, offers only `home` and `stats` as startup routes without a session instead of five screens that would open empty, and names the `d` row "Switch Music Source" to match the help menu. The copy-song-URL and copy-album-URL keys now follow the track that plays, not the browse scope: a Spotify track playing under another source still copies, and with nothing playing both keys say so instead of doing nothing.
13+
- **The sidebar, the help menu and Settings only offer what your source can do**: without a Spotify session the Library panel used to be hidden under every other source, which also hid Friends and Stats even though neither needs Spotify, while under the Spotify scope it kept offering Liked Songs, Albums, Artists, Podcasts, Discover and Recently Played, each ending in the "Spotify not connected" message. The panel is now drawn under every source with the rows that work: Friends and Stats always, Local Files and AI DJ when built in, and the six Spotify rows only with a Spotify session; a mouse click on the panel lands on the row you see. The help menu (`?`) applies the same rule: a rebindable key that needs Spotify or another source stays listed with a `(needs Spotify)` / `(not for Local Files)` / `(Internet Radio only)` suffix, and a fixed key of a screen your source cannot reach is left out. Settings hides the Spotify-only behavior rows (the playback poll, the liked and episode icons, the saved-library sort orders, the community pin) without a session, marks the Spotify-only key rows the same way, offers only `home` and `stats` as startup routes without a session instead of five screens that would open empty, and names the `d` row "Switch Music Source" to match the help menu. The copy-song-URL and copy-album-URL keys now follow the track that plays, not the browse scope: a Spotify track playing under another source still copies, and with nothing playing both keys say so instead of doing nothing. The search key without a Spotify session says so too, instead of opening an input that cannot search.
1414

1515
- **Disconnecting headphones no longer freezes spotatui**: with a decoded source playing (Local Files, Subsonic, Internet Radio, YouTube, Qobuz), unplugging the output device — Bluetooth headphones disconnecting, a USB DAC pulled — left the audio engine waiting on an audio callback that would never run again. Because that wait happens on the same serial queue every other request goes through, the whole app stopped responding: playback was silent *and* searches, browsing, and playback controls stopped working until restart. The engine now notices the device is gone - both when it is unplugged and when the system simply moves its default output elsewhere, which nothing reports - and refuses those calls instead of waiting, then rebuilds the output on the new default device and puts the track back where it was. Unplugging pauses it there, the same thing macOS does when AirPods come out; plugging something in keeps it playing, now through the device you just connected. Radio stops instead (a live stream has no position to return to), and a track playing from the queue resumes at the next queued item ([#496](https://github.com/LargeModGames/spotatui/issues/496)).
1616

‎src/core/app/construction.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,14 +244,28 @@ impl Default for App {
244244
impl App {
245245
/// `App::default()` with a Spotify session, for tests that need one without
246246
/// an `IoEvent` channel.
247-
#[cfg(all(test, feature = "tui"))]
247+
#[cfg(test)]
248248
pub(crate) fn default_connected() -> App {
249249
App {
250250
spotify_connected: true,
251251
..App::default()
252252
}
253253
}
254254

255+
/// This app under a browse scope, for tests.
256+
#[cfg(all(test, feature = "tui"))]
257+
pub(crate) fn under_source(mut self, source: Source) -> App {
258+
self.active_source = source;
259+
self
260+
}
261+
262+
/// This app with a Spotify playback context, for tests.
263+
#[cfg(all(test, feature = "tui"))]
264+
pub(crate) fn with_playback(mut self, context: CurrentPlaybackContext) -> App {
265+
self.current_playback_context = Some(context);
266+
self
267+
}
268+
255269
#[cfg(test)]
256270
pub fn new(
257271
io_tx: Sender<IoEvent>,

‎src/core/driver/mod.rs‎

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,22 @@ pub struct Driver {
154154
radio_stream_started: bool,
155155
}
156156

157+
/// The startup route's data fetch, gated on what the route needs.
158+
fn fetch_startup_route(app: &mut App) {
159+
let route = app.get_current_route().id.clone();
160+
if !app.availability(route.startup_requirement()).is_available() {
161+
return;
162+
}
163+
match route {
164+
RouteId::RecentlyPlayed => app.dispatch(IoEvent::GetRecentlyPlayed),
165+
RouteId::AlbumList => app.dispatch(IoEvent::GetCurrentUserSavedAlbums(None)),
166+
RouteId::Artists => app.dispatch(IoEvent::GetFollowedArtists(None)),
167+
RouteId::Podcasts => app.dispatch(IoEvent::GetCurrentUserSavedShows(None)),
168+
RouteId::Stats => app.reload_stats(),
169+
_ => {}
170+
}
171+
}
172+
157173
impl Driver {
158174
pub fn new(
159175
shared_position: Option<Arc<AtomicU64>>,
@@ -867,18 +883,7 @@ impl Driver {
867883
// handlers that normally fetch a screen's data on navigation — kick
868884
// off that fetch here or the screen renders empty until re-entered.
869885
// (Home needs nothing extra; Discover fetches from within its menu.)
870-
// The fetch is gated on the route's startup requirement.
871-
let needs = app.get_current_route().id.startup_requirement();
872-
if app.availability(needs).is_available() {
873-
match app.get_current_route().id {
874-
RouteId::RecentlyPlayed => app.dispatch(IoEvent::GetRecentlyPlayed),
875-
RouteId::AlbumList => app.dispatch(IoEvent::GetCurrentUserSavedAlbums(None)),
876-
RouteId::Artists => app.dispatch(IoEvent::GetFollowedArtists(None)),
877-
RouteId::Podcasts => app.dispatch(IoEvent::GetCurrentUserSavedShows(None)),
878-
RouteId::Stats => app.reload_stats(),
879-
_ => {}
880-
}
881-
}
886+
fetch_startup_route(app);
882887
// A persisted non-Spotify active source needs its sidebar data loaded
883888
// too (all of these are inert no-ops when the feature is off).
884889
app.load_source_sidebar(app.active_source);
@@ -939,3 +944,53 @@ impl Driver {
939944
}
940945
}
941946
}
947+
948+
#[cfg(test)]
949+
mod tests {
950+
use super::*;
951+
use crate::core::app::ActiveBlock;
952+
use crate::core::user_config::UserConfig;
953+
use std::sync::mpsc::{channel, Receiver};
954+
use std::time::SystemTime;
955+
956+
fn app_on(route: RouteId, block: ActiveBlock, connected: bool) -> (App, Receiver<IoEvent>) {
957+
let (tx, rx) = channel();
958+
let mut app = App::new(tx, UserConfig::new(), connected.then(SystemTime::now));
959+
app.push_navigation_stack(route, block);
960+
(app, rx)
961+
}
962+
963+
#[test]
964+
fn a_spotify_startup_route_fetches_nothing_without_a_session() {
965+
let (mut app, rx) = app_on(RouteId::AlbumList, ActiveBlock::AlbumList, false);
966+
967+
fetch_startup_route(&mut app);
968+
969+
assert!(!rx
970+
.try_iter()
971+
.any(|event| matches!(event, IoEvent::GetCurrentUserSavedAlbums(_))));
972+
}
973+
974+
#[test]
975+
fn a_spotify_startup_route_fetches_its_data_with_a_session() {
976+
let (mut app, rx) = app_on(RouteId::AlbumList, ActiveBlock::AlbumList, true);
977+
978+
fetch_startup_route(&mut app);
979+
980+
assert!(rx
981+
.try_iter()
982+
.any(|event| matches!(event, IoEvent::GetCurrentUserSavedAlbums(_))));
983+
}
984+
985+
#[test]
986+
fn the_stats_startup_route_loads_without_a_session() {
987+
let (mut app, rx) = app_on(RouteId::Stats, ActiveBlock::Stats, false);
988+
989+
fetch_startup_route(&mut app);
990+
991+
assert!(app.stats_loading);
992+
assert!(rx
993+
.try_iter()
994+
.any(|event| matches!(event, IoEvent::LoadListeningStats(_))));
995+
}
996+
}

‎src/tui/handlers/empty.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,20 @@ pub fn handler(key: Key, app: &mut App) {
7474
mod tests {
7575
use super::*;
7676
use crate::core::app::RouteId;
77+
use crate::core::source::Source;
78+
79+
#[test]
80+
fn up_and_left_reach_the_library_under_a_free_source() {
81+
let mut app = App::default().under_source(Source::Local);
82+
app.set_current_route_state(Some(ActiveBlock::Empty), Some(ActiveBlock::MyPlaylists));
83+
84+
handler(Key::Up, &mut app);
85+
assert_eq!(app.get_current_route().hovered_block, ActiveBlock::Library);
86+
87+
app.set_current_route_state(None, Some(ActiveBlock::AlbumTracks));
88+
handler(Key::Left, &mut app);
89+
assert_eq!(app.get_current_route().hovered_block, ActiveBlock::Library);
90+
}
7791

7892
#[test]
7993
fn on_enter() {

‎src/tui/handlers/mod.rs‎

Lines changed: 83 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ use crate::core::app::{
4747
ActiveBlock, App, ArtistBlock, InputContext, PlaybackOwner, RouteId, SearchResultBlock,
4848
NOTHING_PLAYING_STATUS,
4949
};
50+
use crate::core::requirement::{Availability, Capability, Requirement};
5051
use crate::core::source::Source;
5152
use crate::tui::event::Key;
5253

@@ -399,25 +400,40 @@ pub(super) fn focus_global_search(app: &mut App) {
399400
app.set_current_route_state(Some(ActiveBlock::Input), Some(ActiveBlock::Input));
400401
}
401402

402-
/// Focuses the search input, or reports why the active source has none.
403+
/// Focuses the search input, or reports why the key does nothing now.
403404
pub(super) fn open_global_search(app: &mut App) -> bool {
404-
if app.active_source.supports_search() {
405-
focus_global_search(app);
406-
true
407-
} else {
408-
app.set_status_message(
409-
format!("Search isn't available for {}", app.active_source.label()),
410-
4,
411-
);
412-
false
405+
match app.availability(Requirement::Capability(Capability::Search)) {
406+
Availability::Available => {
407+
focus_global_search(app);
408+
true
409+
}
410+
Availability::NeedsSpotify => {
411+
app.set_status_message("Search needs a Spotify session", 4);
412+
false
413+
}
414+
Availability::NotForSource(_) | Availability::OnlyForSource(_) => {
415+
app.set_status_message(
416+
format!("Search isn't available for {}", app.active_source.label()),
417+
4,
418+
);
419+
false
420+
}
413421
}
414422
}
415423

416-
/// The copy keys read the Spotify playback, so they need Spotify to own it.
424+
/// The copy keys read the Spotify playback, so Spotify must own it and have an item.
417425
fn copy_url(app: &mut App, copy: Action) {
418426
match app.playback_owner() {
419427
PlaybackOwner::Spotify | PlaybackOwner::NativeSpotify => {
420-
app.apply(copy);
428+
let has_item = app
429+
.current_playback_context
430+
.as_ref()
431+
.is_some_and(|context| context.item.is_some());
432+
if has_item {
433+
app.apply(copy);
434+
} else {
435+
app.set_status_message(NOTHING_PLAYING_STATUS, 4);
436+
}
421437
}
422438
PlaybackOwner::None => app.set_status_message(NOTHING_PLAYING_STATUS, 4),
423439
PlaybackOwner::Queue | PlaybackOwner::Decoded => {
@@ -685,7 +701,7 @@ mod tests {
685701

686702
#[test]
687703
fn search_key_on_the_error_page_dismisses_the_error_first() {
688-
let mut app = App::default();
704+
let mut app = App::default_connected();
689705
app.handle_error(anyhow::anyhow!("boom"));
690706
assert_eq!(app.get_current_route().id, RouteId::Error);
691707

@@ -952,7 +968,7 @@ mod tests {
952968

953969
#[test]
954970
fn search_key_in_playlist_track_table_opens_global_search_input() {
955-
let mut app = App::default();
971+
let mut app = App::default_connected();
956972
let playlist_id = PlaylistId::from_id("37i9dQZF1DX4WYpdgoIcn6")
957973
.unwrap()
958974
.into_static();
@@ -968,7 +984,7 @@ mod tests {
968984

969985
#[test]
970986
fn search_key_outside_playlist_track_table_opens_global_search_input() {
971-
let mut app = App::default();
987+
let mut app = App::default_connected();
972988
app.track_table.context = Some(TrackTableContext::SavedTracks);
973989
app.push_navigation_stack(RouteId::TrackTable, ActiveBlock::TrackTable);
974990

@@ -1094,10 +1110,35 @@ mod tests {
10941110
);
10951111
}
10961112

1113+
fn spotify_track_context() -> CurrentPlaybackContext {
1114+
CurrentPlaybackContext {
1115+
device: Device {
1116+
id: Some("device-1".to_string()),
1117+
is_active: true,
1118+
is_private_session: false,
1119+
is_restricted: false,
1120+
name: "Desk Speaker".to_string(),
1121+
_type: DeviceType::Computer,
1122+
volume_percent: Some(42),
1123+
},
1124+
repeat_state: RepeatState::Off,
1125+
shuffle_state: false,
1126+
context: None,
1127+
timestamp: Utc::now(),
1128+
progress: None,
1129+
is_playing: true,
1130+
item: Some(PlayableItem::Track(full_track(
1131+
"0000000000000000000001",
1132+
"Track 1",
1133+
))),
1134+
currently_playing_type: CurrentlyPlayingType::Track,
1135+
actions: Actions::default(),
1136+
}
1137+
}
1138+
10971139
#[test]
10981140
fn copy_url_keys_follow_the_playback_owner_not_the_browse_scope() {
1099-
let mut app = App::default();
1100-
app.active_source = Source::Local;
1141+
let mut app = App::default().under_source(Source::Local);
11011142
app.set_current_route_state(Some(ActiveBlock::Empty), Some(ActiveBlock::Library));
11021143

11031144
handle_app(app.user_config.keys.copy_song_url, &mut app);
@@ -1107,14 +1148,36 @@ mod tests {
11071148
handle_app(app.user_config.keys.copy_album_url, &mut app);
11081149
assert_eq!(app.status_message.as_deref(), Some("Nothing is playing"));
11091150

1110-
// A session under the Local scope still owns the Spotify playback.
1111-
let mut app = App::default_connected();
1112-
app.active_source = Source::Local;
1151+
// An idle session is not a playing track.
1152+
let mut app = App::default_connected().under_source(Source::Local);
1153+
app.set_current_route_state(Some(ActiveBlock::Empty), Some(ActiveBlock::Library));
1154+
handle_app(app.user_config.keys.copy_album_url, &mut app);
1155+
assert_eq!(app.status_message.as_deref(), Some("Nothing is playing"));
1156+
1157+
// A Spotify track playing under the Local scope still copies; with no
1158+
// clipboard in a test App the copy exits without a message.
1159+
let mut app = App::default_connected()
1160+
.under_source(Source::Local)
1161+
.with_playback(spotify_track_context());
11131162
app.set_current_route_state(Some(ActiveBlock::Empty), Some(ActiveBlock::Library));
11141163
handle_app(app.user_config.keys.copy_album_url, &mut app);
11151164
assert_eq!(app.status_message, None);
11161165
}
11171166

1167+
#[test]
1168+
fn the_search_key_without_a_session_says_so_instead_of_opening_the_input() {
1169+
let mut app = App::default();
1170+
app.set_current_route_state(Some(ActiveBlock::Empty), Some(ActiveBlock::Library));
1171+
1172+
handle_app(app.user_config.keys.search, &mut app);
1173+
1174+
assert_ne!(app.get_current_route().active_block, ActiveBlock::Input);
1175+
assert_eq!(
1176+
app.status_message.as_deref(),
1177+
Some("Search needs a Spotify session")
1178+
);
1179+
}
1180+
11181181
/// The DJ's own tests all call `ai_dj::handler` directly, which is the branch
11191182
/// taken once the DJ screen already has focus (`handle_app` returns early for
11201183
/// `ActiveBlock::AiDj`). That leaves the `dj_pick_model` arm of the global match

‎src/tui/handlers/mouse.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1144,7 +1144,7 @@ mod tests {
11441144

11451145
#[test]
11461146
fn click_search_input_focuses_input() {
1147-
let mut app = App::default();
1147+
let mut app = App::default_connected();
11481148
app.view.size = Viewport {
11491149
width: 160,
11501150
height: 50,

‎tools/gates.count‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@
77
# the two adoption counters may only rise.
88

99
crate_tui_refs_outside_tui = 2 # target 0
10-
app_field_writes_in_tui_handlers = 317 # target 0 (writes into app.view do not count)
10+
app_field_writes_in_tui_handlers = 315 # target 0 (writes into app.view do not count)
1111
ioevent_refs_in_tui = 51 # target 0
1212
synthetic_keys_in_mouse_handler = 3 # target 0 (the content-table re-entries through handle_block_events)
1313
wildcard_arms_in_action_tree = 0 # target 0, must stay 0
1414
view_writes_outside_tui = 12 # target 0 (producers outside tui/ and core/app/ writing App::view)
1515
action_refs_in_tui_handlers = 173 # adoption: may only rise
16-
test_attribute_total = 1795 # adoption: may only rise
16+
test_attribute_total = 1800 # adoption: may only rise

0 commit comments

Comments
 (0)