[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [amarok] /: CollectionBrowser: do not expand tree needlessly
From: Matěj_Laitl <matej () laitl ! cz>
Date: 2014-05-12 21:05:56
Message-ID: E1WjxQ4-0007aK-MC () scm ! kde ! org
[Download RAW message or body]
Git commit 3b5c6b2039ad943ae209341b5920fee929da9320 by Matěj Laitl.
Committed on 12/05/2014 at 21:01.
Pushed by laitl into branch 'master'.
CollectionBrowser: do not expand tree needlessly
The reasons were 2:
* we were adding every collection to m_expandedCollections all the
time, which then resulted in their expansion in each slotFilter()
* we were doing auto-expansion even in cases where collection was
updated, not just when user typed something
(fixed in is still 2.9 for consistency with earlier commits)
BACKPORT
BUG: 300557
FIXED-IN: 2.9
M +1 -0 ChangeLog
M +21 -16 src/browsers/CollectionTreeItemModelBase.cpp
M +8 -3 src/browsers/CollectionTreeItemModelBase.h
M +5 -5 src/browsers/CollectionTreeView.cpp
M +1 -1 src/browsers/CollectionTreeView.h
M +1 -1 tests/browsers/TestSingleCollectionTreeItemModel.cpp
http://commits.kde.org/amarok/3b5c6b2039ad943ae209341b5920fee929da9320
diff --git a/ChangeLog b/ChangeLog
index 103b108..a0365ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -23,6 +23,7 @@ VERSION 2.8.1
compilers currently supported by KDE.
BUGFIXES:
+ * Collection Browser no longer excessively expands the tree. (BR 300557)
* Properly calculate and store Aft tags in mp4 files. Patch By Stefano
Pettini <stefano.pettini@gmail.com> (BR 332811)
* Update Progress Slider when restoring from System Tray. Patch By Abhay
diff --git a/src/browsers/CollectionTreeItemModelBase.cpp \
b/src/browsers/CollectionTreeItemModelBase.cpp index e45efd3..da35825 100644
--- a/src/browsers/CollectionTreeItemModelBase.cpp
+++ b/src/browsers/CollectionTreeItemModelBase.cpp
@@ -68,6 +68,7 @@ CollectionTreeItemModelBase::CollectionTreeItemModelBase( )
, m_loading1( QPixmap( KStandardDirs::locate("data", \
"amarok/images/loading1.png" ) ) )
, m_loading2( QPixmap( KStandardDirs::locate("data", \
"amarok/images/loading2.png" ) ) ) , m_currentAnimPixmap( m_loading1 )
+ , m_autoExpand( false )
{
m_timeLine = new QTimeLine( 10000, this );
m_timeLine->setFrameRange( 0, 20 );
@@ -644,7 +645,8 @@ CollectionTreeItemModelBase::queryDone()
//stop timer if there are no more animations active
if( m_runningQueries.isEmpty() )
{
- emit allQueriesFinished();
+ emit allQueriesFinished( m_autoExpand );
+ m_autoExpand = false; // reset to default value
m_timeLine->stop();
}
qm->deleteLater();
@@ -851,10 +853,6 @@ CollectionTreeItemModelBase::handleNormalQueryResult( \
Collections::QueryMaker *q
//simply insert the item, nothing will change if it is already in \
the set m_expandedItems.insert( parent->data() );
}
- else
- {
- m_expandedCollections.insert( parent->parentCollection() );
- }
}
}
@@ -1081,14 +1079,16 @@ void
CollectionTreeItemModelBase::setCurrentFilter( const QString &filter )
{
m_currentFilter = filter;
- slotFilter();
+ slotFilter( /* autoExpand */ true );
}
void
-CollectionTreeItemModelBase::slotFilter()
+CollectionTreeItemModelBase::slotFilter( bool autoExpand )
{
+ m_autoExpand = autoExpand;
filterChildren();
+ // following is not auto-expansion, it is restoring the state before filtering
foreach( Collections::Collection *expanded, m_expandedCollections )
{
CollectionTreeItem *expandedItem = m_collections.value( \
expanded->collectionId() ).second; @@ -1127,22 +1127,27 @@ \
CollectionTreeItemModelBase::slotCollapsed( const QModelIndex &index ) void
CollectionTreeItemModelBase::slotExpanded( const QModelIndex &index )
{
- if( index.isValid() )
+ if( !index.isValid() )
+ return;
+
+ CollectionTreeItem *item = static_cast<CollectionTreeItem*>( \
index.internalPointer() ); + // we are really only interested in the special nodes \
here. + // we have to remember whether the user expanded a various artists/no \
labels node or not. + // otherwise we won't be able to automatically expand the \
special node after filtering again + // there is exactly one special node per type \
per collection, so use the collection to store that information +
+ // we also need to store collection expansion state here as they are no longer
+ // added to th expanded set in handleNormalQueryResult()
+ switch( item->type() )
{
- CollectionTreeItem *item = static_cast<CollectionTreeItem*>( \
index.internalPointer() );
- //we are really only interested in the special nodes here.
- //we have to remember whether the user expanded a various artists/no labels \
node or not.
- //otherwise we won't be able to automatically expand the special node after \
filtering again
- //there is exactly one special node per type per collection, so use the \
collection to store that information
- switch( item->type() )
- {
case CollectionTreeItem::VariousArtist:
case CollectionTreeItem::NoLabel:
m_expandedSpecialNodes.insert( item->parentCollection() );
break;
+ case CollectionTreeItem::Collection:
+ m_expandedCollections.insert( item->parentCollection() );
default:
break;
- }
}
}
diff --git a/src/browsers/CollectionTreeItemModelBase.h \
b/src/browsers/CollectionTreeItemModelBase.h index d326908..8517755 100644
--- a/src/browsers/CollectionTreeItemModelBase.h
+++ b/src/browsers/CollectionTreeItemModelBase.h
@@ -87,7 +87,7 @@ class AMAROK_EXPORT CollectionTreeItemModelBase : public \
QAbstractItemModel
/**
* Return true if there are any queries still running. If this returns true,
- * you can expect allQueriesFinished() signal in some time.
+ * you can expect allQueriesFinished(bool) signal in some time.
*/
bool hasRunningQueries() const;
@@ -98,7 +98,7 @@ class AMAROK_EXPORT CollectionTreeItemModelBase : public \
QAbstractItemModel
signals:
void expandIndex( const QModelIndex &index );
- void allQueriesFinished();
+ void allQueriesFinished( bool autoExpand );
public slots:
virtual void queryDone();
@@ -113,8 +113,12 @@ class AMAROK_EXPORT CollectionTreeItemModelBase : public \
QAbstractItemModel
/**
* Apply the current filter.
+ *
+ * @param autoExpand whether to trigger automatic expansion of the tree \
after + * filtering is done. This should be set to true only if filter is run \
after + * user has actually just typed something and defaults to false.
*/
- void slotFilter();
+ void slotFilter( bool autoExpand = false );
void slotCollapsed( const QModelIndex &index );
void slotExpanded( const QModelIndex &index );
@@ -174,6 +178,7 @@ class AMAROK_EXPORT CollectionTreeItemModelBase : public \
QAbstractItemModel
mutable QHash<Collections::QueryMaker* , CollectionTreeItem* > \
m_compilationQueries;
mutable QHash<Collections::QueryMaker* , CollectionTreeItem* > \
m_noLabelsQueries;
mutable QMultiHash<CollectionTreeItem*, Collections::QueryMaker*> \
m_runningQueries; + bool m_autoExpand; // whether to expand tree after queries \
are done
protected slots:
void startAnimationTick();
diff --git a/src/browsers/CollectionTreeView.cpp \
b/src/browsers/CollectionTreeView.cpp index 7649647..10b5cbc 100644
--- a/src/browsers/CollectionTreeView.cpp
+++ b/src/browsers/CollectionTreeView.cpp
@@ -95,7 +95,7 @@ CollectionTreeView::setModel( QAbstractItemModel *model )
if( !m_treeModel )
return;
- connect( m_treeModel, SIGNAL(allQueriesFinished()), SLOT(slotCheckAutoExpand()) \
); + connect( m_treeModel, SIGNAL(allQueriesFinished(bool)), \
SLOT(slotCheckAutoExpand(bool)) ); connect( m_treeModel, \
SIGNAL(expandIndex(QModelIndex)), SLOT(slotExpandIndex(QModelIndex)) );
@@ -652,9 +652,9 @@ CollectionTreeView::slotExpandIndex( const QModelIndex &index )
}
void
-CollectionTreeView::slotCheckAutoExpand()
+CollectionTreeView::slotCheckAutoExpand( bool reallyExpand )
{
- if( !m_filterModel )
+ if( !m_filterModel || !reallyExpand )
return;
// Cases where root is not collections but
@@ -911,12 +911,12 @@ CollectionTreeView::slotAddFilteredTracksToPlaylist()
return;
// disconnect any possible earlier connection we've done
- disconnect( m_treeModel, SIGNAL(allQueriesFinished()),
+ disconnect( m_treeModel, SIGNAL(allQueriesFinished(bool)),
this, SLOT(slotAddFilteredTracksToPlaylist()) );
if( m_treeModel->hasRunningQueries() )
// wait for the queries to finish
- connect( m_treeModel, SIGNAL(allQueriesFinished()),
+ connect( m_treeModel, SIGNAL(allQueriesFinished(bool)),
this, SLOT(slotAddFilteredTracksToPlaylist()) );
else
{
diff --git a/src/browsers/CollectionTreeView.h b/src/browsers/CollectionTreeView.h
index 13f18c2..fc9f732 100644
--- a/src/browsers/CollectionTreeView.h
+++ b/src/browsers/CollectionTreeView.h
@@ -104,7 +104,7 @@ class CollectionTreeView: public Amarok::PrettyTreeView
void slotExpanded( const QModelIndex &index );
void slotExpandIndex( const QModelIndex &index );
- void slotCheckAutoExpand();
+ void slotCheckAutoExpand( bool reallyExpand = true );
void slotReplacePlaylistWithChildTracks();
void slotAppendChildTracks();
diff --git a/tests/browsers/TestSingleCollectionTreeItemModel.cpp \
b/tests/browsers/TestSingleCollectionTreeItemModel.cpp index f741b31..f014179 100644
--- a/tests/browsers/TestSingleCollectionTreeItemModel.cpp
+++ b/tests/browsers/TestSingleCollectionTreeItemModel.cpp
@@ -141,7 +141,7 @@ TestSingleCollectionTreeItemModel::initTestCase()
{ \
if( itemModel->canFetchMore( idx ) ) { \
itemModel->fetchMore( idx ); \
- QVERIFY( QTest::kWaitForSignal( itemModel, SIGNAL(allQueriesFinished()), \
5000 ) ); \ + QVERIFY( QTest::kWaitForSignal( itemModel, \
SIGNAL(allQueriesFinished(bool)), 5000 ) ); \ } \
}
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic