Bug 134205 - Wish: Apply-Button for the Queue-Manager for better Usability
Summary: Wish: Apply-Button for the Queue-Manager for better Usability
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-17 13:50 UTC by Marc Schiffbauer
Modified: 2006-12-17 09:10 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
[Patch] Added Apply button to Queue Manager (8.09 KB, patch)
2006-12-16 07:00 UTC, Warren Koo Tze Mew
Details
[Patch] Apply button for Queue Manager. (12.35 KB, patch)
2006-12-17 01:06 UTC, Warren Koo Tze Mew
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Schiffbauer 2006-09-17 13:50:26 UTC
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
Comment 1 Warren Koo Tze Mew 2006-12-16 07:00:51 UTC
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
Comment 2 Mark Kretschmann 2006-12-16 09:26:21 UTC
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).

Comment 3 Warren Koo Tze Mew 2006-12-17 01:06:50 UTC
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.
Comment 4 Mark Kretschmann 2006-12-17 09:10:38 UTC
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;