Play playlist 'CTRL+o' added #488 #695
Conversation
|
@ionescuaandrei can you fix the CI fail? |
|
Sure I will, check and have a look |
aome510
left a comment
aome510
left a comment
There was a problem hiding this comment.
Thanks for updating but it seems CIs still fail
| // 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(()); | ||
| } |
There was a problem hiding this comment.
no this is not a correct pattern. You should define a command with keybindings and handle event based on the command
| PlayerRequest::TransferPlayback(..) => { | ||
| anyhow::bail!("`TransferPlayback` should be handled earlier") | ||
| } | ||
| PlayerRequest::PlayCurrentPlaylist => todo!(), |
| 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) | ||
| } | ||
| } |
There was a problem hiding this comment.
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
2 participants
This pull request introduces new playback functionalities and key bindings to the Spotify player, enhancing the user experience by allowing more control over playback actions. The most important changes include adding new commands and actions for playing selected playlists and contexts, as well as updating key bindings and UI elements to support these new features.
New Playback Functionalities:
PlayCurrentPlaylisttoPlayerRequestenum inspotify_player/src/client/request.rsto handle playing the current playlist.PlayCurrentPlaylistinClientinspotify_player/src/client/mod.rs.New Commands and Actions:
PlaySelectedPlaylisttoCommandenum andPlayContexttoActionenum inspotify_player/src/command.rs. [1] [2]PlayContextfor various entities like albums, artists, playlists, and shows inspotify_player/src/command.rs. [1] [2] [3] [4]Key Bindings and Event Handling:
Alt+Pto play the current playlist inspotify_player/src/event/mod.rs.PlayContextaction in different contexts (album, artist, playlist, show, episode) inspotify_player/src/event/mod.rs. [1] [2] [3] [4] [5] [6]UI Updates:
render_context_pagefunction to include a "Play Context" button in the UI inspotify_player/src/ui/page.rs.PlaySelectedPlaylistinKeymapConfiginspotify_player/src/config/keymap.rs.[Copilot is generating a summary...]