Skip to content

Play playlist 'CTRL+o' added #488 #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ionescuaandrei wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Play playlist 'CTRL+o' added #488 #695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38ec4e9
Play playlist 'CTRL+o' added #488
ionescuaandrei Mar 23, 2025
9d26e9e
Merge branch 'master' into feature/add-play#488
aome510 Mar 25, 2025
e8314db
Play playlist 'CTRL+o' added (fixing the CI)#488
ionescuaandrei Mar 26, 2025
a58077b
Merge branch 'feature/add-play#488' of https://github.com/ionescuaand…
ionescuaandrei Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion spotify_player/src/client/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ impl Client {
playback.is_playing = true;
}
}

PlayerRequest::Pause => {
if playback.is_playing {
self.pause_playback(device_id).await?;
Expand Down Expand Up @@ -301,6 +300,7 @@ impl Client {
PlayerRequest::TransferPlayback(..) => {
anyhow::bail!("`TransferPlayback` should be handled earlier")
}
PlayerRequest::PlayCurrentPlaylist => todo!(),
Copy link
Owner

@aome510 aome510 Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this for?

};

Ok(Some(playback))
Expand Down
1 change: 1 addition & 0 deletions spotify_player/src/client/request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub enum PlayerRequest {
Resume,
Pause,
ResumePause,
PlayCurrentPlaylist,
SeekTrack(chrono::Duration),
Repeat,
Shuffle,
Expand Down
10 changes: 7 additions & 3 deletions spotify_player/src/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub enum Command {
MovePlaylistItemDown,

CreatePlaylist,
PlaySelectedPlaylist,
}

#[derive(Clone, Copy, Debug, Deserialize)]
Expand All @@ -102,6 +103,7 @@ pub enum Action {
CopyLink,
Follow,
Unfollow,
PlayContext,
}

#[derive(Debug)]
Expand Down Expand Up @@ -214,6 +216,7 @@ pub fn construct_track_actions(track: &Track, data: &DataReadGuard) -> Vec<Actio
/// constructs a list of actions on an album
pub fn construct_album_actions(album: &Album, data: &DataReadGuard) -> Vec<Action> {
let mut actions = vec![
Action::PlayContext,
Action::GoToArtist,
Action::GoToRadio,
Action::ShowActionsOnArtist,
Expand All @@ -230,7 +233,7 @@ pub fn construct_album_actions(album: &Album, data: &DataReadGuard) -> Vec<Actio

/// constructs a list of actions on an artist
pub fn construct_artist_actions(artist: &Artist, data: &DataReadGuard) -> Vec<Action> {
let mut actions = vec![Action::GoToRadio, Action::CopyLink];
let mut actions = vec![Action::PlayContext, Action::GoToRadio, Action::CopyLink];

if data
.user_data
Expand All @@ -247,7 +250,7 @@ pub fn construct_artist_actions(artist: &Artist, data: &DataReadGuard) -> Vec<Ac

/// constructs a list of actions on an playlist
pub fn construct_playlist_actions(playlist: &Playlist, data: &DataReadGuard) -> Vec<Action> {
let mut actions = vec![Action::GoToRadio, Action::CopyLink];
let mut actions = vec![Action::PlayContext, Action::GoToRadio, Action::CopyLink];

if data
.user_data
Expand All @@ -264,7 +267,7 @@ pub fn construct_playlist_actions(playlist: &Playlist, data: &DataReadGuard) ->

/// constructs a list of actions on a show
pub fn construct_show_actions(show: &Show, data: &DataReadGuard) -> Vec<Action> {
let mut actions = vec![Action::CopyLink];
let mut actions = vec![Action::PlayContext, Action::CopyLink];
if data.user_data.saved_shows.iter().any(|s| s.id == show.id) {
actions.push(Action::DeleteFromLibrary);
} else {
Expand Down Expand Up @@ -356,6 +359,7 @@ impl Command {
Self::MovePlaylistItemUp => "move playlist item up one position",
Self::MovePlaylistItemDown => "move playlist item down one position",
Self::CreatePlaylist => "create a new playlist",
Self::PlaySelectedPlaylist => "play the selected playlist",
Self::VolumeChange { offset: _ } => unreachable!(),
}
.to_string()
Expand Down
4 changes: 4 additions & 0 deletions spotify_player/src/config/keymap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,10 @@ impl Default for KeymapConfig {
key_sequence: "g c".into(),
command: Command::JumpToCurrentTrackInContext,
},
Keymap {
key_sequence: "C-o".into(),
command: Command::PlaySelectedPlaylist,
},
],
}
}
Expand Down
71 changes: 71 additions & 0 deletions spotify_player/src/event/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ fn handle_key_event(
}
};

// Example key binding: Alt+P to play the current playlist
if &key_sequence.keys[..] == [Key::Alt(crossterm::event::KeyCode::Char('p'))] {
client_pub.send(ClientRequest::Player(PlayerRequest::PlayCurrentPlaylist))?;
// Mark this event as handled and reset the key sequence
ui.input_key_sequence.keys.clear();
return Ok(());
}
Comment on lines +121 to +127
Copy link
Owner

@aome510 aome510 Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no this is not a correct pattern. You should define a command with keybindings and handle event based on the command


// if the key sequence is not handled, let the global handler handle it
let handled = if handled {
true
Expand Down Expand Up @@ -253,6 +261,14 @@ pub fn handle_action_in_context(
ui.popup = None;
Ok(true)
}
Action::PlayContext => {
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::URIs(vec![track.id.clone().into()], None),
None,
)))?;
ui.popup = None;
Ok(true)
}
_ => Ok(false),
},
ActionContext::Album(album) => match action {
Expand Down Expand Up @@ -295,6 +311,15 @@ pub fn handle_action_in_context(
ui.popup = None;
Ok(true)
}
Action::PlayContext => {
let context_id = ContextId::Album(album.id.clone());
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::Context(context_id, None),
None,
)))?;
ui.popup = None;
Ok(true)
}
_ => Ok(false),
},
ActionContext::Artist(artist) => match action {
Expand Down Expand Up @@ -324,6 +349,15 @@ pub fn handle_action_in_context(
})?;
Ok(true)
}
Action::PlayContext => {
let context_id = ContextId::Artist(artist.id.clone());
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::Context(context_id, None),
None,
)))?;
ui.popup = None;
Ok(true)
}
_ => Ok(false),
},
ActionContext::Playlist(playlist) => match action {
Expand Down Expand Up @@ -356,6 +390,15 @@ pub fn handle_action_in_context(
ui.popup = None;
Ok(true)
}
Action::PlayContext => {
let context_id = ContextId::Playlist(playlist.id.clone());
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::Context(context_id, None),
None,
)))?;
ui.popup = None;
Ok(true)
}
_ => Ok(false),
},
ActionContext::Show(show) => match action {
Expand All @@ -375,6 +418,15 @@ pub fn handle_action_in_context(
ui.popup = None;
Ok(true)
}
Action::PlayContext => {
let context_id = ContextId::Show(show.id.clone());
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::Context(context_id, None),
None,
)))?;
ui.popup = None;
Ok(true)
}
_ => Ok(false),
},
ActionContext::Episode(episode) => match action {
Expand Down Expand Up @@ -428,6 +480,25 @@ pub fn handle_action_in_context(
}
Ok(false)
}
Action::PlayContext => {
if let Some(show) = &episode.show {
let context_id = ContextId::Show(show.id.clone());
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::Context(
context_id,
Some(rspotify::model::Offset::Uri(episode.id.uri())),
),
None,
)))?;
} else {
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::URIs(vec![episode.id.clone().into()], None),
None,
)))?;
}
ui.popup = None;
Ok(true)
}
_ => Ok(false),
},
// TODO: support actions for playlist folders
Expand Down
19 changes: 19 additions & 0 deletions spotify_player/src/event/page.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,25 @@ fn handle_command_for_context_page(
ui.new_search_popup();
Ok(true)
}
Command::PlaySelectedPlaylist => {
if let PageState::Context {
context_page_type: ContextPageType::Browsing(context_id),
..
} = ui.current_page()
{
if let ContextId::Playlist(_) = context_id {
client_pub.send(ClientRequest::Player(PlayerRequest::StartPlayback(
Playback::Context(context_id.clone(), None),
None,
)))?;
Ok(true)
} else {
Ok(false)
}
} else {
Ok(false)
}
}
Comment on lines +301 to +319
Copy link
Owner

