Bug 222760 - strange selection behaviour in left pane
Summary: strange selection behaviour in left pane
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Internet Services (show other bugs)
Version: 2.3-GIT
Platform: Compiled Sources Unspecified
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-14 21:38 UTC by Frederik Schwarzer
Modified: 2010-05-18 11:50 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 2.3.1


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frederik Schwarzer 2010-01-14 21:38:51 UTC
Version:           2.2-GIT (080c603ff) (using Devel)
Installed from:    Compiled sources

The left pane holding the collection, the streams and so on behaves strangely on selections.

E.g.:
open the list of "Cool Streams" and select by dragging from below the stream list upwards until you have several list items selected. Now try to click one item. For me nothing works. Clicking does nothing, dragging alters the initial selection again. Well, doubleclick works. :) However, something is bogus there.

The same happens in the collection. To being able to select from below the list, you might need to enter some search term first.

Regards
Comment 1 Myriam Schweingruber 2010-01-14 22:03:18 UTC
Confirmed with current git head c8e96c0.
In the Cool Streams, one has to unselect each of the selected items before being able to click normally again.
Comment 2 Frederik Schwarzer 2010-02-19 06:30:29 UTC
I still have that problem in recent Git version.

Could you please talk to people who actually had that problem before closing such a report? Most of the bugs reported here work for me, but I do not close them, because there are other people experiencing them anyway.
Comment 3 Rick W. Chen 2010-05-15 20:33:25 UTC
commit 6a9b70cfe6f645d2a75ef65e87e31adf6bb2f45c
Author: Rick W. Chen <stuffcorpse@archlinux.us>
Date:   Sun May 16 03:02:28 2010 +1200

    Remove extra delays of single click operations in CollectionTreeView
    
    If KDE is running in single click mode, a noteable delay occurs when
    e.g. expanding an item, starting drag for the PUD. Under double click
    mode, there's no such delay. Further, this delay is set to
    DoubleClickInterval, which makes the ui seem unresponsive.
    
    This patch removes that delay and fixes some issues related to selection
    of items.
    
    BUG:222760
    CCBUG:226533

diff --git a/ChangeLog b/ChangeLog
index 7ac7979..4120ef2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,10 +5,13 @@ Amarok ChangeLog
 
 VERSION 2.3.1
   CHANGES:
+    * Improved responsiveness when expanding/collapsing items in the
+      collection browser when using single-click mode.
     * The Collection scanner now runs with idle priority when invoked by
       Amarok. Batch scan users can invoke it with the --idlepriority flag.
 
   BUGFIXES:
+    * Fixed strange selection behaviour in the music sources pane (BR 222760).
     * Fixed factor used when filtering lengths in collection browser.
     * Fixed wrong value used when filtering comments in collection browser.
     * Don't truncate the "Label:" label in the TagDialog. (BR 235957)
diff --git a/src/browsers/CollectionTreeItem.cpp b/src/browsers/CollectionTreeItem.cpp
index cc16862..efb7daa 100644
--- a/src/browsers/CollectionTreeItem.cpp
+++ b/src/browsers/CollectionTreeItem.cpp
@@ -388,7 +388,6 @@ CollectionTreeItem::descendentTracks()
 bool
 CollectionTreeItem::allDescendentTracksLoaded() const
 {
-    Meta::TrackPtr track;
     if( isTrackItem() )
         return true;
 
@@ -400,7 +399,6 @@ CollectionTreeItem::allDescendentTracksLoaded() const
         if( !item->allDescendentTracksLoaded() )
             return false;
     }
-
     return true;
 }
 
diff --git a/src/browsers/CollectionTreeView.cpp b/src/browsers/CollectionTreeView.cpp
index 4a87de2..360cac5 100644
--- a/src/browsers/CollectionTreeView.cpp
+++ b/src/browsers/CollectionTreeView.cpp
@@ -66,7 +66,6 @@ CollectionTreeView::CollectionTreeView( QWidget *parent)
     , m_cmSeperator( 0 )
     , m_dragMutex()
     , m_ongoingDrag( false )
