Review Request: Playlist Queue Editor

[Andreas Hartmetz]

> On 2010-11-28 23:46:36, Leo Franchi wrote:
> > Ok, a few comments from using it. Here are issues I think need to be addressed:
> > 
> > * I think the action should be centered like the other actions below the playlist, probable with a separator.
> > * I think the default size of the dialog is too big, especially given how small the items are
> > * Lacking icon for 3rd/bottom button
> > * The list is sort of lacking. It should at least show the Artist as well, maybe like Artist - Title.
> > * List doesn't update on track progression, but jumps ahead once a button is pressed and tracks have progressed
> > * Dialog is modal---it shouldn't be, so the user can also add/remove stuff from the queue with the queue manager open. Hand in hand with this is the fact that the playlist should update to reflect any changes made in the dialog immediately
> > 
> > And these are issues I think are nice but not required for version 1:
> > 
> > * Tracks should be re-arrangable by drag-n-drop
> > * Tracks from the playlist should be draggable onto the Queue Manager to queue them at a specific place
> > 
> > cheers,
> > leo

* Listview item text with too little information: fixedName() looked like it would change the least, but I think you're right: displaying useful information in the common case is more important. Queueing streams that often have no defined end is not a common thing to do.
* List not updating on track progression: I expected the queueChanged() signal from the playlist because track progression effectively dequeues a track. I'll try to fix it in the playlist.
* Modality: Intentional so the playlist doesn't change too much while the dialog exists, possibly causing bugs. I expect that one would usually click on items in the playlist to enqueue them - you will likely discover them there with no dialog open (because it's just annoying to have another window you don't need most of the time). Then you edit the queue. I don't use dynamic playlist though, so if the playlist can change anyway while the dialog is up it could as well be non-modal.
* Updating the playlist: this probably means emitting the right signal at the right place. Any ideas?


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100177/#review436
-----------------------------------------------------------


On 2010-11-29 00:00:22, Andreas Hartmetz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100177/
> -----------------------------------------------------------
> 
> (Updated 2010-11-29 00:00:22)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> This adds a Queue Editor, with limited features (move up / move down / clear) like in Amarok 1.
> The KAction for it is not very well placed in the main UI, I hope you guys can help me.
> A few design changes were necessary, too, as you can see in the patch.
> 
> 
> This addresses bug 198180.
>     https://bugs.kde.org/show_bug.cgi?id=198180
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt c5ee1e3 
>   src/MainWindow.cpp f755b6f 
>   src/likeback/LikeBackBar.cpp 6964f88 
>   src/playlist/PlaylistActions.h a0b2275 
>   src/playlist/PlaylistActions.cpp d9284e7 
>   src/playlist/PlaylistController.cpp f05c250 
>   src/playlist/PlaylistDock.h f7f1231 
>   src/playlist/PlaylistDock.cpp 6170f78 
>   src/playlist/PlaylistItem.h 6bfeb68 
>   src/playlist/PlaylistModel.h 7b7633f 
>   src/playlist/PlaylistModel.cpp 0162321 
>   src/playlist/PlaylistQueueEditor.h PRE-CREATION 
>   src/playlist/PlaylistQueueEditor.cpp PRE-CREATION 
>   src/playlist/PlaylistQueueEditor.ui PRE-CREATION 
>   src/playlist/navigators/DynamicTrackNavigator.cpp a1c67e7 
>   src/playlist/navigators/TrackNavigator.h 6154f9a 
>   src/playlist/navigators/TrackNavigator.cpp 2a70b08 
>   src/playlist/proxymodels/AbstractModel.h 03ca4d6 
>   src/playlist/proxymodels/ProxyBase.h 2e6f9e7 
>   src/playlist/proxymodels/ProxyBase.cpp 37cb8cb 
>   src/playlist/view/PlaylistViewCommon.cpp 2a5c9f6 
>   src/playlist/view/listview/PrettyItemDelegate.cpp 5494279 
>   src/playlist/view/listview/PrettyListView.cpp 0b2e41e 
>   src/statusbar/StatusBar.cpp 1a67677 
> 
> Diff: http://git.reviewboard.kde.org/r/100177/diff
> 
> 
> Testing
> -------
> 
> "worksforme"
> 
> 
> Thanks,
> 
> Andreas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101129/944248dc/attachment.htm