Version: (using KDE KDE 3.5.4) Installed from: Ubuntu Packages OS: Linux Hey AmaroK Devs! I would like to see an Apply-Button between the OK and Cancel Buttons in the Queue-Manager. With this one could D'n'D sings into it, change order etc, apply the changes and then continue to work on the queue. At the moment you need to press 'OK' and reopen the Queue-Manager Window. What do you think? Thanks for that lovely App. Cheers -Marc
Created attachment 18951 [details] [Patch] Added Apply button to Queue Manager The patch modified the Queue Manager and Playlist. Queue Manager: -Added Apply button to dialog -Added an extra SLOT for the Apply button to use -Refactored the function called when playlist is updated so that adding items uses In list and removing items uses Out list Playlist: -Refactored updating Playlist from Queue Manager so Ok and Apply buttons can use same function -Modified how the In & Out list are generated when queuing selected items otherwise the Queue Manager receives an incorrect list from the SIGNAL
Warren, good patch, but I have a nitpick: The Apply button should generally be disabled until the user makes a modification. Then it should become enabled. It's an important visual cue that is used everywhere in KDE (e.g. check Amarok's settings dialog).
Created attachment 18957 [details] [Patch] Apply button for Queue Manager. As per Mark's comments, the Apply button is disabled by default and enabled when the user makes a modification. Clicking Apply resets the Apply button to disabled until another modification is made.
SVN commit 614294 by markey: Add Apply-Button for the Queue-Manager. Queue Manager: -Added Apply button to dialog -Added an extra SLOT for the Apply button to use -Refactored the function called when playlist is updated so that adding items uses In list and removing items uses Out list Playlist: -Refactored updating Playlist from Queue Manager so Ok and Apply buttons can use same function -Modified how the In & Out list are generated when queuing selected items otherwise the Queue Manager receives an incorrect list from the SIGNAL Patch by Warren Koo Tze Mew <kootzem@gmail.com>. BUG: 134205 M +29 -18 playlist.cpp M +1 -0 playlist.h M +101 -21 queuemanager.cpp M +10 -1 queuemanager.h --- trunk/extragear/multimedia/amarok/src/playlist.cpp #614293:614294 @@ -1289,11 +1289,16 @@ { // Dequeuing selection with dynamic doesn't work due to the moving of the track after the last queued if( dynamicMode() ) + { + ( !m_nextTracks.containsRef( *it ) ? in : out ).append( *it ); dynamicList.append( *it ); + } else + { queue( *it, true ); + ( m_nextTracks.containsRef( *it ) ? in : out ).append( *it ); + } - ( m_nextTracks.containsRef( *it ) ? in : out ).append( *it ); } if( dynamicMode() ) @@ -3507,26 +3512,32 @@ QueueManager dialog; if( dialog.exec() == QDialog::Accepted ) { - PLItemList oldQueue = m_nextTracks; - m_nextTracks = dialog.newQueue(); + changeFromQueueManager(dialog.newQueue()); + } +} - PLItemList in, out; - // make sure we repaint items no longer queued - for( PlaylistItem* item = oldQueue.first(); item; item = oldQueue.next() ) - if( !m_nextTracks.containsRef( item ) ) - out << item; - for( PlaylistItem* item = m_nextTracks.first(); item; item = m_nextTracks.next() ) - if( !oldQueue.containsRef( item ) ) - in << item; +void +Playlist::changeFromQueueManager(QPtrList<PlaylistItem> list) +{ + PLItemList oldQueue = m_nextTracks; + m_nextTracks = list; - emit queueChanged( in, out ); + PLItemList in, out; + // make sure we repaint items no longer queued + for( PlaylistItem* item = oldQueue.first(); item; item = oldQueue.next() ) + if( !m_nextTracks.containsRef( item ) ) + out << item; + for( PlaylistItem* item = m_nextTracks.first(); item; item = m_nextTracks.next() ) + if( !oldQueue.containsRef( item ) ) + in << item; - // repaint newly queued or altered queue items - if( dynamicMode() ) - sortQueuedItems(); - else - refreshNextTracks(); - } + emit queueChanged( in, out ); + + // repaint newly queued or altered queue items + if( dynamicMode() ) + sortQueuedItems(); + else + refreshNextTracks(); } void --- trunk/extragear/multimedia/amarok/src/playlist.h #614293:614294 @@ -235,6 +235,7 @@ void setStopAfterMode( int mode ); void showCurrentTrack() { ensureItemCentered( m_currentTrack ); } void showQueueManager(); + void changeFromQueueManager(QPtrList<PlaylistItem> list); void shuffle(); void undo(); void updateMetaData( const MetaBundle& ); --- trunk/extragear/multimedia/amarok/src/queuemanager.cpp #614293:614294 @@ -11,6 +11,9 @@ * * ***************************************************************************/ +#define DEBUG_PREFIX "QueueManager" +#include "debug.h" + #include "amarok.h" #include "amarokconfig.h" //check if dynamic mode #include "playlist.h" @@ -154,6 +157,7 @@ QueueList::moveSelectedUp() // SLOT { QPtrList<QListViewItem> selected = selectedItems(); + bool item_moved = false; // Whilst it would be substantially faster to do this: ((*it)->itemAbove())->move( *it ), // this would only work for sequentially ordered items @@ -169,15 +173,20 @@ after = ( item->itemAbove() )->itemAbove(); moveItem( item, 0, after ); + item_moved = true; } ensureItemVisible( selected.first() ); + + if( item_moved ) + emit changed(); } void QueueList::moveSelectedDown() // SLOT { QPtrList<QListViewItem> list = selectedItems(); + bool item_moved = false; for( QListViewItem *item = list.last(); item; item = list.prev() ) { @@ -187,9 +196,13 @@ continue; moveItem( item, 0, after ); + item_moved = true; } ensureItemVisible( list.last() ); + + if( item_moved ) + emit changed(); } void @@ -197,24 +210,40 @@ { setSelected( currentItem(), true ); + bool item_removed = false; QPtrList<QListViewItem> selected = selectedItems(); for( QListViewItem *item = selected.first(); item; item = selected.next() ) + { delete item; + item_removed = true; + } if( isEmpty() ) QueueManager::instance()->updateButtons(); + + if( item_removed ) + emit changed(); } void +QueueList::clear() // SLOT +{ + KListView::clear(); + emit changed(); +} + +void QueueList::contentsDragEnterEvent( QDragEnterEvent *e ) { + debug() << "contentsDrageEnterEvent()" << endl; e->accept( e->source() == reinterpret_cast<KListView*>( Playlist::instance() )->viewport() ); } void QueueList::contentsDragMoveEvent( QDragMoveEvent *e ) { + debug() << "contentsDrageMoveEvent()" << endl; KListView::contentsDragMoveEvent( e ); // Must be overloaded for dnd to work @@ -225,9 +254,12 @@ void QueueList::contentsDropEvent( QDropEvent *e ) { + debug() << "contentsDragDropEvent()" << endl; if( e->source() == viewport() ) + { KListView::contentsDropEvent( e ); - + emit changed(); + } else { QListViewItem *parent = 0; @@ -247,7 +279,7 @@ QueueManager *QueueManager::s_instance = 0; QueueManager::QueueManager( QWidget *parent, const char *name ) - : KDialogBase( KDialogBase::Swallow, 0, parent, name, false, 0, Ok|Cancel ) + : KDialogBase( KDialogBase::Swallow, 0, parent, name, false, 0, Ok|Apply|Cancel ) { s_instance = this; @@ -295,7 +327,10 @@ connect( pl, SIGNAL( selectionChanged() ), SLOT( updateButtons() ) ); connect( m_listview, SIGNAL( selectionChanged() ), SLOT( updateButtons() ) ); connect( pl, SIGNAL( queueChanged(const PLItemList &, const PLItemList &) ), - SLOT( addQueuedItems(const PLItemList &, const PLItemList &) ) ); + SLOT( changeQueuedItems(const PLItemList &, const PLItemList &) ) ); + connect( this, SIGNAL( applyClicked()), SLOT( applyNow() ) ); + connect( m_listview, SIGNAL( changed() ), this, SLOT ( changed() ) ); + s_instance->enableButtonApply(false); insertItems(); } @@ -306,6 +341,14 @@ } void +QueueManager::applyNow() +{ + Playlist *pl = Playlist::instance(); + pl->changeFromQueueManager( newQueue() ); + s_instance->enableButtonApply(false); +} + +void QueueManager::addItems( QListViewItem *after ) { /* @@ -317,12 +360,12 @@ - After a drag, those items are still selected in the playlist, so we can find out which PlaylistItems were dragged by selectedItems(); */ - if( !after ) after = m_listview->lastChild(); QPtrList<QListViewItem> list = Playlist::instance()->selectedItems(); + bool item_added = false; for( QListViewItem *item = list.first(); item; item = list.next() ) { #define item static_cast<PlaylistItem*>(item) @@ -334,19 +377,22 @@ after = new QueueItem( m_listview, after, title ); m_map[ after ] = item; + item_added = true; } #undef item } + if( item_added ) + emit m_listview->changed(); } void -QueueManager::addQueuedItems( const PLItemList &in, const PLItemList &out ) //SLOT +QueueManager::changeQueuedItems( const PLItemList &in, const PLItemList &out ) //SLOT { QPtrListIterator<PlaylistItem> it(in); for( it.toFirst(); it; ++it ) addQueuedItem( *it ); it = QPtrListIterator<PlaylistItem>(out); - for( it.toFirst(); it; ++it ) addQueuedItem( *it ); + for( it.toFirst(); it; ++it ) removeQueuedItem( *it ); } void @@ -377,29 +423,50 @@ after = new QueueItem( m_listview, after, title ); m_map[ after ] = item; } - else //track is in the queue, remove it. +} + +void +QueueManager::removeQueuedItem( PlaylistItem *item ) +{ + Playlist *pl = Playlist::instance(); + if( !pl ) return; //should never happen + + const int index = pl->m_nextTracks.findRef( item ); + + QListViewItem *after; + if( !index ) after = 0; + else { - QListViewItem *removableItem = m_listview->findItem( title, 0 ); + int find = m_listview->childCount(); + if( index - 1 <= find ) + find = index - 1; + after = m_listview->itemAtIndex( find ); + } - if( removableItem ) + QValueList<PlaylistItem*> current = m_map.values(); + QValueListIterator<PlaylistItem*> newItem = current.find( item ); + + QString title = i18n("%1 - %2").arg( item->artist(), item->title() ); + + QListViewItem *removableItem = m_listview->findItem( title, 0 ); + + if( removableItem ) + { + //Remove the key from the map, so we can re-queue the item + QMapIterator<QListViewItem*, PlaylistItem*> end( m_map.end() ); + for( QMapIterator<QListViewItem*, PlaylistItem*> it = m_map.begin(); it != end; ++it ) { - //Remove the key from the map, so we can re-queue the item - QMapIterator<QListViewItem*, PlaylistItem*> end( m_map.end() ); - for( QMapIterator<QListViewItem*, PlaylistItem*> it = m_map.begin(); it != end; ++it ) + if( it.data() == item ) { - if( it.data() == item ) - { - m_map.remove( it ); + m_map.remove( it ); - //Remove the item from the queuelist - m_listview->takeItem( removableItem ); - delete removableItem; - return; - } + //Remove the item from the queuelist + m_listview->takeItem( removableItem ); + delete removableItem; + return; } } } - } /// Playlist uses this to determine the altered queue and reflect the changes. @@ -433,10 +500,19 @@ } void +QueueManager::changed() // SLOT +{ + s_instance->enableButtonApply(true); +} + + +void QueueManager::removeSelected() //SLOT { QPtrList<QListViewItem> selected = m_listview->selectedItems(); + bool item_removed = false; + for( QListViewItem *item = selected.first(); item; item = selected.next() ) { //Remove the key from the map, so we can re-queue the item @@ -447,7 +523,11 @@ //Remove the item from the queuelist m_listview->takeItem( item ); delete item; + item_removed = true; } + + if( item_removed ) + emit m_listview->changed(); } void --- trunk/extragear/multimedia/amarok/src/queuemanager.h #614293:614294 @@ -38,6 +38,8 @@ { Q_OBJECT + friend class QueueManager; + public: QueueList( QWidget *parent, const char *name = 0 ); ~QueueList() {}; @@ -50,6 +52,7 @@ void moveSelectedUp(); void moveSelectedDown(); void removeSelected(); + virtual void clear(); private: void contentsDragEnterEvent( QDragEnterEvent *e ); @@ -57,6 +60,9 @@ void contentsDropEvent( QDropEvent *e ); void keyPressEvent( QKeyEvent *e ); void viewportPaintEvent( QPaintEvent* ); + + signals: + void changed(); }; class QueueManager : public KDialogBase @@ -72,16 +78,19 @@ static QueueManager *instance() { return s_instance; } public slots: + void applyNow(); void addItems( QListViewItem *after = 0 ); /// For the add button (uses selected playlist tracks) - void addQueuedItems( const PLItemList &in, const PLItemList &out ); /// For keeping queue/dequeue in sync + void changeQueuedItems( const PLItemList &in, const PLItemList &out ); /// For keeping queue/dequeue in sync void updateButtons(); private slots: void removeSelected(); + void changed(); private: void insertItems(); void addQueuedItem( PlaylistItem *item ); + void removeQueuedItem( PlaylistItem *item ); QMap<QListViewItem*, PlaylistItem*> m_map; QueueList *m_listview;