320129 2013-05-22 10:36:15 +0000 JJ: Tracks cannot be dragged around when playlist was shuffled 2013-05-31 12:23:59 +0000 1 1 2 Applications amarok Playlist 2.7-git Other Linux RESOLVED FIXED junior-jobs NOR minor 2.8 1 matej amarok-bugs-null konrad.zemek teo http://commits.kde.org/amarok/5659f0f8ad73a2894ebe8d5da3f7925e75ce6901 2.8 0 oldest_to_newest 1370398 0 matej 2013-05-22 10:36:15 +0000 When you click Shuffle "sorting" in playlist sort widget, the tracks get shuffled, but you cannot move one track around in the playlist. This is because the shuffle *action* is implementing as a *state*, which is IMO suboptimal. I suggest that we make the Shuffle an action instead. It would actually change track order of the bottom model of the playlist, not just *sorting* of the QSortFilterProxyModel. Speaking about the ui, I think we can retain the Shuffle button in the playlist sorting panel, but it would just appear for a short interval upon clicking (0.5 a second? whatever looks good) and then disappear. The action needs to be undoable using the playlist undo button. Marking this as a junior job. Konrad, you've previously touched this code, perhaps you'd like to try this one, too? 1370421 1 konrad.zemek 2013-05-22 11:41:19 +0000 Sure, I'll take a look at it once I have some free time (today maybe, friday at the latest). Additionaly, a way to hint that the "Shuffle" entry works in a different way would be to place a separator between this entry and entries above... but that's just a minor detail. I'm a bit torn about the button appearing, though. I think it may very well not show at all, as the effects of shuffing should be immediately visible anyway. 1370423 2 matej 2013-05-22 11:48:17 +0000 (In reply to comment #1) > Sure, I'll take a look at it once I have some free time (today maybe, friday > at the latest). No need to hurry. :) > Additionaly, a way to hint that the "Shuffle" entry works in a different way > would be to place a separator between this entry and entries above... but > that's just a minor detail. Good idea. > I'm a bit torn about the button appearing, > though. I think it may very well not show at all, as the effects of shuffing > should be immediately visible anyway. You're right. Just try it locally and then submit what works better for your eye. 1371429 3 konrad.zemek 2013-05-27 00:20:50 +0000 https://git.reviewboard.kde.org/r/110658/ 1372811 4 matej 2013-05-31 12:23:59 +0000 Git commit 5659f0f8ad73a2894ebe8d5da3f7925e75ce6901 by Matěj Laitl, on behalf of Konrad Zemek. Committed on 31/05/2013 at 14:21. Pushed by laitl into branch 'master'. Playlist sort widget: reimplement Shuffle "sort" as an action. Remove now-unnecessary Shuffle handlers in sort-related functions. Additionally, PlaylistController::moveRows() has been modified to help with big move actions (validation complexity reduced). Playlist::BreadcrumbItemMenu introduced to deduplicate logic between Playlist::BreadcrumbItem and Playlist::BreadcrumbAddMenuButton. GUI: In the playlist sort widget, the Shuffle menu entry is now separated from other entries. Activating the entry no longer results in a "Shuffle" sort level being added. FIXED-IN: 2.8 REVIEW: 110658 CCMAIL: amarok-promo@kde.org M +1 -0 ChangeLog M +21 -0 src/playlist/PlaylistActions.cpp M +5 -0 src/playlist/PlaylistActions.h M +60 -64 src/playlist/PlaylistBreadcrumbItem.cpp M +56 -26 src/playlist/PlaylistBreadcrumbItem.h M +32 -47 src/playlist/PlaylistBreadcrumbItemSortButton.cpp M +1 -2 src/playlist/PlaylistBreadcrumbItemSortButton.h M +2 -27 src/playlist/PlaylistBreadcrumbLevel.cpp M +0 -1 src/playlist/PlaylistBreadcrumbLevel.h M +9 -7 src/playlist/PlaylistController.cpp M +12 -2 src/playlist/PlaylistController.h M +0 -4 src/playlist/PlaylistDefines.h M +8 -4 src/playlist/PlaylistModel.cpp M +20 -15 src/playlist/PlaylistSortWidget.cpp M +6 -1 src/playlist/PlaylistSortWidget.h M +4 -69 src/playlist/proxymodels/SortAlgorithms.cpp M +0 -22 src/playlist/proxymodels/SortAlgorithms.h M +0 -2 src/playlist/proxymodels/SortScheme.cpp M +24 -24 tests/playlist/TestPlaylistModels.cpp M +3 -1 tests/playlist/TestPlaylistModels.h http://commits.kde.org/amarok/5659f0f8ad73a2894ebe8d5da3f7925e75ce6901