Bug 134205

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;