@aome510 aome510 Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the logic doesn't seem right. You trigger this command in a playlist context page, so it's more like PlayCurrentPlaylist. I think this command's functionality is covered by PlayRandom command which supports all context not just playlist context.

For PlaySelectedPlaylist command, I expect that you can trigger it on a selected item in a playlist list. You did implement Action::PlayContext already for selected playlist so probably don't need to define a separate command

_ => window::handle_command_for_focused_context_window(command, client_pub, ui, state),
}
}
Expand Down
16 changes: 13 additions & 3 deletions spotify_player/src/ui/page.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,14 +287,24 @@ pub fn render_context_page(
let data = state.data.read();
match data.caches.context.get(&id.uri()) {
Some(context) => {
// render context description
let chunks = Layout::vertical([Constraint::Length(1), Constraint::Fill(0)]).split(rect);
let chunks = Layout::vertical([Constraint::Length(2), Constraint::Fill(0)]).split(rect);
// Render context description
frame.render_widget(
Paragraph::new(context.description()).style(ui.theme.page_desc()),
chunks[0],
);
let rect = chunks[1];

// Insert a "Play Context" button
let button_rect = Layout::horizontal([Constraint::Length(14), Constraint::Fill(0)])
.split(chunks[0])[0];
frame.render_widget(
Paragraph::new("Playing")
.alignment(tui::layout::Alignment::Center)
.style(ui.theme.app()),
button_rect,
);

let rect = chunks[1];
match context {
Context::Artist {
top_tracks,
Expand Down
Loading