Add shortcuts to ease navigation#1835
Add shortcuts to ease navigation#1835Perdu wants to merge 7 commits intostrawberrymusicplayer:masterfrom
Conversation
|
CI run errors are unrelated to the patch |
|
CTRL + End and CTRL + Home is already for moving up and down the playlist without selecting the track, so another combination must be used. I also don't like CTRL + C since that's usually copy to clipboard or interrupt program. CTRL + W works here, it minimizes the window. |
This avoids intercepting Qt behaviours such as closing settings window
It will return to first playlist if none is active
bc0fdc8 to
d849ba7
Compare
OK, I moved them to Ctrl+Shift+End and Ctrl+Shift+Home
Hmm, this does not work for me (on i3, if relevant). I cannot test properly. Edit: once again, CI errors are unrelated (is it because I force-pushed?) |
There was a problem hiding this comment.
Pull request overview
This PR adds keyboard shortcuts to improve playlist tab navigation in the Strawberry music player. However, there are critical bugs that need to be addressed before merging.
Key changes:
- Adds Ctrl+Shift+End to navigate to the last playlist tab
- Adds Ctrl+Shift+Home to navigate to the active (playing/paused) playlist tab
- Adds Ctrl+W to close the current playlist tab (changes existing Ctrl+W window-hide shortcut to Ctrl+H)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/playlist/playlisttabbar.h | Adds CloseCurrentTab method declaration |
| src/playlist/playlisttabbar.cpp | Implements CloseCurrentTab to close the currently selected tab with safety check |
| src/playlist/playlistcontainer.h | Adds method declarations for new navigation functions and updates SetActions signature |
| src/playlist/playlistcontainer.cpp | Implements GoToLastPlaylistTab and GoToActivePlaylistTab; connects new actions to handlers |
| src/core/mainwindow.ui | Defines three new actions for the playlist navigation shortcuts |
| src/core/mainwindow.cpp | Assigns keyboard shortcuts to actions and updates SetActions call; changes window-hide shortcut from Ctrl+W to Ctrl+H |
Critical Issues Found:
- Function signature mismatch between header and implementation files will cause wrong actions to be connected
- Potential crash when navigating to active tab if no playlist is currently active
- Duplicate signal connection causing clear playlist to be triggered twice
- Documentation mismatch between PR description and actual keyboard shortcuts implemented
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Comments addressed. This PR can be reviewed now. |
|
Hello! Will this be reviewed/merged? I've been using it for months and can confirm it's working as expected. |
Notes: