From 0eedc38b8afec5e7dc443735e411a5dac656bd19 Mon Sep 17 00:00:00 2001 From: Thomas <48214567+ThomasFrans@users.noreply.github.com> Date: Tue, 20 Sep 2022 13:09:51 -0700 Subject: [PATCH] Improve context menus to make the UX/UI more consistent (#923) * Add save option to context menu of all possible ListItems * Add play options to context menus * Fix for playlists and tracks * Move playback controls into main menu --- src/model/album.rs | 10 ++++ src/model/artist.rs | 10 ++++ src/model/episode.rs | 5 ++ src/model/playlist.rs | 14 ++++++ src/model/show.rs | 10 ++++ src/model/track.rs | 10 ++++ src/traits.rs | 11 +++++ src/ui/contextmenu.rs | 110 +++++++++++++++++++++--------------------- 8 files changed, 125 insertions(+), 55 deletions(-) diff --git a/src/model/album.rs b/src/model/album.rs index 1f6e2df..dd6b924 100644 --- a/src/model/album.rs +++ b/src/model/album.rs @@ -302,6 +302,16 @@ impl ListItem for Album { ) } + #[inline] + fn is_saved(&self, library: Arc) -> Option { + Some(library.is_saved_album(self)) + } + + #[inline] + fn is_playable(&self) -> bool { + true + } + fn as_listitem(&self) -> Box { Box::new(self.clone()) } diff --git a/src/model/artist.rs b/src/model/artist.rs index 8ef090c..cf57e55 100644 --- a/src/model/artist.rs +++ b/src/model/artist.rs @@ -202,6 +202,16 @@ impl ListItem for Artist { .map(|id| format!("https://open.spotify.com/artist/{}", id)) } + #[inline] + fn is_saved(&self, library: Arc) -> Option { + Some(library.is_followed_artist(self)) + } + + #[inline] + fn is_playable(&self) -> bool { + true + } + fn as_listitem(&self) -> Box { Box::new(self.clone()) } diff --git a/src/model/episode.rs b/src/model/episode.rs index 90208a2..ec61a7e 100644 --- a/src/model/episode.rs +++ b/src/model/episode.rs @@ -110,6 +110,11 @@ impl ListItem for Episode { Some(format!("https://open.spotify.com/episode/{}", self.id)) } + #[inline] + fn is_playable(&self) -> bool { + true + } + fn as_listitem(&self) -> Box { Box::new(self.clone()) } diff --git a/src/model/playlist.rs b/src/model/playlist.rs index 1951d86..d04e84d 100644 --- a/src/model/playlist.rs +++ b/src/model/playlist.rs @@ -324,6 +324,20 @@ impl ListItem for Playlist { )) } + fn is_saved(&self, library: Arc) -> Option { + // save status of personal playlists can't be toggled for safety + if !library.is_followed_playlist(self) { + return None; + } + + Some(library.is_saved_playlist(self)) + } + + #[inline] + fn is_playable(&self) -> bool { + true + } + fn as_listitem(&self) -> Box { Box::new(self.clone()) } diff --git a/src/model/show.rs b/src/model/show.rs index 9dcabd7..398af14 100644 --- a/src/model/show.rs +++ b/src/model/show.rs @@ -150,6 +150,16 @@ impl ListItem for Show { Some(format!("https://open.spotify.com/show/{}", self.id)) } + #[inline] + fn is_saved(&self, library: Arc) -> Option { + Some(library.is_saved_show(self)) + } + + #[inline] + fn is_playable(&self) -> bool { + true + } + fn as_listitem(&self) -> Box { Box::new(self.clone()) } diff --git a/src/model/track.rs b/src/model/track.rs index 3ed7e2f..c2ca9e8 100644 --- a/src/model/track.rs +++ b/src/model/track.rs @@ -333,6 +333,16 @@ impl ListItem for Track { Some(self.clone()) } + #[inline] + fn is_saved(&self, library: Arc) -> Option { + Some(library.is_saved_track(&Playable::Track(self.clone()))) + } + + #[inline] + fn is_playable(&self) -> bool { + true + } + fn as_listitem(&self) -> Box { Box::new(self.clone()) } diff --git a/src/traits.rs b/src/traits.rs index 426a0a2..49c9bc6 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -47,6 +47,17 @@ pub trait ListItem: Sync + Send + 'static { None } + #[allow(unused_variables)] + #[inline] + fn is_saved(&self, library: Arc) -> Option { + None + } + + #[inline] + fn is_playable(&self) -> bool { + false + } + fn as_listitem(&self) -> Box; } diff --git a/src/ui/contextmenu.rs b/src/ui/contextmenu.rs index 4075703..1fbffaa 100644 --- a/src/ui/contextmenu.rs +++ b/src/ui/contextmenu.rs @@ -1,4 +1,3 @@ -use std::borrow::Borrow; use std::sync::Arc; use cursive::view::{Margins, ViewWrapper}; @@ -15,6 +14,7 @@ use crate::model::track::Track; use crate::queue::Queue; #[cfg(feature = "share_clipboard")] use crate::sharing::write_share; +use crate::spotify::PlayerEvent; use crate::traits::{ListItem, ViewExt}; use crate::ui::layout::Layout; use crate::ui::modal::Modal; @@ -42,7 +42,6 @@ pub struct SelectArtistActionMenu { } enum ContextMenuAction { - PlayTrack(Box), ShowItem(Box), SelectArtist(Vec), SelectArtistAction(Artist), @@ -50,36 +49,14 @@ enum ContextMenuAction { ShareUrl(String), AddToPlaylist(Box), ShowRecommendations(Box), - ToggleTrackSavedStatus(Box), + ToggleSavedStatus(Box), + Play(Box), + PlayNext(Box), + TogglePlayback, + Queue(Box), } impl ContextMenu { - pub fn play_track_dialog(queue: Arc, track: Track) -> NamedView { - let track_title = track.title.clone(); - let mut track_action_select = SelectView::::new(); - track_action_select.add_item("Play now", true); - track_action_select.add_item("Add to queue", false); - track_action_select.set_on_submit(move |s, selected| { - match selected { - true => track.borrow().clone().play(queue.clone()), - false => track.borrow().clone().queue(queue.clone()), - } - s.pop_layer(); - }); - let dialog = Dialog::new() - .title(format!("Play track: {}", track_title)) - .dismiss_button("Cancel") - .padding(Margins::lrtb(1, 1, 1, 0)) - .content(ScrollView::new( - track_action_select.with_name("playtrack_select"), - )); - - PlayTrackMenu { - dialog: Modal::new_ext(dialog), - } - .with_name("playtrackmenu") - } - pub fn add_track_dialog( library: Arc, spotify: Spotify, @@ -126,7 +103,7 @@ impl ContextMenu { let dialog = Dialog::new() .title("Add track to playlist") - .dismiss_button("Cancel") + .dismiss_button("Close") .padding(Margins::lrtb(1, 1, 1, 0)) .content(ScrollView::new(list_select.with_name("addplaylist_select"))); @@ -159,7 +136,7 @@ impl ContextMenu { let dialog = Dialog::new() .title("Select artist") - .dismiss_button("Cancel") + .dismiss_button("Close") .padding(Margins::lrtb(1, 1, 1, 0)) .content(ScrollView::new(artist_select.with_name("artist_select"))); @@ -210,7 +187,7 @@ impl ContextMenu { "Select action for artist: {}", artist.name.as_str() )) - .dismiss_button("Cancel") + .dismiss_button("Close") .padding(Margins::lrtb(1, 1, 1, 0)) .content(ScrollView::new( artist_action_select.with_name("artist_action_select"), @@ -225,11 +202,34 @@ impl ContextMenu { Dialog::text("This track is already in your playlist") .title("Track already exists") .padding(Margins::lrtb(1, 1, 1, 0)) - .dismiss_button("Cancel") + .dismiss_button("Close") } pub fn new(item: &dyn ListItem, queue: Arc, library: Arc) -> NamedView { let mut content: SelectView = SelectView::new(); + + if item.is_playable() { + if item.is_playing(queue.clone()) + && queue.get_spotify().get_current_status() + == PlayerEvent::Paused(queue.get_spotify().get_current_progress()) + { + // the item is the current track, but paused + content.insert_item(0, "Resume", ContextMenuAction::TogglePlayback); + } else if !item.is_playing(queue.clone()) { + // the item is not the current track + content.insert_item(0, "Play", ContextMenuAction::Play(item.as_listitem())); + } else { + // the item is the current track and playing + content.insert_item(0, "Pause", ContextMenuAction::TogglePlayback); + } + content.insert_item( + 1, + "Play next", + ContextMenuAction::PlayNext(item.as_listitem()), + ); + content.insert_item(2, "Queue", ContextMenuAction::Queue(item.as_listitem())); + } + if let Some(artists) = item.artists() { let action = match artists.len() { 0 => None, @@ -244,9 +244,11 @@ impl ContextMenu { ) } } + if let Some(a) = item.album(queue.clone()) { content.add_item("Show album", ContextMenuAction::ShowItem(Box::new(a))); } + #[cfg(feature = "share_clipboard")] { if let Some(url) = item.share_url() { @@ -256,28 +258,27 @@ impl ContextMenu { content.add_item("Share album", ContextMenuAction::ShareUrl(url)); } } + if let Some(t) = item.track() { - content.insert_item( - 0, - "Play track", - ContextMenuAction::PlayTrack(Box::new(t.clone())), - ); content.add_item( "Add to playlist", ContextMenuAction::AddToPlaylist(Box::new(t.clone())), ); content.add_item( "Similar tracks", - ContextMenuAction::ShowRecommendations(Box::new(t.clone())), - ); - content.add_item( - match library.is_saved_track(&Playable::Track(t.clone())) { - true => "Unsave track", - false => "Save track", - }, - ContextMenuAction::ToggleTrackSavedStatus(Box::new(t)), + ContextMenuAction::ShowRecommendations(Box::new(t)), ) } + // If the item is saveable, its save state will be set + if let Some(savestatus) = item.is_saved(library.clone()) { + content.add_item( + match savestatus { + true => "Unsave", + false => "Save", + }, + ContextMenuAction::ToggleSavedStatus(item.as_listitem()), + ); + } // open detail view of artist/album { @@ -288,10 +289,6 @@ impl ContextMenu { s.pop_layer(); match action { - ContextMenuAction::PlayTrack(track) => { - let dialog = Self::play_track_dialog(queue, *track.clone()); - s.add_layer(dialog); - } ContextMenuAction::ShowItem(item) => { if let Some(view) = item.open(queue, library) { s.call_on_name("main", move |v: &mut Layout| v.push_view(view)); @@ -311,10 +308,6 @@ impl ContextMenu { s.call_on_name("main", move |v: &mut Layout| v.push_view(view)); } } - ContextMenuAction::ToggleTrackSavedStatus(track) => { - let mut track: Track = *track.clone(); - track.toggle_saved(library); - } ContextMenuAction::SelectArtist(artists) => { let dialog = Self::select_artist_dialog(library, queue, artists.clone()); s.add_layer(dialog); @@ -324,13 +317,20 @@ impl ContextMenu { Self::select_artist_action_dialog(library, queue, artist.clone()); s.add_layer(dialog); } + ContextMenuAction::ToggleSavedStatus(item) => { + item.as_listitem().toggle_saved(library) + } + ContextMenuAction::Play(item) => item.as_listitem().play(queue), + ContextMenuAction::PlayNext(item) => item.as_listitem().play_next(queue), + ContextMenuAction::TogglePlayback => queue.toggleplayback(), + ContextMenuAction::Queue(item) => item.as_listitem().queue(queue), } }); } let dialog = Dialog::new() .title(item.display_left(library)) - .dismiss_button("Cancel") + .dismiss_button("Close") .padding(Margins::lrtb(1, 1, 1, 0)) .content(content.with_name("contextmenu_select")); Self {