-    , m_justDoubleClicked( false )
 {
     setSortingEnabled( true );
     sortByColumn( 0, Qt::AscendingOrder );
@@ -87,8 +86,6 @@ CollectionTreeView::CollectionTreeView( QWidget *parent)
 
     connect( this, SIGNAL( collapsed( const QModelIndex & ) ), SLOT( slotCollapsed( const QModelIndex & ) ) );
     connect( this, SIGNAL( expanded( const QModelIndex & ) ), SLOT( slotExpanded( const QModelIndex & ) ) );
-
-    connect( &m_clickTimer, SIGNAL( timeout() ), this, SLOT( slotClickTimeout() ) );
 }
 
 void CollectionTreeView::setModel( QAbstractItemModel * model )
@@ -277,38 +274,44 @@ void CollectionTreeView::mouseDoubleClickEvent( QMouseEvent *event )
         return;
     }
 
-    QModelIndex origIndex = indexAt( event->pos() );
-    CollectionTreeItem *item = getItemFromIndex( origIndex );
-
-    if( !item )
+    QModelIndex index = indexAt( event->pos() );
+    if( !index.isValid() )
     {
         event->accept();
         return;
     }
 
-    if( event->button() != Qt::LeftButton || event->modifiers()
-        || KGlobalSettings::singleClick() || ( item && item->isTrackItem() ) )
+    if( model()->hasChildren( index ) )
+    {
+        if( event->button() == Qt::LeftButton &&
+            event->modifiers() == Qt::NoModifier )
+        {
+            setExpanded( index, !isExpanded( index ) );
+        }
+    }
+    else
     {
+        CollectionTreeItem *item = getItemFromIndex( index );
         playChildTracks( item, Playlist::AppendAndPlay );
-        update();
-        event->accept();
-        return;
     }
-
-    m_clickTimer.stop();
-    //m_justDoubleClicked is necessary because the mouseReleaseEvent still
-    //comes through, but after the mouseDoubleClickEvent, so we need to tell
-    //mouseReleaseEvent to ignore that one event
-    m_justDoubleClicked = true;
-    setExpanded( origIndex, !isExpanded( origIndex ) );
     event->accept();
 }
 
 void CollectionTreeView::mousePressEvent( QMouseEvent *event )
 {
-    if( KGlobalSettings::singleClick() )
-        setItemsExpandable( false );
-    update();
+    QModelIndex index = indexAt( event->pos() );
+
+    if( index.isValid() &&
+        event->button() == Qt::LeftButton &&
+        event->modifiers() == Qt::NoModifier &&
+        KGlobalSettings::singleClick() &&
+        model()->hasChildren( index ) )
+    {
+        setCurrentIndex( index );
+        setExpanded( index, !isExpanded( index ) );
+        event->accept();
+        return;
+    }
     Amarok::PrettyTreeView::mousePressEvent( event );
 }
 
@@ -318,46 +321,21 @@ void CollectionTreeView::mouseReleaseEvent( QMouseEvent *event )
     {
         connect( m_pd, SIGNAL( fadeHideFinished() ), m_pd, SLOT( deleteLater() ) );
         m_pd->hide();
+        m_pd = 0;
     }
-    m_pd = 0;
 
-    setItemsExpandable( true );
     if( event->button() == Qt::MidButton )
     {
         QModelIndex origIndex = indexAt( event->pos() );
-        CollectionTreeItem *item = getItemFromIndex( origIndex );
-        if ( item )
+        if( origIndex.isValid() )
         {
+            CollectionTreeItem *item = getItemFromIndex( origIndex );
             playChildTracks( item, Playlist::AppendAndPlay );
-            update();
             event->accept();
             return;
         }
     }
-    if( event->button() != Qt::LeftButton
-            || event->modifiers()
-            || selectedIndexes().size() > 1)
-    {
-        Amarok::PrettyTreeView::mousePressEvent( event );
-        update();
-        return;
-    }
-
-    if( m_clickTimer.isActive() || m_justDoubleClicked )
-    {
-        //it's a double-click...so ignore it
-        m_clickTimer.stop();
-        m_justDoubleClicked = false;
-        m_savedClickIndex = QModelIndex();
-        event->accept();
-        return;
-    }
-
-    m_savedClickIndex = indexAt( event->pos() );
-    KConfigGroup cg( KGlobal::config(), "KDE" );
-    m_clickTimer.start( cg.readEntry( "DoubleClickInterval", 400 ) );
-    m_clickLocation = event->pos();
-    event->accept();
+    Amarok::PrettyTreeView::mouseReleaseEvent( event );
 }
 
 void CollectionTreeView::mouseMoveEvent( QMouseEvent *event )
