Merge pull request #4193 from ronso0/playlist-selection-fix

Sidebar item selection fix
mixxxdj · Aug 17, 2021 · 54a68c4 · 54a68c4
2 parents fd79c05 + 185e373
commit 54a68c4
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 17 deletions.
diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp
@@ -179,32 +179,23 @@ void BasePlaylistFeature::selectPlaylistInSidebar(int playlistId, bool select) {
 void BasePlaylistFeature::activateChild(const QModelIndex& index) {
     //qDebug() << "BasePlaylistFeature::activateChild()" << index;
     int playlistId = playlistIdFromIndex(index);
-    if (playlistId == kInvalidPlaylistId) {
+    VERIFY_OR_DEBUG_ASSERT(playlistId != kInvalidPlaylistId) {
         return;
     }
-
     m_pPlaylistTableModel->setTableModel(playlistId);
     emit showTrackModel(m_pPlaylistTableModel);
     emit enableCoverArtDisplay(true);
-    // Update selection
-    emit featureSelect(this, m_lastRightClickedIndex);
-
-    if (!m_pSidebarWidget) {
-        return;
-    }
-    m_pSidebarWidget->selectChildIndex(index);
 }
 
 void BasePlaylistFeature::activatePlaylist(int playlistId) {
-    // qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId;
-    if (playlistId == kInvalidPlaylistId) {
+    VERIFY_OR_DEBUG_ASSERT(playlistId != kInvalidPlaylistId) {
         return;
     }
     QModelIndex index = indexFromPlaylistId(playlistId);
-    if (!index.isValid()) {
+    //qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId << index;
+    VERIFY_OR_DEBUG_ASSERT(index.isValid()) {
         return;
     }
-
     m_lastRightClickedIndex = index;
     m_pPlaylistTableModel->setTableModel(playlistId);
     emit showTrackModel(m_pPlaylistTableModel);

diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp
@@ -282,6 +282,7 @@ TreeItemModel* CrateFeature::sidebarModel() const {
 }
 
 void CrateFeature::activateChild(const QModelIndex& index) {
+    //qDebug() << "CrateFeature::activateChild()" << index;
     CrateId crateId(crateIdFromIndex(index));
     VERIFY_OR_DEBUG_ASSERT(crateId.isValid()) {
         return;
@@ -397,7 +398,11 @@ void CrateFeature::slotDeleteCrate() {
             qWarning() << "Refusing to delete locked crate" << crate;
             return;
         }
-        if (m_pTrackCollection->deleteCrate(crate.getId())) {
+        CrateId crateId = crate.getId();
+        // Store sibling id to restore selection after crate was deleted
+        // to avoid the scroll position being reset to Crate root item.
+        storePrevSiblingCrateId(crateId);
+        if (m_pTrackCollection->deleteCrate(crateId)) {
             qDebug() << "Deleted crate" << crate;
             return;
         }
@@ -768,16 +773,34 @@ void CrateFeature::slotExportTrackFiles() {
     track_export.exportTracks();
 }
 
+void CrateFeature::storePrevSiblingCrateId(CrateId crateId) {
+    QModelIndex actIndex = indexFromCrateId(crateId);
+    for (int i = (actIndex.row() + 1); i >= (actIndex.row() - 1); i -= 2) {
+        QModelIndex newIndex = actIndex.sibling(i, actIndex.column());
+        if (newIndex.isValid()) {
+            TreeItem* pTreeItem = m_pSidebarModel->getItem(newIndex);
+            DEBUG_ASSERT(pTreeItem != nullptr);
+            if (!pTreeItem->hasChildren()) {
+                m_prevSiblingCrate = crateIdFromIndex(newIndex);
+            }
+        }
+    }
+}
+
 void CrateFeature::slotCrateTableChanged(CrateId crateId) {
     if (m_lastRightClickedIndex.isValid() &&
             (crateIdFromIndex(m_lastRightClickedIndex) == crateId)) {
-        // Preserve crate selection
+        // Try to restore previous selection
         m_lastRightClickedIndex = rebuildChildModel(crateId);
         if (m_lastRightClickedIndex.isValid()) {
+            // Select last active crate
             activateCrate(crateId);
+        } else if (m_prevSiblingCrate.isValid()) {
+            // Select neighbour of deleted crate
+            activateCrate(m_prevSiblingCrate);
         }
     } else {
-        // Discard crate selection
+        // No valid selection to restore
         rebuildChildModel();
     }
 }

diff --git a/src/library/trackset/crate/cratefeature.h b/src/library/trackset/crate/cratefeature.h
@@ -102,6 +102,11 @@ class CrateFeature : public BaseTrackSetFeature {
 
     CrateTableModel m_crateTableModel;
 
+    // Stores the id of a crate in the sidebar that is adjacent to the crate(crateId).
+    void storePrevSiblingCrateId(CrateId crateId);
+    // Can be used to restore a similiar selection after the sidebar model was rebuilt.
+    CrateId m_prevSiblingCrate;
+
     QModelIndex m_lastRightClickedIndex;
     TrackId m_selectedTrackId;
 

diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp
@@ -212,15 +212,20 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) {
 }
 
 void WLibrarySidebar::selectIndex(const QModelIndex& index) {
+    //qDebug() << "WLibrarySidebar::selectIndex" << index;
+    if (!index.isValid()) {
+        return;
+    }
     auto* pModel = new QItemSelectionModel(model());
     pModel->select(index, QItemSelectionModel::Select);
     if (selectionModel()) {
         selectionModel()->deleteLater();
     }
-    setSelectionModel(pModel);
     if (index.parent().isValid()) {
         expand(index.parent());
     }
+    setSelectionModel(pModel);
+    setCurrentIndex(index);
     scrollTo(index);
 }
 
@@ -232,6 +237,9 @@ void WLibrarySidebar::selectChildIndex(const QModelIndex& index, bool selectItem
         return;
     }
     QModelIndex translated = sidebarModel->translateChildIndex(index);
+    if (!translated.isValid()) {
+        return;
+    }
 
     if (selectItem) {
         auto* pModel = new QItemSelectionModel(sidebarModel);
@@ -240,6 +248,7 @@ void WLibrarySidebar::selectChildIndex(const QModelIndex& index, bool selectItem
             selectionModel()->deleteLater();
         }
         setSelectionModel(pModel);
+        setCurrentIndex(translated);
     }
 
     QModelIndex parentIndex = translated.parent();