From 7fdb43575d44c83d3a3de73926d469399992ebc5 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 11 Aug 2021 01:48:58 +0200 Subject: [PATCH 1/4] Library sidebar: fix playlist selection Remove unnecessary & conflicting code. Previously Up/Down key presses in the sidebar would NOT select the pevious/next playlist (Playlists, History) but make the selection jump to Tracks (and sometimes apply the key press after that). --- src/library/trackset/baseplaylistfeature.cpp | 9 --------- src/library/trackset/crate/cratefeature.cpp | 1 + 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 956d68026aa..6c5fe973e04 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -179,17 +179,9 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { if (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) { @@ -201,7 +193,6 @@ void BasePlaylistFeature::activatePlaylist(int playlistId) { if (!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 index 1485fa77213..25b645901a6 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -279,6 +279,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; From 8b10db9f2da01c7b4d6f260ca1478d9c538085f5 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 11 Aug 2021 01:58:43 +0200 Subject: [PATCH 2/4] BasePlaylistFeature: use debug assertions --- src/library/trackset/baseplaylistfeature.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 6c5fe973e04..3718f9d0bd8 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -176,7 +176,7 @@ 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); @@ -186,11 +186,11 @@ void BasePlaylistFeature::activateChild(const QModelIndex& 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()) { + VERIFY_OR_DEBUG_ASSERT(index.isValid()) { return; } m_lastRightClickedIndex = index; From 42d925a94855c5d49657db80b61bd91300a92819 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 13 Aug 2021 02:09:53 +0200 Subject: [PATCH 3/4] Sidebar: fix item selection after delete, create, rename --- src/library/trackset/baseplaylistfeature.cpp | 2 +- src/library/trackset/crate/cratefeature.cpp | 2 ++ src/widget/wlibrarysidebar.cpp | 11 ++++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 3718f9d0bd8..276906c65bb 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -185,11 +185,11 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { } void BasePlaylistFeature::activatePlaylist(int playlistId) { - // qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId; VERIFY_OR_DEBUG_ASSERT(playlistId != kInvalidPlaylistId) { return; } QModelIndex index = indexFromPlaylistId(playlistId); + //qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId << index; VERIFY_OR_DEBUG_ASSERT(index.isValid()) { return; } diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 25b645901a6..0ba8cf30cc1 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -395,6 +395,8 @@ void CrateFeature::slotDeleteCrate() { qWarning() << "Refusing to delete locked crate" << crate; return; } + // TODO Store sibling index to restore selection after crate was deleted + // to avoid scroll position reset (to Crate root item) if (m_pTrackCollection->deleteCrate(crate.getId())) { qDebug() << "Deleted crate" << crate; return; diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 6a083ad3dc4..5e866367b99 100644 --- 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(); From 185e373fdcac64d26ad59d69586a15764203e97c Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 13 Aug 2021 20:57:33 +0200 Subject: [PATCH 4/4] Crates: restore sidebar selection position after crate deletion --- src/library/trackset/crate/cratefeature.cpp | 30 +++++++++++++++++---- src/library/trackset/crate/cratefeature.h | 5 ++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 0ba8cf30cc1..aa4bd3c7c43 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -395,9 +395,11 @@ void CrateFeature::slotDeleteCrate() { qWarning() << "Refusing to delete locked crate" << crate; return; } - // TODO Store sibling index to restore selection after crate was deleted - // to avoid scroll position reset (to Crate root item) - 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 +770,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 index 57e2b80e18e..ea5c6c3f50d 100644 --- 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; TrackPointer m_pSelectedTrack;