@@ -366,17 +344,8 @@ void CollectionTreeView::mouseMoveEvent( QMouseEvent *event )
     if( event->buttons() || event->modifiers() )
     {
         Amarok::PrettyTreeView::mouseMoveEvent( event );
-        update();
         return;
     }
-
-    QPoint point = event->pos() - m_clickLocation;
-    KConfigGroup cg( KGlobal::config(), "KDE" );
-    if( point.manhattanLength() > cg.readEntry( "StartDragDistance", 4 ) )
-    {
-        m_clickTimer.stop();
-        slotClickTimeout();
-    }
     event->accept();
 }
 
@@ -390,22 +359,12 @@ CollectionTreeItem* CollectionTreeView::getItemFromIndex( QModelIndex &index )
 
     if( !filteredIndex.isValid() )
     {
-        return NULL;
+        return 0;
     }
 
     return static_cast<CollectionTreeItem*>( filteredIndex.internalPointer() );
 }
 
-void CollectionTreeView::slotClickTimeout()
-{
-    m_clickTimer.stop();
-    if( m_savedClickIndex.isValid() && KGlobalSettings::singleClick() )
-    {
-        setExpanded( m_savedClickIndex, !isExpanded( m_savedClickIndex ) );
-    }
-    m_savedClickIndex = QModelIndex();
-}
-
 void CollectionTreeView::keyPressEvent( QKeyEvent *event )
 {
     QModelIndexList indices = selectedIndexes();
@@ -555,8 +514,13 @@ CollectionTreeView::startDrag(Qt::DropActions supportedActions)
 
 void CollectionTreeView::selectionChanged(const QItemSelection & selected, const QItemSelection & deselected)
 {
-    Q_UNUSED( deselected )
     QModelIndexList indexes = selected.indexes();
+
+    QModelIndexList changedIndexes = indexes;
+    changedIndexes << deselected.indexes();
+    foreach( const QModelIndex &index, changedIndexes )
+        update( index );
+
     if ( indexes.count() < 1 )
         return;
 
@@ -566,9 +530,7 @@ void CollectionTreeView::selectionChanged(const QItemSelection & selected, const
     else
         index = indexes[0];
 
-
     CollectionTreeItem * item = static_cast<CollectionTreeItem *>( index.internalPointer() );
-
     emit( itemSelected ( item ) );
 }
 
diff --git a/src/browsers/CollectionTreeView.h b/src/browsers/CollectionTreeView.h
index 4b3de7c..a13ab6c 100644
--- a/src/browsers/CollectionTreeView.h
+++ b/src/browsers/CollectionTreeView.h
@@ -78,7 +78,6 @@ class CollectionTreeView: public Amarok::PrettyTreeView
         void mouseReleaseEvent( QMouseEvent *event );
         void keyPressEvent( QKeyEvent * event );
         void startDrag( Qt::DropActions supportedActions );
-        //void changeEvent ( QEvent * event );
 
     protected slots:
         virtual void selectionChanged ( const QItemSelection & selected, const QItemSelection & deselected );
@@ -87,7 +86,6 @@ class CollectionTreeView: public Amarok::PrettyTreeView
         void slotExpanded( const QModelIndex &index );
 
         void slotCheckAutoExpand();
-        void slotClickTimeout();
         
         void slotPlayChildTracks();
         void slotAppendChildTracks();
@@ -142,10 +140,6 @@ class CollectionTreeView: public Amarok::PrettyTreeView
 
         QMutex m_dragMutex;
         bool m_ongoingDrag;
-        QPoint m_clickLocation;
-        QTimer m_clickTimer;
-        QModelIndex m_savedClickIndex;
-        bool m_justDoubleClicked;
 
     signals:
         void itemSelected( CollectionTreeItem * item );