From bd9656d0d16b9cc984194eb0deb0e4aa43e5cb51 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 24 Jan 2023 01:37:16 +0100 Subject: [PATCH 01/44] Library playlist features: use kInvalidPlaylistId instead of -1 --- src/library/banshee/bansheeplaylistmodel.cpp | 4 ++-- src/library/baseexternallibraryfeature.cpp | 2 +- src/library/baseexternalplaylistmodel.cpp | 8 +++---- src/library/dao/playlistdao.cpp | 23 ++++++++++---------- src/library/dao/playlistdao.h | 2 ++ src/library/itunes/itunesfeature.cpp | 2 +- src/library/playlisttablemodel.cpp | 4 ++-- src/library/rekordbox/rekordboxfeature.cpp | 6 ++--- src/library/trackset/baseplaylistfeature.h | 2 -- src/library/traktor/traktorfeature.cpp | 2 +- 10 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/library/banshee/bansheeplaylistmodel.cpp b/src/library/banshee/bansheeplaylistmodel.cpp index 8e9b802d4b3..0115f1e63f1 100644 --- a/src/library/banshee/bansheeplaylistmodel.cpp +++ b/src/library/banshee/bansheeplaylistmodel.cpp @@ -66,7 +66,7 @@ const QString kCrate = QStringLiteral(CLM_CRATE); BansheePlaylistModel::BansheePlaylistModel(QObject* pParent, TrackCollectionManager* pTrackCollectionManager, BansheeDbConnection* pConnection) : BaseSqlTableModel(pParent, pTrackCollectionManager, "mixxx.db.model.banshee_playlist"), m_pConnection(pConnection), - m_playlistId(-1) { + m_playlistId(kInvalidPlaylistId) { m_tempTableName = BANSHEE_TABLE + QString::number(sTableNumber.fetchAndAddAcquire(1)); } @@ -77,7 +77,7 @@ BansheePlaylistModel::~BansheePlaylistModel() { void BansheePlaylistModel::dropTempTable() { if (m_playlistId >= 0) { // Clear old playlist - m_playlistId = -1; + m_playlistId = kInvalidPlaylistId; QSqlQuery query(m_database); QString strQuery("DROP TABLE IF EXISTS %1"); if (!query.exec(strQuery.arg(m_tempTableName))) { diff --git a/src/library/baseexternallibraryfeature.cpp b/src/library/baseexternallibraryfeature.cpp index 53d34909c67..7fe4c8751eb 100644 --- a/src/library/baseexternallibraryfeature.cpp +++ b/src/library/baseexternallibraryfeature.cpp @@ -114,7 +114,7 @@ void BaseExternalLibraryFeature::slotImportAsMixxxPlaylist() { int playlistId = playlistDao.createUniquePlaylist(&playlist); - if (playlistId != -1) { + if (playlistId != kInvalidPlaylistId) { playlistDao.appendTracksToPlaylist(trackIds, playlistId); } else { // Do not change strings here without also changing strings in diff --git a/src/library/baseexternalplaylistmodel.cpp b/src/library/baseexternalplaylistmodel.cpp index 6f6477357e2..54cf8ba12e0 100644 --- a/src/library/baseexternalplaylistmodel.cpp +++ b/src/library/baseexternalplaylistmodel.cpp @@ -24,7 +24,7 @@ BaseExternalPlaylistModel::BaseExternalPlaylistModel(QObject* parent, m_playlistsTable(playlistsTable), m_playlistTracksTable(playlistTracksTable), m_trackSource(trackSource), - m_currentPlaylistId(-1) { + m_currentPlaylistId(kInvalidPlaylistId) { } BaseExternalPlaylistModel::~BaseExternalPlaylistModel() { @@ -100,20 +100,20 @@ void BaseExternalPlaylistModel::setPlaylist(const QString& playlist_path) { } // TODO(XXX): Why not last-insert id? - int playlistId = -1; + int playlistId = kInvalidPlaylistId; QSqlRecord finder_query_record = finder_query.record(); while (finder_query.next()) { playlistId = finder_query.value(finder_query_record.indexOf("id")).toInt(); } - if (playlistId == -1) { + if (playlistId == kInvalidPlaylistId) { qWarning() << "ERROR: Could not get the playlist ID for playlist:" << playlist_path; return; } // Store search text QString currSearch = currentSearch(); - if (m_currentPlaylistId != -1 && !currSearch.trimmed().isEmpty()) { + if (m_currentPlaylistId != kInvalidPlaylistId && !currSearch.trimmed().isEmpty()) { m_searchTexts.insert(m_currentPlaylistId, currSearch); } diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index c5bc96d4e96..a8475afca07 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -62,7 +62,7 @@ int PlaylistDAO::createPlaylist(const QString& name, const HiddenType hidden) { if (!query.exec()) { LOG_FAILED_QUERY(query); - return -1; + return kInvalidPlaylistId; } //Get the id of the last playlist. @@ -83,7 +83,7 @@ int PlaylistDAO::createPlaylist(const QString& name, const HiddenType hidden) { if (!query.exec()) { LOG_FAILED_QUERY(query); - return -1; + return kInvalidPlaylistId; } int playlistId = query.lastInsertId().toInt(); @@ -97,10 +97,10 @@ int PlaylistDAO::createUniquePlaylist(QString* pName, const HiddenType hidden) { int playlistId = getPlaylistIdFromName(*pName); int i = 1; - if (playlistId != -1) { + if (playlistId != kInvalidPlaylistId) { // Calculate a unique name *pName += "(%1)"; - while (playlistId != -1) { + while (playlistId != kInvalidPlaylistId) { i++; playlistId = getPlaylistIdFromName(pName->arg(i)); } @@ -165,7 +165,7 @@ int PlaylistDAO::getPlaylistIdFromName(const QString& name) const { } else { LOG_FAILED_QUERY(query); } - return -1; + return kInvalidPlaylistId; } void PlaylistDAO::deletePlaylist(const int playlistId) { @@ -441,7 +441,7 @@ int PlaylistDAO::getPlaylistId(const int index) const { if (!query.exec()) { LOG_FAILED_QUERY(query); - return -1; + return kInvalidPlaylistId; } int currentRow = 0; @@ -451,7 +451,7 @@ int PlaylistDAO::getPlaylistId(const int index) const { return id; } } - return -1; + return kInvalidPlaylistId; } PlaylistDAO::HiddenType PlaylistDAO::getHiddenType(const int playlistId) const { @@ -765,15 +765,14 @@ int PlaylistDAO::getPreviousPlaylist(const int currentPlaylistId, HiddenType hid if (!query.exec()) { LOG_FAILED_QUERY(query); - return -1; + return kInvalidPlaylistId; } // Get the id of the highest playlist - int previousPlaylistId = -1; if (query.next()) { - previousPlaylistId = query.value(query.record().indexOf("id")).toInt(); + return query.value(query.record().indexOf("id")).toInt(); } - return previousPlaylistId; + return kInvalidPlaylistId; } bool PlaylistDAO::copyPlaylistTracks(const int sourcePlaylistID, const int targetPlaylistID) { @@ -1196,7 +1195,7 @@ void PlaylistDAO::setAutoDJProcessor(AutoDJProcessor* pAutoDJProcessor) { void PlaylistDAO::addTracksToAutoDJQueue(const QList& trackIds, AutoDJSendLoc loc) { int iAutoDJPlaylistId = getPlaylistIdFromName(AUTODJ_TABLE); - if (iAutoDJPlaylistId == -1) { + if (iAutoDJPlaylistId == kInvalidPlaylistId) { return; } diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index 16903dd76c8..754dcceab3f 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -28,6 +28,8 @@ const QString PLAYLISTTRACKSTABLE_DATETIMEADDED = QStringLiteral("pl_datetime_ad class AutoDJProcessor; +constexpr int kInvalidPlaylistId = -1; + class PlaylistDAO : public QObject, public virtual DAO { Q_OBJECT public: diff --git a/src/library/itunes/itunesfeature.cpp b/src/library/itunes/itunesfeature.cpp index b6fbdb9dc91..496ae65bf0a 100644 --- a/src/library/itunes/itunesfeature.cpp +++ b/src/library/itunes/itunesfeature.cpp @@ -710,7 +710,7 @@ void ITunesFeature::parsePlaylist(QXmlStreamReader& xml, QSqlQuery& query_insert //qDebug() << "Parse Playlist"; QString playlistname; - int playlist_id = -1; + int playlist_id = kInvalidPlaylistId; int playlist_position = -1; int track_reference = -1; //indicates that we haven't found the < diff --git a/src/library/playlisttablemodel.cpp b/src/library/playlisttablemodel.cpp index 8d5f613a367..37a31cb9984 100644 --- a/src/library/playlisttablemodel.cpp +++ b/src/library/playlisttablemodel.cpp @@ -18,7 +18,7 @@ PlaylistTableModel::PlaylistTableModel(QObject* parent, const char* settingsNamespace, bool keepDeletedTracks) : TrackSetTableModel(parent, pTrackCollectionManager, settingsNamespace), - m_iPlaylistId(-1), + m_iPlaylistId(kInvalidPlaylistId), m_keepDeletedTracks(keepDeletedTracks) { connect(&m_pTrackCollectionManager->internalCollection()->getPlaylistDAO(), &PlaylistDAO::tracksChanged, @@ -129,7 +129,7 @@ void PlaylistTableModel::setTableModel(int playlistId) { } // Store search text QString currSearch = currentSearch(); - if (m_iPlaylistId != -1 && !currSearch.trimmed().isEmpty()) { + if (m_iPlaylistId != kInvalidPlaylistId && !currSearch.trimmed().isEmpty()) { m_searchTexts.insert(m_iPlaylistId, currSearch); } diff --git a/src/library/rekordbox/rekordboxfeature.cpp b/src/library/rekordbox/rekordboxfeature.cpp index 75ed800d019..c38c37d8c6b 100644 --- a/src/library/rekordbox/rekordboxfeature.cpp +++ b/src/library/rekordbox/rekordboxfeature.cpp @@ -289,7 +289,7 @@ QString getText(rekordbox_pdb_t::device_sql_string_t* deviceString) { } int createDevicePlaylist(QSqlDatabase& database, const QString& devicePath) { - int playlistID = -1; + int playlistID = kInvalidPlaylistId; QSqlQuery queryInsertIntoDevicePlaylist(database); queryInsertIntoDevicePlaylist.prepare( @@ -726,7 +726,7 @@ void buildPlaylistTree( return; } - int playlistID = -1; + int playlistID = kInvalidPlaylistId; while (idQuery.next()) { playlistID = idQuery.value(idQuery.record().indexOf("id")).toInt(); } @@ -796,7 +796,7 @@ void clearDeviceTables(QSqlDatabase& database, TreeItem* child) { ScopedTransaction transaction(database); int trackID = -1; - int playlistID = -1; + int playlistID = kInvalidPlaylistId; QSqlQuery tracksQuery(database); tracksQuery.prepare("select id from " + kRekordboxLibraryTable + " where device=:device"); tracksQuery.bindValue(":device", child->getLabel()); diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 044a4c81d34..1c9845ac76d 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -21,8 +21,6 @@ class TrackCollectionManager; class TreeItem; class WLibrarySidebar; -constexpr int kInvalidPlaylistId = -1; - class BasePlaylistFeature : public BaseTrackSetFeature { Q_OBJECT diff --git a/src/library/traktor/traktorfeature.cpp b/src/library/traktor/traktorfeature.cpp index e6d524d6d5c..0cb61d2dd6d 100644 --- a/src/library/traktor/traktorfeature.cpp +++ b/src/library/traktor/traktorfeature.cpp @@ -513,7 +513,7 @@ void TraktorFeature::parsePlaylistEntries( } //playlist_id = id_query.lastInsertId().toInt(); - int playlist_id = -1; + int playlist_id = kInvalidPlaylistId; const int idColumn = id_query.record().indexOf("id"); while (id_query.next()) { playlist_id = id_query.value(idColumn).toInt(); From bbe8112f4d4c91f6cd4323be86eeadfb2108f5ba Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 24 Jan 2023 23:07:11 +0100 Subject: [PATCH 02/44] BasePlaylistFeature: move DAO connections to separate function --- src/library/trackset/baseplaylistfeature.cpp | 26 +++++++++++--------- src/library/trackset/baseplaylistfeature.h | 1 + 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index f7f7bc70813..65b2d04414f 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -52,6 +52,18 @@ BasePlaylistFeature::BasePlaylistFeature( pModel->setParent(this); initActions(); + connectPlaylistDAO(); + connect(m_pLibrary, + &Library::trackSelected, + this, + [this](const TrackPointer& pTrack) { + const auto trackId = pTrack ? pTrack->getId() : TrackId{}; + slotTrackSelected(trackId); + }); + connect(m_pLibrary, + &Library::switchToView, + this, + &BasePlaylistFeature::slotResetSelectedTrack); } void BasePlaylistFeature::initActions() { @@ -130,7 +142,9 @@ void BasePlaylistFeature::initActions() { &QAction::triggered, this, &BasePlaylistFeature::slotExportTrackFiles); +} +void BasePlaylistFeature::connectPlaylistDAO() { connect(&m_playlistDao, &PlaylistDAO::added, this, @@ -151,18 +165,6 @@ void BasePlaylistFeature::initActions() { &PlaylistDAO::renamed, this, &BasePlaylistFeature::slotPlaylistTableRenamed); - - connect(m_pLibrary, - &Library::trackSelected, - this, - [this](const TrackPointer& pTrack) { - const auto trackId = pTrack ? pTrack->getId() : TrackId{}; - slotTrackSelected(trackId); - }); - connect(m_pLibrary, - &Library::switchToView, - this, - &BasePlaylistFeature::slotResetSelectedTrack); } int BasePlaylistFeature::playlistIdFromIndex(const QModelIndex& index) { diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 1c9845ac76d..47f9d5e2edd 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -122,6 +122,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature { private: void initActions(); + void connectPlaylistDAO(); virtual QString getRootViewHtml() const = 0; void markTreeItem(TreeItem* pTreeItem); From 9748e3618d2b61e34e8524a74a84e3e4057d355a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 24 Jan 2023 23:19:20 +0100 Subject: [PATCH 03/44] Library sidebar/features: some fixes, minor cleanup, improve comments --- src/library/dao/playlistdao.cpp | 2 -- src/library/sidebarmodel.cpp | 21 ++++++++++---------- src/library/trackset/baseplaylistfeature.cpp | 16 ++++++++------- src/library/trackset/crate/cratefeature.cpp | 17 ++++++++-------- src/library/trackset/setlogfeature.cpp | 13 +++++------- src/widget/wlibrarysidebar.cpp | 16 ++++++++------- 6 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index a8475afca07..35a6c9431bc 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -754,8 +754,6 @@ void PlaylistDAO::addPlaylistToAutoDJQueue(const int playlistId, AutoDJSendLoc l } int PlaylistDAO::getPreviousPlaylist(const int currentPlaylistId, HiddenType hidden) const { - // Find out the highest position existing in the playlist so we know what - // position this track should have. QSqlQuery query(m_database); query.prepare(QStringLiteral( "SELECT max(id) as id FROM Playlists " diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 1cef8f14b4c..0f170dd0b11 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -13,10 +13,10 @@ namespace { -// The time between selecting and activating (= clicking) a feature item -// in the sidebar tree. This is essential to allow smooth scrolling through -// a list of items with an encoder or the keyboard! A value of 300 ms has -// been chosen as a compromise between usability and responsiveness. +/// The time between selecting and activating (= clicking) a feature item +/// in the sidebar tree. This is essential to allow smooth scrolling through +/// a list of items with an encoder or the keyboard! A value of 300 ms has +/// been chosen as a compromise between usability and responsiveness. constexpr int kPressedUntilClickedTimeoutMillis = 300; } // anonymous namespace @@ -290,6 +290,8 @@ void SidebarModel::slotPressedUntilClickedTimeout() { } } +/// Connected to WLibrarySidebar::pressed signal, called after left click, +/// right click and selection change via keyboard or controller void SidebarModel::pressed(const QModelIndex& index) { stopPressedUntilClickedTimer(); if (index.isValid()) { @@ -317,6 +319,7 @@ void SidebarModel::clicked(const QModelIndex& index) { } } +/// Invoked by double click and click on tree node expand icons void SidebarModel::doubleClicked(const QModelIndex& index) { stopPressedUntilClickedTimer(); if (index.isValid()) { @@ -337,9 +340,7 @@ void SidebarModel::rightClicked(const QPoint& globalPos, const QModelIndex& inde if (index.isValid()) { if (index.internalPointer() == this) { m_sFeatures[index.row()]->onRightClick(globalPos); - } - else - { + } else { TreeItem* pTreeItem = static_cast(index.internalPointer()); if (pTreeItem) { LibraryFeature* pFeature = pTreeItem->feature(); @@ -406,7 +407,7 @@ bool SidebarModel::dropAccept(const QModelIndex& index, const QList& urls, bool SidebarModel::hasTrackTable(const QModelIndex& index) const { if (index.internalPointer() == this) { - return m_sFeatures[index.row()]->hasTrackTable(); + return m_sFeatures[index.row()]->hasTrackTable(); } return false; } @@ -429,7 +430,7 @@ bool SidebarModel::dragMoveAccept(const QModelIndex& index, const QUrl& url) { return result; } -// Translates an index from the child models to an index of the sidebar models +/// Translates an index from the child models to an index of the sidebar models QModelIndex SidebarModel::translateSourceIndex(const QModelIndex& index) { /* These method is called from the slot functions below. * QObject::sender() return the object which emitted the signal @@ -454,8 +455,6 @@ QModelIndex SidebarModel::translateIndex( TreeItem* pItem = static_cast(index.internalPointer()); translatedIndex = createIndex(index.row(), index.column(), pItem); } else { - //Comment from Tobias Rafreider --> Dead Code???? - for (int i = 0; i < m_sFeatures.size(); ++i) { if (m_sFeatures[i]->sidebarModel() == pModel) { translatedIndex = createIndex(i, 0, this); diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 65b2d04414f..ef3fa6785e1 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -190,7 +190,7 @@ void BasePlaylistFeature::selectPlaylistInSidebar(int playlistId, bool select) { return; } QModelIndex index = indexFromPlaylistId(playlistId); - if (index.isValid() && m_pSidebarWidget) { + if (index.isValid()) { m_pSidebarWidget->selectChildIndex(index, select); } } @@ -246,7 +246,7 @@ void BasePlaylistFeature::slotRenamePlaylist() { if (locked) { qDebug() << "Skipping playlist rename because playlist" << playlistId - << "is locked."; + << oldName << "is locked."; return; } QString newName; @@ -585,7 +585,7 @@ void BasePlaylistFeature::slotExportPlaylist() { "mixxx.db.model.playlist_export")); emit saveModelState(); - pPlaylistTableModel->setTableModel(m_pPlaylistTableModel->getPlaylist()); + pPlaylistTableModel->setTableModel(playlistId); pPlaylistTableModel->setSort( pPlaylistTableModel->fieldIndex( ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), @@ -621,13 +621,17 @@ void BasePlaylistFeature::slotExportPlaylist() { } void BasePlaylistFeature::slotExportTrackFiles() { + int playlistId = playlistIdFromIndex(m_lastRightClickedIndex); + if (playlistId == kInvalidPlaylistId) { + return; + } QScopedPointer pPlaylistTableModel( new PlaylistTableModel(this, m_pLibrary->trackCollectionManager(), "mixxx.db.model.playlist_export")); emit saveModelState(); - pPlaylistTableModel->setTableModel(m_pPlaylistTableModel->getPlaylist()); + pPlaylistTableModel->setTableModel(playlistId); pPlaylistTableModel->setSort(pPlaylistTableModel->fieldIndex( ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), Qt::AscendingOrder); @@ -732,9 +736,7 @@ void BasePlaylistFeature::updateChildModel(int playlistId) { } } -/** - * Clears the child model dynamically, but the invisible root item remains - */ +/// Clears the child model dynamically, but the invisible root item remains void BasePlaylistFeature::clearChildModel() { m_lastRightClickedIndex = QModelIndex(); m_pSidebarModel->removeRows(0, m_pSidebarModel->rowCount()); diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index fb44211fde5..f9e088937d1 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -164,11 +164,11 @@ void CrateFeature::connectLibrary(Library* pLibrary) { } void CrateFeature::connectTrackCollection() { - connect(m_pTrackCollection, + connect(m_pTrackCollection, // created new, duplicated or imported playlist to new crate &TrackCollection::crateInserted, this, &CrateFeature::slotCrateTableChanged); - connect(m_pTrackCollection, + connect(m_pTrackCollection, // renamed, un/locked, toggled AutoDJ source &TrackCollection::crateUpdated, this, &CrateFeature::slotCrateTableChanged); @@ -176,7 +176,7 @@ void CrateFeature::connectTrackCollection() { &TrackCollection::crateDeleted, this, &CrateFeature::slotCrateTableChanged); - connect(m_pTrackCollection, + connect(m_pTrackCollection, // crate tracks hidden, unhidden or purged &TrackCollection::crateTracksChanged, this, &CrateFeature::slotCrateContentChanged); @@ -723,10 +723,10 @@ void CrateFeature::slotAnalyzeCrate() { } void CrateFeature::slotExportPlaylist() { - CrateId crateId = m_crateTableModel.selectedCrate(); + CrateId crateId = crateIdFromIndex(m_lastRightClickedIndex); Crate crate; if (m_pTrackCollection->crates().readCrateById(crateId, &crate)) { - qDebug() << "Exporting crate" << crate; + qDebug() << "Exporting crate" << crateId << crate; } else { qDebug() << "Failed to export crate" << crateId; } @@ -768,7 +768,7 @@ void CrateFeature::slotExportPlaylist() { // Create a new table model since the main one might have an active search. QScopedPointer pCrateTableModel( new CrateTableModel(this, m_pLibrary->trackCollectionManager())); - pCrateTableModel->selectCrate(m_crateTableModel.selectedCrate()); + pCrateTableModel->selectCrate(crateId); pCrateTableModel->select(); if (fileLocation.endsWith(".csv", Qt::CaseInsensitive)) { @@ -780,8 +780,8 @@ void CrateFeature::slotExportPlaylist() { QList playlistItems; int rows = pCrateTableModel->rowCount(); for (int i = 0; i < rows; ++i) { - QModelIndex index = m_crateTableModel.index(i, 0); - playlistItems << m_crateTableModel.getTrackLocation(index); + QModelIndex index = pCrateTableModel->index(i, 0); + playlistItems << pCrateTableModel->getTrackLocation(index); } exportPlaylistItemsIntoFile( fileLocation, @@ -810,6 +810,7 @@ void CrateFeature::slotExportTrackFiles() { void CrateFeature::storePrevSiblingCrateId(CrateId crateId) { QModelIndex actIndex = indexFromCrateId(crateId); + m_prevSiblingCrate = CrateId(); for (int i = (actIndex.row() + 1); i >= (actIndex.row() - 1); i -= 2) { QModelIndex newIndex = actIndex.sibling(i, actIndex.column()); if (newIndex.isValid()) { diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index c9a1b892692..01db07dba87 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -66,14 +66,8 @@ SetlogFeature::SetlogFeature( } SetlogFeature::~SetlogFeature() { - // If the history playlist we created doesn't have any tracks in it then - // delete it so we don't end up with tons of empty playlists. This is mostly - // for developers since they regularly open Mixxx without loading a track. - if (m_playlistId != kInvalidPlaylistId && - m_playlistDao.tracksInPlaylist(m_playlistId) == 0) { - m_playlistDao.deletePlaylist(m_playlistId); - } - // Also clean history up when shutting down in case the track threshold changed + // Clean up history when shutting down in case the track threshold changed, + // incl. potentially empty current playlist deleteAllUnlockedPlaylistsWithFewerTracks(); } @@ -212,6 +206,8 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { .toDateTime(); // Create the TreeItem whose parent is the invisible root item + // Show only [kNumToplevelHistoryEntries -1] recent playlists at the top + // level before grouping them by year. if (row >= kNumToplevelHistoryEntries) { int yearCreated = dateCreated.date().year(); @@ -280,6 +276,7 @@ void SetlogFeature::decorateChild(TreeItem* item, int playlistId) { } } +/// Invoked on startup to create new current playlist and by "Finish current and start new" void SetlogFeature::slotGetNewPlaylist() { //qDebug() << "slotGetNewPlaylist() successfully triggered !"; diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 33db934f438..9b08e2c2ab2 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -173,7 +173,7 @@ void WLibrarySidebar::dropEvent(QDropEvent * event) { } void WLibrarySidebar::toggleSelectedItem() { - QModelIndexList selectedIndices = this->selectionModel()->selectedRows(); + QModelIndexList selectedIndices = selectionModel()->selectedRows(); if (selectedIndices.size() > 0) { QModelIndex index = selectedIndices.at(0); // Activate the item so its content shows in the main library. @@ -184,7 +184,7 @@ void WLibrarySidebar::toggleSelectedItem() { } bool WLibrarySidebar::isLeafNodeSelected() { - QModelIndexList selectedIndices = this->selectionModel()->selectedRows(); + QModelIndexList selectedIndices = selectionModel()->selectedRows(); if (selectedIndices.size() > 0) { QModelIndex index = selectedIndices.at(0); if(!index.model()->hasChildren(index)) { @@ -198,6 +198,8 @@ bool WLibrarySidebar::isLeafNodeSelected() { return false; } +/// Invoked by actual keypresses (requires widget focus) and emulated keypresses +/// sent by LibraryControl void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { switch (event->key()) { case Qt::Key_Return: @@ -254,25 +256,25 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { emit setLibraryFocus(FocusWidget::TracksTable); return; case kRenameSidebarItemShortcutKey: { // F2 - // Rename crate or playlist (internal, external, history + // Rename crate or playlist (internal, external, history) QModelIndexList selectedIndices = selectionModel()->selectedRows(); if (selectedIndices.isEmpty()) { return; } - // If an expanded item is selected let QTreeView collapse it QModelIndex selIndex = selectedIndices.first(); VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { qDebug() << "invalid sidebar index"; return; } if (isExpanded(selIndex)) { + // Root views / knots can not be renamed return; } emit renameItem(selIndex); return; } - case kHideRemoveShortcutKey: { // Del / Cmd+Backspace deletes items - // Rename crate or playlist (internal, external, history + case kHideRemoveShortcutKey: { // Del (macOS: Cmd+Backspace) + // Delete crate or playlist (internal, external, history) if (event->modifiers() != kHideRemoveShortcutModifier) { return; } @@ -280,13 +282,13 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { if (selectedIndices.isEmpty()) { return; } - // If an expanded item is selected let QTreeView collapse it QModelIndex selIndex = selectedIndices.first(); VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { qDebug() << "invalid sidebar index"; return; } if (isExpanded(selIndex)) { + // Root views / knots can not be deleted return; } emit deleteItem(selIndex); From 74ec98f4b636a80d1d6d2cbc5739f1466db9fb49 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 29 Jan 2023 00:03:04 +0100 Subject: [PATCH 04/44] WLibrarySidebar::keyPressEvent: ensure new selection is visible --- src/widget/wlibrarysidebar.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 9b08e2c2ab2..2c2f491f202 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -224,6 +224,10 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { qDebug() << "invalid sidebar index"; return; } + // Ensure the new selection is visible even if it was already selected/ + // focused, like when the topmost item was selected but out of sight and + // we pressed Up, Home or PageUp. + scrollTo(selIndex); emit pressed(selIndex); return; } From 443b17438335a68ede052b72916b148a1007fd2c Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 28 Jan 2023 01:17:00 +0100 Subject: [PATCH 05/44] Playlists/Crates: use last right-clicked id for playlist file import don't rely on the right-clicked crate being selected in the feature's main table model, use a fresh temporary model instead --- src/library/trackset/baseplaylistfeature.cpp | 37 ++++++++++++++------ src/library/trackset/baseplaylistfeature.h | 2 +- src/library/trackset/crate/cratefeature.cpp | 33 ++++++++++++----- src/library/trackset/crate/cratefeature.h | 2 +- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index ef3fa6785e1..a2456f130b2 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -463,6 +463,10 @@ void BasePlaylistFeature::slotImportPlaylist() { if (playlistFile.isEmpty()) { return; } + int playlistId = playlistIdFromIndex(m_lastRightClickedIndex); + if (playlistId == kInvalidPlaylistId) { + return; + } // Update the import/export playlist directory QString fileDirectory(playlistFile); @@ -470,19 +474,35 @@ void BasePlaylistFeature::slotImportPlaylist() { m_pConfig->set(kConfigKeyLastImportExportPlaylistDirectory, ConfigValue(fileDirectory)); - slotImportPlaylistFile(playlistFile); - activateChild(m_lastRightClickedIndex); + slotImportPlaylistFile(playlistFile, playlistId); } -void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlist_file) { +void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlistFile, + int playlistId) { + if (playlistFile.isEmpty()) { + return; + } // The user has picked a new directory via a file dialog. This means the // system sandboxer (if we are sandboxed) has granted us permission to this // folder. We don't need access to this file on a regular basis so we do not // register a security bookmark. - QList locations = Parser::parse(playlist_file); + // Create a new table model since the main one might have another playlist + // selected which is not the playlist that received the right-click. + QScopedPointer pPlaylistTableModel( + new PlaylistTableModel(this, + m_pLibrary->trackCollectionManager(), + "mixxx.db.model.playlist_export")); + pPlaylistTableModel->setTableModel(playlistId); + pPlaylistTableModel->setSort( + pPlaylistTableModel->fieldIndex( + ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), + Qt::AscendingOrder); + pPlaylistTableModel->select(); + + QList locations = Parser::parse(playlistFile); // Iterate over the List that holds locations of playlist entries - m_pPlaylistTableModel->addTracks(QModelIndex(), locations); + pPlaylistTableModel->addTracks(QModelIndex(), locations); } void BasePlaylistFeature::slotCreateImportPlaylist() { @@ -523,17 +543,14 @@ void BasePlaylistFeature::slotCreateImportPlaylist() { } lastPlaylistId = m_playlistDao.createPlaylist(name); - if (lastPlaylistId != kInvalidPlaylistId) { - emit saveModelState(); - m_pPlaylistTableModel->setTableModel(lastPlaylistId); - } else { + if (lastPlaylistId == kInvalidPlaylistId) { QMessageBox::warning(nullptr, tr("Playlist Creation Failed"), tr("An unknown error occurred while creating playlist: ") + name); return; } - slotImportPlaylistFile(playlistFile); + slotImportPlaylistFile(playlistFile, lastPlaylistId); } activatePlaylist(lastPlaylistId); } diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 47f9d5e2edd..25e20e3b8a9 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -69,7 +69,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature { void slotRenamePlaylist(); void slotTogglePlaylistLock(); void slotImportPlaylist(); - void slotImportPlaylistFile(const QString& playlist_file); + void slotImportPlaylistFile(const QString& playlistFile, int playlistId); void slotCreateImportPlaylist(); void slotExportPlaylist(); // Copy all of the tracks in a playlist to a new directory. diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index f9e088937d1..535f5455795 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -630,11 +630,22 @@ void CrateFeature::slotImportPlaylist() { m_pConfig->set(kConfigKeyLastImportExportCrateDirectoryKey, ConfigValue(fileDirectory)); - slotImportPlaylistFile(playlistFile); + CrateId crateId = crateIdFromIndex(m_lastRightClickedIndex); + Crate crate; + if (m_pTrackCollection->crates().readCrateById(crateId, &crate)) { + qDebug() << "Importing playlist file" << playlistFile << "into crate" + << crateId << crate; + } else { + qDebug() << "Importing playlist file" << playlistFile << "into crate" + << crateId << crate << "failed!"; + return; + } + + slotImportPlaylistFile(playlistFile, crateId); activateChild(m_lastRightClickedIndex); } -void CrateFeature::slotImportPlaylistFile(const QString& playlistFile) { +void CrateFeature::slotImportPlaylistFile(const QString& playlistFile, CrateId crateId) { // The user has picked a new directory via a file dialog. This means the // system sandboxer (if we are sandboxed) has granted us permission to this // folder. We don't need access to this file on a regular basis so we do not @@ -645,7 +656,14 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlistFile) { if (locations.empty()) { return; } - m_crateTableModel.addTracks(QModelIndex(), locations); + + // Create a new table model since the main one might have another crate + // selected which is not the crate that received the right-click. + QScopedPointer pCrateTableModel( + new CrateTableModel(this, m_pLibrary->trackCollectionManager())); + pCrateTableModel->selectCrate(crateId); + pCrateTableModel->select(); + pCrateTableModel->addTracks(QModelIndex(), locations); } void CrateFeature::slotCreateImportCrate() { @@ -663,7 +681,7 @@ void CrateFeature::slotCreateImportCrate() { CrateId lastCrateId; - // For each selected file + // For each selected file create a new crate for (const QString& playlistFile : playlistFiles) { const QFileInfo fileInfo(playlistFile); @@ -687,9 +705,7 @@ void CrateFeature::slotCreateImportCrate() { } } - if (m_pTrackCollection->insertCrate(crate, &lastCrateId)) { - m_crateTableModel.selectCrate(lastCrateId); - } else { + if (!m_pTrackCollection->insertCrate(crate, &lastCrateId)) { QMessageBox::warning(nullptr, tr("Crate Creation Failed"), tr("An unknown error occurred while creating crate: ") + @@ -697,7 +713,7 @@ void CrateFeature::slotCreateImportCrate() { return; } - slotImportPlaylistFile(playlistFile); + slotImportPlaylistFile(playlistFile, lastCrateId); } activateCrate(lastCrateId); } @@ -729,6 +745,7 @@ void CrateFeature::slotExportPlaylist() { qDebug() << "Exporting crate" << crateId << crate; } else { qDebug() << "Failed to export crate" << crateId; + return; } QString lastCrateDirectory = m_pConfig->getValue( diff --git a/src/library/trackset/crate/cratefeature.h b/src/library/trackset/crate/cratefeature.h index 35d8f0adf80..3dbbfdb72b3 100644 --- a/src/library/trackset/crate/cratefeature.h +++ b/src/library/trackset/crate/cratefeature.h @@ -62,7 +62,7 @@ class CrateFeature : public BaseTrackSetFeature { void slotAutoDjTrackSourceChanged(); void slotToggleCrateLock(); void slotImportPlaylist(); - void slotImportPlaylistFile(const QString& playlist_file); + void slotImportPlaylistFile(const QString& playlistFile, CrateId crateId); void slotCreateImportCrate(); void slotExportPlaylist(); // Copy all of the tracks in a crate to a new directory (like a thumbdrive). From b576824ef4313486ec19b85055a5445c4d013a20 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 25 Jan 2023 00:06:51 +0100 Subject: [PATCH 06/44] Library sidebar: don't select on right-click, only set focus TODO: styled only in LateNight PaleMoon, adjust stylesheets for other skins/themes --- res/skins/LateNight/style_palemoon.qss | 11 ++++-- src/library/sidebarmodel.cpp | 4 +-- src/widget/wlibrarysidebar.cpp | 48 ++++++++++++++++++++++++-- src/widget/wlibrarysidebar.h | 4 +++ 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/res/skins/LateNight/style_palemoon.qss b/res/skins/LateNight/style_palemoon.qss index 4d9d3f45a7a..a9b8c3258e0 100644 --- a/res/skins/LateNight/style_palemoon.qss +++ b/res/skins/LateNight/style_palemoon.qss @@ -2539,6 +2539,9 @@ WLibrarySidebar { /* background of Color delegate in selected row */ selection-background-color: #2c454f; } +WLibrarySidebar { + outline: none; +} /* Selected rows in Tree and Tracks table */ WLibrarySidebar::item:selected, WTrackTableView::item:selected, @@ -2546,9 +2549,13 @@ WTrackTableView::item:selected, color: #fff; background-color: #2c454f; } -WLibrarySidebar, -WLibrarySidebar::item:focus { +WLibrarySidebar::item:selected:focus { + outline: none; + border: 0px; +} +WLibrarySidebar::item:!selected:focus { outline: none; + border: 1px solid white; } WTrackTableView:focus, diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 0f170dd0b11..2becb7369f8 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -290,8 +290,8 @@ void SidebarModel::slotPressedUntilClickedTimeout() { } } -/// Connected to WLibrarySidebar::pressed signal, called after left click, -/// right click and selection change via keyboard or controller +/// Connected to WLibrarySidebar::pressed signal, called after left click and +/// selection change via keyboard or controller void SidebarModel::pressed(const QModelIndex& index) { stopPressedUntilClickedTimer(); if (index.isValid()) { diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 2c2f491f202..6d793d263e2 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -34,8 +34,13 @@ WLibrarySidebar::WLibrarySidebar(QWidget* parent) void WLibrarySidebar::contextMenuEvent(QContextMenuEvent *event) { //if (event->state() & Qt::RightButton) { //Dis shiz don werk on windowze - QModelIndex clickedItem = indexAt(event->pos()); - emit rightClicked(event->globalPos(), clickedItem); + const QModelIndex clickedIndex = indexAt(event->pos()); + if (!clickedIndex.isValid()) { + return; + } + // Use this instead of setCurrentIndex() to keep current selection + selectionModel()->setCurrentIndex(clickedIndex, QItemSelectionModel::NoUpdate); + emit rightClicked(event->globalPos(), clickedIndex); //} } @@ -201,8 +206,12 @@ bool WLibrarySidebar::isLeafNodeSelected() { /// Invoked by actual keypresses (requires widget focus) and emulated keypresses /// sent by LibraryControl void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { + // TODO(XXX) Should first keyEvent ensure previous item has focus? I.e. if the selected + // item is not focused, require second press to perform the desired action. + switch (event->key()) { case Qt::Key_Return: + focusSelectedIndex(); toggleSelectedItem(); return; case Qt::Key_Down: @@ -211,6 +220,8 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { case Qt::Key_PageUp: case Qt::Key_End: case Qt::Key_Home: { + // make the selected item the navigation starting point + focusSelectedIndex(); // Let the tree view move up and down for us. QTreeView::keyPressEvent(event); // After the selection changed force-activate (click) the newly selected @@ -303,6 +314,20 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { } } +void WLibrarySidebar::mousePressEvent(QMouseEvent* event) { + // handle right click only in contextMenuEvent() to not select the clicked index + if (event->buttons().testFlag(Qt::RightButton)) { + return; + } + QTreeView::mousePressEvent(event); +} + +void WLibrarySidebar::focusInEvent(QFocusEvent* event) { + // Clear the current index, i.e. remove the focus indicator + selectionModel()->clearCurrentIndex(); + QTreeView::focusInEvent(event); +} + void WLibrarySidebar::selectIndex(const QModelIndex& index) { //qDebug() << "WLibrarySidebar::selectIndex" << index; if (!index.isValid()) { @@ -351,6 +376,25 @@ void WLibrarySidebar::selectChildIndex(const QModelIndex& index, bool selectItem scrollTo(translated, EnsureVisible); } +/// Refocus the selected item after right-click +void WLibrarySidebar::focusSelectedIndex() { + // After the context menu was activated (and closed, with or without clicking + // an action), the currentIndex is the right-clicked item. + // If if the currentIndex is not selected, make the selection the currentIndex + QModelIndexList selectedIndices = selectionModel()->selectedRows(); + if (selectedIndices.isEmpty()) { + // QTreeView will handle this, i.e. select an index in keyPressEvent() + return; + } + QModelIndex selIndex = selectedIndices.first(); + VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { + return; + } + if (selIndex != selectionModel()->currentIndex()) { + setCurrentIndex(selIndex); + } +} + bool WLibrarySidebar::event(QEvent* pEvent) { if (pEvent->type() == QEvent::ToolTip) { updateTooltip(); diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index 0bc67fc61b4..ecf33561ac5 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -24,6 +24,8 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void dragEnterEvent(QDragEnterEvent * event) override; void dropEvent(QDropEvent * event) override; void keyPressEvent(QKeyEvent* event) override; + void mousePressEvent(QMouseEvent* event) override; + void focusInEvent(QFocusEvent* event) override; void timerEvent(QTimerEvent* event) override; void toggleSelectedItem(); bool isLeafNodeSelected(); @@ -43,6 +45,8 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { bool event(QEvent* pEvent) override; private: + void focusSelectedIndex(); + QBasicTimer m_expandTimer; QModelIndex m_hoverIndex; }; From 2f8681d54b9a3021ea2275d29ef1d331c35e1612 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 26 Jan 2023 01:29:28 +0100 Subject: [PATCH 07/44] Library sidebar: don't select newly created or duplicated crate/playlist --- src/library/trackset/baseplaylistfeature.cpp | 10 +++++----- src/library/trackset/crate/cratefeature.cpp | 12 ++++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index a2456f130b2..7bcbbb211f9 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -148,7 +148,7 @@ void BasePlaylistFeature::connectPlaylistDAO() { connect(&m_playlistDao, &PlaylistDAO::added, this, - &BasePlaylistFeature::slotPlaylistTableChangedAndSelect); + &BasePlaylistFeature::slotPlaylistTableChangedAndScrollTo); connect(&m_playlistDao, &PlaylistDAO::lockChanged, this, @@ -327,7 +327,9 @@ void BasePlaylistFeature::slotDuplicatePlaylist() { if (newPlaylistId != kInvalidPlaylistId && m_playlistDao.copyPlaylistTracks(oldPlaylistId, newPlaylistId)) { - activatePlaylist(newPlaylistId); + // Note: this assumes the sidebar model was already updated by slotPlaylisttableChanged + // and the sidebar scrolled to the new playlist + activatePlaylist(oldPlaylistId); } } @@ -377,9 +379,7 @@ void BasePlaylistFeature::slotCreatePlaylist() { int playlistId = m_playlistDao.createPlaylist(name); - if (playlistId != kInvalidPlaylistId) { - activatePlaylist(playlistId); - } else { + if (playlistId == kInvalidPlaylistId) { QMessageBox::warning(nullptr, tr("Playlist Creation Failed"), tr("An unknown error occurred while creating playlist: ") + name); diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 535f5455795..b1a17d850a3 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -403,7 +403,8 @@ void CrateFeature::slotCreateCrate() { CrateFeatureHelper(m_pTrackCollection, m_pConfig) .createEmptyCrate(); if (crateId.isValid()) { - activateCrate(crateId); + // expand Crates and scroll to new crate + m_pSidebarWidget->selectChildIndex(indexFromCrateId(crateId), false); } } @@ -492,11 +493,14 @@ void CrateFeature::slotRenameCrate() { void CrateFeature::slotDuplicateCrate() { Crate crate; if (readLastRightClickedCrate(&crate)) { - CrateId crateId = + CrateId newCrateId = CrateFeatureHelper(m_pTrackCollection, m_pConfig) .duplicateCrate(crate); - if (crateId.isValid()) { - activateCrate(crateId); + if (newCrateId.isValid()) { + qDebug() << "Duplicate crate" << crate << ", new crate:" << newCrateId; + // expand Crates and scroll to new crate + m_pSidebarWidget->selectChildIndex(indexFromCrateId(newCrateId), false); + activateCrate(crate.getId()); } } else { qDebug() << "Failed to duplicate selected crate"; From def2ddf1d04aed641149a33aeefd0082a6041449 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 27 Jan 2023 23:37:42 +0100 Subject: [PATCH 08/44] Library sidebar: don't rebuild tree model after item rename or lock change, just update labels --- src/library/trackset/baseplaylistfeature.cpp | 18 +++++++++++--- src/library/trackset/baseplaylistfeature.h | 1 + src/library/trackset/crate/cratefeature.cpp | 7 +++++- src/library/trackset/crate/cratefeature.h | 1 + src/library/trackset/playlistfeature.cpp | 22 ++++++++--------- src/library/trackset/playlistfeature.h | 1 + src/library/trackset/setlogfeature.cpp | 26 ++++++++------------ src/library/trackset/setlogfeature.h | 1 + 8 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 7bcbbb211f9..12b0adf8a22 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -152,7 +152,7 @@ void BasePlaylistFeature::connectPlaylistDAO() { connect(&m_playlistDao, &PlaylistDAO::lockChanged, this, - &BasePlaylistFeature::slotPlaylistTableChangedAndScrollTo); + &BasePlaylistFeature::slotPlaylistTableLockChanged); connect(&m_playlistDao, &PlaylistDAO::deleted, this, @@ -737,16 +737,26 @@ void BasePlaylistFeature::htmlLinkClicked(const QUrl& link) { } void BasePlaylistFeature::updateChildModel(int playlistId) { + // qDebug() << "BasePlaylistFeature::updateChildModel:" << playlistId; + if (playlistId == kInvalidPlaylistId) { + return; + } QString playlistLabel = fetchPlaylistLabel(playlistId); - QVariant variantId = QVariant(playlistId); for (int row = 0; row < m_pSidebarModel->rowCount(); ++row) { QModelIndex index = m_pSidebarModel->index(row, 0); TreeItem* pTreeItem = m_pSidebarModel->getItem(index); DEBUG_ASSERT(pTreeItem != nullptr); - if (!pTreeItem->hasChildren() && // leaf node - pTreeItem->getData() == variantId) { + if (pTreeItem->hasChildren()) { + for (TreeItem* pChild : qAsConst(pTreeItem->children())) { + if (pChild->getData() == variantId) { + pTreeItem = pChild; + break; + } + } + } + if (pTreeItem->getData() == variantId) { pTreeItem->setLabel(playlistLabel); decorateChild(pTreeItem, playlistId); } diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 25e20e3b8a9..25e70589940 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -54,6 +54,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature { slotPlaylistTableChanged(playlistId); selectPlaylistInSidebar(playlistId, false); }; + virtual void slotPlaylistTableLockChanged(int playlistId) = 0; virtual void slotPlaylistTableRenamed(int playlistId, const QString& newName) = 0; virtual void slotPlaylistContentChanged(QSet playlistIds) = 0; void slotCreatePlaylist(); diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index b1a17d850a3..ad706f599c0 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -171,7 +171,7 @@ void CrateFeature::connectTrackCollection() { connect(m_pTrackCollection, // renamed, un/locked, toggled AutoDJ source &TrackCollection::crateUpdated, this, - &CrateFeature::slotCrateTableChanged); + &CrateFeature::slotUpdateCrateLabel); connect(m_pTrackCollection, &TrackCollection::crateDeleted, this, @@ -868,6 +868,11 @@ void CrateFeature::slotCrateContentChanged(CrateId crateId) { updateChildModel(updatedCrateIds); } +void CrateFeature::slotUpdateCrateLabel(CrateId updatedCrateId) { + QSet updatedCrateIds = {updatedCrateId}; + updateChildModel(updatedCrateIds); +} + void CrateFeature::slotUpdateCrateLabels(const QSet& updatedCrateIds) { updateChildModel(updatedCrateIds); } diff --git a/src/library/trackset/crate/cratefeature.h b/src/library/trackset/crate/cratefeature.h index 3dbbfdb72b3..b76e5ceafc0 100644 --- a/src/library/trackset/crate/cratefeature.h +++ b/src/library/trackset/crate/cratefeature.h @@ -73,6 +73,7 @@ class CrateFeature : public BaseTrackSetFeature { void htmlLinkClicked(const QUrl& link); void slotTrackSelected(TrackId trackId); void slotResetSelectedTrack(); + void slotUpdateCrateLabel(CrateId updatedCrateId); void slotUpdateCrateLabels(const QSet& updatedCrateIds); private: diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index 076bbc72e5e..c6ac580a765 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -285,27 +285,25 @@ void PlaylistFeature::slotPlaylistTableChanged(int playlistId) { void PlaylistFeature::slotPlaylistContentChanged(QSet playlistIds) { for (const auto playlistId : qAsConst(playlistIds)) { - enum PlaylistDAO::HiddenType type = - m_playlistDao.getHiddenType(playlistId); - if (type == PlaylistDAO::PLHT_NOT_HIDDEN || - type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist + if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { updateChildModel(playlistId); } } } +void PlaylistFeature::slotPlaylistTableLockChanged(int playlistId) { + // qDebug() << "slotPlaylistTableLockChanged() playlistId:" << playlistId; + if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { + updateChildModel(playlistId); + } +} + void PlaylistFeature::slotPlaylistTableRenamed( int playlistId, const QString& newName) { Q_UNUSED(newName); //qDebug() << "slotPlaylistTableChanged() playlistId:" << playlistId; - enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); - if (type == PlaylistDAO::PLHT_NOT_HIDDEN || - type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist - clearChildModel(); - m_lastRightClickedIndex = constructChildModel(playlistId); - if (type != PlaylistDAO::PLHT_UNKNOWN) { - activatePlaylist(playlistId); - } + if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { + updateChildModel(playlistId); } } diff --git a/src/library/trackset/playlistfeature.h b/src/library/trackset/playlistfeature.h index 66e1f39638b..162f6a6cfbe 100644 --- a/src/library/trackset/playlistfeature.h +++ b/src/library/trackset/playlistfeature.h @@ -38,6 +38,7 @@ class PlaylistFeature : public BasePlaylistFeature { private slots: void slotPlaylistTableChanged(int playlistId) override; void slotPlaylistContentChanged(QSet playlistIds) override; + void slotPlaylistTableLockChanged(int playlistId) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; protected: diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 01db07dba87..8577454ccc5 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -459,30 +459,24 @@ void SetlogFeature::reloadChildModel(int playlistId) { void SetlogFeature::slotPlaylistContentChanged(QSet playlistIds) { for (const auto playlistId : qAsConst(playlistIds)) { - enum PlaylistDAO::HiddenType type = - m_playlistDao.getHiddenType(playlistId); - if (type == PlaylistDAO::PLHT_SET_LOG || - type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist + if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { updateChildModel(playlistId); } } } +void SetlogFeature::slotPlaylistTableLockChanged(int playlistId) { + // qDebug() << "slotPlaylistTableLockChanged() playlistId:" << playlistId; + if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { + updateChildModel(playlistId); + } +} + void SetlogFeature::slotPlaylistTableRenamed(int playlistId, const QString& newName) { Q_UNUSED(newName); //qDebug() << "slotPlaylistTableRenamed() playlistId:" << playlistId; - enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); - if (type == PlaylistDAO::PLHT_SET_LOG || - type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist - clearChildModel(); - m_lastRightClickedIndex = constructChildModel(playlistId); - if (type != PlaylistDAO::PLHT_UNKNOWN) { - if (playlistId == m_pPlaylistTableModel->getPlaylist()) { - activatePlaylist(playlistId); - } else if (m_pSidebarWidget) { - m_pSidebarWidget->selectChildIndex(indexFromPlaylistId(playlistId), false); - } - } + if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { + updateChildModel(playlistId); } } diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index d0cf14ca246..c96173c6c68 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -40,6 +40,7 @@ class SetlogFeature : public BasePlaylistFeature { void slotPlayingTrackChanged(TrackPointer currentPlayingTrack); void slotPlaylistTableChanged(int playlistId) override; void slotPlaylistContentChanged(QSet playlistIds) override; + void slotPlaylistTableLockChanged(int playlistId) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; private: From aabd985da5cadfac44340e4f4c5da9877194d903 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 26 Jan 2023 02:22:36 +0100 Subject: [PATCH 09/44] Library sidebar: track left & right clicks separately --- src/library/trackset/baseplaylistfeature.cpp | 9 ++++++--- src/library/trackset/baseplaylistfeature.h | 1 + src/library/trackset/crate/cratefeature.cpp | 14 +++++++++++--- src/library/trackset/crate/cratefeature.h | 2 ++ src/library/trackset/setlogfeature.cpp | 17 ++++++++++++----- 5 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 12b0adf8a22..5ceadb3f610 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -203,6 +203,8 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { // like the year folder in the history feature return; } + m_lastClickedIndex = index; + m_lastRightClickedIndex = QModelIndex(); emit saveModelState(); m_pPlaylistTableModel->setTableModel(playlistId); emit showTrackModel(m_pPlaylistTableModel); @@ -218,17 +220,18 @@ void BasePlaylistFeature::activatePlaylist(int playlistId) { VERIFY_OR_DEBUG_ASSERT(index.isValid()) { return; } + m_lastClickedIndex = index; + m_lastRightClickedIndex = QModelIndex(); emit saveModelState(); - m_lastRightClickedIndex = index; m_pPlaylistTableModel->setTableModel(playlistId); emit showTrackModel(m_pPlaylistTableModel); emit enableCoverArtDisplay(true); // Update selection - emit featureSelect(this, m_lastRightClickedIndex); + emit featureSelect(this, m_lastClickedIndex); if (!m_pSidebarWidget) { return; } - m_pSidebarWidget->selectChildIndex(m_lastRightClickedIndex); + m_pSidebarWidget->selectChildIndex(m_lastClickedIndex); } void BasePlaylistFeature::renameItem(const QModelIndex& index) { diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 25e70589940..6a8d78b2a82 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -97,6 +97,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature { QModelIndex indexFromPlaylistId(int playlistId); PlaylistDAO& m_playlistDao; + QModelIndex m_lastClickedIndex; QModelIndex m_lastRightClickedIndex; QPointer m_pSidebarWidget; diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index ad706f599c0..03ed473ecc9 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -295,12 +295,19 @@ TreeItemModel* CrateFeature::sidebarModel() const { return m_pSidebarModel; } +void CrateFeature::activate() { + m_lastClickedIndex = QModelIndex(); + BaseTrackSetFeature::activate(); +} + void CrateFeature::activateChild(const QModelIndex& index) { //qDebug() << "CrateFeature::activateChild()" << index; CrateId crateId(crateIdFromIndex(index)); VERIFY_OR_DEBUG_ASSERT(crateId.isValid()) { return; } + m_lastClickedIndex = index; + m_lastRightClickedIndex = QModelIndex(); emit saveModelState(); m_crateTableModel.selectCrate(crateId); emit showTrackModel(&m_crateTableModel); @@ -317,13 +324,14 @@ bool CrateFeature::activateCrate(CrateId crateId) { return false; } emit saveModelState(); - m_lastRightClickedIndex = index; + m_lastClickedIndex = index; + m_lastRightClickedIndex = QModelIndex(); m_crateTableModel.selectCrate(crateId); emit showTrackModel(&m_crateTableModel); emit enableCoverArtDisplay(true); // Update selection - emit featureSelect(this, m_lastRightClickedIndex); - activateChild(m_lastRightClickedIndex); + emit featureSelect(this, m_lastClickedIndex); + activateChild(m_lastClickedIndex); return true; } diff --git a/src/library/trackset/crate/cratefeature.h b/src/library/trackset/crate/cratefeature.h index b76e5ceafc0..33ea281c919 100644 --- a/src/library/trackset/crate/cratefeature.h +++ b/src/library/trackset/crate/cratefeature.h @@ -42,6 +42,7 @@ class CrateFeature : public BaseTrackSetFeature { TreeItemModel* sidebarModel() const override; public slots: + void activate() override; void activateChild(const QModelIndex& index) override; void onRightClick(const QPoint& globalPos) override; void onRightClickChild(const QPoint& globalPos, const QModelIndex& index) override; @@ -110,6 +111,7 @@ class CrateFeature : public BaseTrackSetFeature { // Can be used to restore a similar selection after the sidebar model was rebuilt. CrateId m_prevSiblingCrate; + QModelIndex m_lastClickedIndex; QModelIndex m_lastRightClickedIndex; TrackId m_selectedTrackId; diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 8577454ccc5..350afda900f 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -481,19 +481,26 @@ void SetlogFeature::slotPlaylistTableRenamed(int playlistId, const QString& newN } void SetlogFeature::activate() { + // The root item was clicked, so actuvate the current playlist. + m_lastClickedIndex = QModelIndex(); activatePlaylist(m_playlistId); } void SetlogFeature::activatePlaylist(int playlistId) { //qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId; + if (playlistId == kInvalidPlaylistId) { + return; + } QModelIndex index = indexFromPlaylistId(playlistId); - if (playlistId != kInvalidPlaylistId && index.isValid()) { + if (index.isValid()) { + emit saveModelState(); m_pPlaylistTableModel->setTableModel(playlistId); emit showTrackModel(m_pPlaylistTableModel); emit enableCoverArtDisplay(true); - // Update selection only, if it is not the current playlist - // since we want the root item to be the current playlist as well - if (playlistId != m_playlistId) { + // Update sidebar selection only if this is a child, incl. current playlist. + // indexFromPlaylistId() can't be used because, in case the root item was + // selected, that would switch to the 'current' child. + if (m_lastClickedIndex.isValid()) { emit featureSelect(this, index); activateChild(index); } @@ -501,6 +508,6 @@ void SetlogFeature::activatePlaylist(int playlistId) { } QString SetlogFeature::getRootViewHtml() const { - // Instead of the help text, the history shows the current entry instead + // Instead of the help text, the history shows the current playlist return QString(); } From 82da0eb87f1e0bf4952ed03a11f087394c06665b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 28 Jan 2023 23:24:46 +0100 Subject: [PATCH 10/44] BasePlaylistFeature::activatePlaylist: remove redundant selectIndex() that is already emitted by SidebarModel::slotFeatureSelect() --- src/library/trackset/baseplaylistfeature.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 5ceadb3f610..29fc91e9c81 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -228,10 +228,6 @@ void BasePlaylistFeature::activatePlaylist(int playlistId) { emit enableCoverArtDisplay(true); // Update selection emit featureSelect(this, m_lastClickedIndex); - if (!m_pSidebarWidget) { - return; - } - m_pSidebarWidget->selectChildIndex(m_lastClickedIndex); } void BasePlaylistFeature::renameItem(const QModelIndex& index) { From 5a696b8c62924d8bb5b757ebd73443457bc80797 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 28 Jan 2023 23:26:33 +0100 Subject: [PATCH 11/44] CrateFeature::activateCrate: remove redundant activateChild() --- src/library/trackset/crate/cratefeature.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 03ed473ecc9..4972d07d85d 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -331,7 +331,6 @@ bool CrateFeature::activateCrate(CrateId crateId) { emit enableCoverArtDisplay(true); // Update selection emit featureSelect(this, m_lastClickedIndex); - activateChild(m_lastClickedIndex); return true; } From fa9278bec8c021da6275e508f2261456c8d547de Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 28 Jan 2023 02:37:52 +0100 Subject: [PATCH 12/44] History feature: fix decoration of current playlist after 'Finish current, start new' --- src/library/trackset/setlogfeature.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 350afda900f..f80e019a5d5 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -305,9 +305,10 @@ void SetlogFeature::slotGetNewPlaylist() { m_recentTracks.clear(); } - reloadChildModel(m_playlistId); // For moving selection - emit showTrackModel(m_pPlaylistTableModel); - activatePlaylist(m_playlistId); + // reload child model again because the 'added' signal fired by PlaylistDAO + // might have triggered slotPlaylistTableChanged() before m_playlistId was set, + // which causes the wrong playlist being decorated as 'current' + slotPlaylistTableChanged(m_playlistId); } void SetlogFeature::slotJoinWithPrevious() { From 8f8bf4896290484ce0eae8794b891bc23868540b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 26 Jan 2023 11:59:37 +0100 Subject: [PATCH 13/44] Library sidebar: add item selection helpers for playlists & crates --- src/library/sidebarmodel.cpp | 12 +++++++ src/library/sidebarmodel.h | 1 + src/library/trackset/baseplaylistfeature.cpp | 4 +++ src/library/trackset/baseplaylistfeature.h | 1 + src/library/trackset/crate/cratefeature.cpp | 4 +++ src/library/trackset/crate/cratefeature.h | 1 + src/widget/wlibrarysidebar.cpp | 34 ++++++++++++++++++++ src/widget/wlibrarysidebar.h | 4 +++ 8 files changed, 61 insertions(+) diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 2becb7369f8..e0539fd95d9 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -138,6 +138,18 @@ QModelIndex SidebarModel::index(int row, int column, return createIndex(row, column, const_cast(this)); } +QModelIndex SidebarModel::getFeatureRootIndex(LibraryFeature* pFeature) { + // qDebug() << "SidebarModel::getFeatureRootIndex for" << pFeature->title().toString(); + QModelIndex ind; + for (int i = 0; i < m_sFeatures.size(); ++i) { + if (m_sFeatures[i] == pFeature) { + ind = index(i, 0); + break; + } + } + return ind; +} + QModelIndex SidebarModel::parent(const QModelIndex& index) const { //qDebug() << "SidebarModel::parent index=" << index.getData(); if (index.isValid()) { diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index 921d93dbd78..debaa338a6b 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -45,6 +45,7 @@ class SidebarModel : public QAbstractItemModel { QModelIndex translateChildIndex(const QModelIndex& index) { return translateIndex(index, index.model()); } + QModelIndex getFeatureRootIndex(LibraryFeature* pFeature); public slots: void pressed(const QModelIndex& index); diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 29fc91e9c81..9e6e89a3b40 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -782,6 +782,10 @@ QModelIndex BasePlaylistFeature::indexFromPlaylistId(int playlistId) { return QModelIndex(); } +bool BasePlaylistFeature::isChildIndexSelectedInSidebar(const QModelIndex& index) { + return m_pSidebarWidget && m_pSidebarWidget->isChildIndexSelected(index); +}; + void BasePlaylistFeature::slotTrackSelected(TrackId trackId) { m_selectedTrackId = trackId; m_playlistDao.getPlaylistsTrackIsIn(m_selectedTrackId, &m_playlistIdsOfSelectedTrack); diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 6a8d78b2a82..cadbb915f2a 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -95,6 +95,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature { // Get the QModelIndex of a playlist based on its id. Returns QModelIndex() // on failure. QModelIndex indexFromPlaylistId(int playlistId); + bool isChildIndexSelectedInSidebar(const QModelIndex& index); PlaylistDAO& m_playlistDao; QModelIndex m_lastClickedIndex; diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 4972d07d85d..718f51b7043 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -348,6 +348,10 @@ bool CrateFeature::readLastRightClickedCrate(Crate* pCrate) const { return true; } +bool CrateFeature::isChildIndexSelectedInSidebar(const QModelIndex& index) { + return m_pSidebarWidget && m_pSidebarWidget->isChildIndexSelected(index); +} + void CrateFeature::onRightClick(const QPoint& globalPos) { m_lastRightClickedIndex = QModelIndex(); QMenu menu(m_pSidebarWidget); diff --git a/src/library/trackset/crate/cratefeature.h b/src/library/trackset/crate/cratefeature.h index 33ea281c919..5f392cd6091 100644 --- a/src/library/trackset/crate/cratefeature.h +++ b/src/library/trackset/crate/cratefeature.h @@ -96,6 +96,7 @@ class CrateFeature : public BaseTrackSetFeature { CrateId crateIdFromIndex(const QModelIndex& index) const; QModelIndex indexFromCrateId(CrateId crateId) const; + bool isChildIndexSelectedInSidebar(const QModelIndex& index); bool readLastRightClickedCrate(Crate* pCrate) const; QString formatRootViewHtml() const; diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 6d793d263e2..c74b065eb9c 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -203,6 +203,40 @@ bool WLibrarySidebar::isLeafNodeSelected() { return false; } +bool WLibrarySidebar::isChildIndexSelected(const QModelIndex& index) { + // qDebug() << "WLibrarySidebar::isChildIndexSelected" << index; + QModelIndexList selectedIndices = selectionModel()->selectedRows(); + if (selectedIndices.size() <= 0) { + // qWarning() << " >> no selection"; + return false; + } + SidebarModel* sidebarModel = qobject_cast(model()); + VERIFY_OR_DEBUG_ASSERT(sidebarModel) { + // qDebug() << " >> model() is not SidebarModel"; + return false; + } + QModelIndex translated = sidebarModel->translateChildIndex(index); + if (!translated.isValid()) { + // qDebug() << " >> index can't be translated"; + return false; + } + return translated == selectedIndices.first(); +} + +bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { + // qDebug() << "WLibrarySidebar::isFeatureRootIndexSelected"; + QModelIndexList selectedIndices = selectionModel()->selectedRows(); + if (selectedIndices.size() <= 0) { + return false; + } + SidebarModel* sidebarModel = qobject_cast(model()); + VERIFY_OR_DEBUG_ASSERT(sidebarModel) { + return false; + } + const QModelIndex rootIndex = sidebarModel->getFeatureRootIndex(pFeature); + return rootIndex == selectedIndices.first(); +} + /// Invoked by actual keypresses (requires widget focus) and emulated keypresses /// sent by LibraryControl void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index ecf33561ac5..555833772c7 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -14,6 +14,8 @@ #include "library/library_decl.h" #include "widget/wbasewidget.h" +class LibraryFeature; + class WLibrarySidebar : public QTreeView, public WBaseWidget { Q_OBJECT public: @@ -29,6 +31,8 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void timerEvent(QTimerEvent* event) override; void toggleSelectedItem(); bool isLeafNodeSelected(); + bool isChildIndexSelected(const QModelIndex& index); + bool isFeatureRootIndexSelected(LibraryFeature* pFeature); public slots: void selectIndex(const QModelIndex&); From 60198d5e376a87c0bcfcabffd2442c7e0f85b3e6 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 26 Jan 2023 15:47:45 +0100 Subject: [PATCH 14/44] History feature: clear view when clicking YEAR items Before, the tracks view would still show the last selected feature/item view. Now, all YEAR nodes are linked to an empty, locked placeholder playlist to clear the view. This also helps with restoring the previous sidebar selection after the childmodel was rebuilt. --- src/library/trackset/baseplaylistfeature.cpp | 5 +++-- src/library/trackset/setlogfeature.cpp | 18 +++++++++++++++--- src/library/trackset/setlogfeature.h | 2 ++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 9e6e89a3b40..9acb7411831 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -199,8 +199,9 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { //qDebug() << "BasePlaylistFeature::activateChild()" << index; int playlistId = playlistIdFromIndex(index); if (playlistId == kInvalidPlaylistId) { - // This happens if user clicks on group nodes - // like the year folder in the history feature + // This happens if user clicks on group nodes. + // Doesn't apply to YEAR nodes in the history feature, they are linked to + // a dummy playlist. return; } m_lastClickedIndex = index; diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index f80e019a5d5..5c9ad069928 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -40,11 +40,22 @@ SetlogFeature::SetlogFeature( QStringLiteral("SETLOGHOME"), QStringLiteral("history")), m_playlistId(kInvalidPlaylistId), + m_placeholderId(kInvalidPlaylistId), m_pLibrary(pLibrary), m_pConfig(pConfig) { // remove unneeded entries deleteAllUnlockedPlaylistsWithFewerTracks(); + // Create empty placeholder playlist for YEAR items + QString placeholderName = "historyPlaceholder"; + m_placeholderId = m_playlistDao.createUniquePlaylist(&placeholderName, + PlaylistDAO::PLHT_UNKNOWN); + VERIFY_OR_DEBUG_ASSERT(m_placeholderId != kInvalidPlaylistId) { + qWarning() << "Failed to create empty History placeholder playlist!"; + } + // just to be safe + m_playlistDao.setPlaylistLocked(m_placeholderId, true); + //construct child model m_pSidebarModel->setRootItem(TreeItem::newRoot(this)); constructChildModel(kInvalidPlaylistId); @@ -67,7 +78,7 @@ SetlogFeature::SetlogFeature( SetlogFeature::~SetlogFeature() { // Clean up history when shutting down in case the track threshold changed, - // incl. potentially empty current playlist + // incl. the empty placeholder playlist and potentially empty current playlist deleteAllUnlockedPlaylistsWithFewerTracks(); } @@ -127,7 +138,8 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex int playlistId = playlistIdFromIndex(index); // not a real entry - if (playlistId == kInvalidPlaylistId) { + if (playlistId == kInvalidPlaylistId || playlistId == m_placeholderId) { + // TODO(ronso0) allow deleting all playlist in YEAR group return; } @@ -217,7 +229,7 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { pGroupItem = i.value(); } else { auto pNewGroupItem = std::make_unique( - QString::number(yearCreated), kInvalidPlaylistId); + QString::number(yearCreated), m_placeholderId); pGroupItem = pNewGroupItem.get(); groups.insert(yearCreated, pGroupItem); itemList.push_back(std::move(pNewGroupItem)); diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index c96173c6c68..422b3ec74bc 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -52,6 +52,8 @@ class SetlogFeature : public BasePlaylistFeature { QAction* m_pJoinWithPreviousAction; QAction* m_pStartNewPlaylist; int m_playlistId; + int m_placeholderId; + QPointer m_libraryWidget; Library* m_pLibrary; UserSettingsPointer m_pConfig; From 117f61affde8a3f421909d0a9348db22a2f16d8a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 26 Jan 2023 17:37:21 +0100 Subject: [PATCH 15/44] History feature: allow deleting all playlists in YEAR node --- src/library/dao/playlistdao.cpp | 49 ++++++---- src/library/dao/playlistdao.h | 7 +- src/library/trackset/setlogfeature.cpp | 121 ++++++++++++++++++++----- src/library/trackset/setlogfeature.h | 3 + src/widget/wlibrarysidebar.cpp | 4 - 5 files changed, 133 insertions(+), 51 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 35a6c9431bc..e0247d5b0fe 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -227,6 +227,33 @@ void PlaylistDAO::deletePlaylist(const int playlistId) { } } +int PlaylistDAO::deletePlaylists(const QStringList& idStringList, PlaylistDAO::HiddenType type) { + if (idStringList.isEmpty()) { + return 0; + } + QString idString = idStringList.join(","); + + qInfo() << "Deleting" << idStringList.size() << "playlists of type" << type; + + // delete tracks assigned to these playlists + auto deleteTracks = FwdSqlQuery(m_database, + QString("DELETE FROM PlaylistTracks WHERE playlist_id IN (%1)") + .arg(idString)); + if (!deleteTracks.execPrepared()) { + return -1; + } + + // delete the playlists + auto deletePlaylists = FwdSqlQuery(m_database, + QString("DELETE FROM Playlists WHERE id IN (%1)").arg(idString)); + if (!deletePlaylists.execPrepared()) { + return -1; + } + + emit deleted(-1); + return idStringList.length(); +} + int PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( PlaylistDAO::HiddenType type, int minNumberOfTracks) { VERIFY_OR_DEBUG_ASSERT(minNumberOfTracks > 0) { @@ -250,28 +277,10 @@ int PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( while (query.next()) { idStringList.append(query.value(0).toString()); } - if (idStringList.isEmpty()) { - return 0; - } - QString idString = idStringList.join(","); - - qInfo() << "Deleting" << idStringList.size() << "playlists of type" << type + qInfo() << "Prepared deletion of" << idStringList.size() << "playlists of type" << type << "that contain fewer than" << minNumberOfTracks << "tracks"; - auto deleteTracks = FwdSqlQuery(m_database, - QString("DELETE FROM PlaylistTracks WHERE playlist_id IN (%1)") - .arg(idString)); - if (!deleteTracks.execPrepared()) { - return -1; - } - - auto deletePlaylists = FwdSqlQuery(m_database, - QString("DELETE FROM Playlists WHERE id IN (%1)").arg(idString)); - if (!deletePlaylists.execPrepared()) { - return -1; - } - - return idStringList.length(); + return deletePlaylists(idStringList, type); } void PlaylistDAO::renamePlaylist(const int playlistId, const QString& newName) { diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index 754dcceab3f..f4e6a84af59 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -57,10 +57,13 @@ class PlaylistDAO : public QObject, public virtual DAO { int createUniquePlaylist(QString* pName, const HiddenType type = PLHT_NOT_HIDDEN); // Delete a playlist void deletePlaylist(const int playlistId); - /// Delete Playlists with fewer entries then "length" + // Delete a set of playlists. for now "type" is just for the logger + int deletePlaylists(const QStringList& idStringList, + const HiddenType type = PLHT_NOT_HIDDEN); + /// Delete Playlists with fewer entries then "minNumberOfTracks" /// Needs to be called inside a transaction. /// @return number of deleted playlists, -1 on error - int deleteAllUnlockedPlaylistsWithFewerTracks(PlaylistDAO::HiddenType type, + int deleteAllUnlockedPlaylistsWithFewerTracks(const PlaylistDAO::HiddenType type, int minNumberOfTracks); // Rename a playlist void renamePlaylist(const int playlistId, const QString& newName); diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 5c9ad069928..20f0908daea 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -72,6 +72,12 @@ SetlogFeature::SetlogFeature( this, &SetlogFeature::slotGetNewPlaylist); + m_pDeleteAllChildPlaylists = new QAction(tr("Delete all child playlists"), this); + connect(m_pDeleteAllChildPlaylists, + &QAction::triggered, + this, + &SetlogFeature::slotDeleteAllChildPlaylists); + // initialized in a new generic slot(get new history playlist purpose) slotGetNewPlaylist(); } @@ -116,9 +122,13 @@ void SetlogFeature::slotDeletePlaylist() { if (playlistId == m_playlistId) { // the current setlog must not be deleted return; + } else if (playlistId == m_placeholderId) { + // this is a YEAR node + slotDeleteAllChildPlaylists(); + } else { + // regular setlog, call the base implementation + BasePlaylistFeature::slotDeletePlaylist(); } - // regular setlog, call the base implementation - BasePlaylistFeature::slotDeletePlaylist(); } void SetlogFeature::onRightClick(const QPoint& globalPos) { @@ -138,8 +148,7 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex int playlistId = playlistIdFromIndex(index); // not a real entry - if (playlistId == kInvalidPlaylistId || playlistId == m_placeholderId) { - // TODO(ronso0) allow deleting all playlist in YEAR group + if (playlistId == kInvalidPlaylistId) { return; } @@ -151,28 +160,42 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex m_pLockPlaylistAction->setText(locked ? tr("Unlock") : tr("Lock")); QMenu menu(m_pSidebarWidget); - //menu.addAction(m_pCreatePlaylistAction); - //menu.addSeparator(); - menu.addAction(m_pAddToAutoDJAction); - menu.addAction(m_pAddToAutoDJTopAction); - menu.addSeparator(); - menu.addAction(m_pRenamePlaylistAction); - if (playlistId != m_playlistId) { - // Todays playlist should not be locked or deleted - menu.addAction(m_pDeletePlaylistAction); - menu.addAction(m_pLockPlaylistAction); - } - if (index.sibling(index.row() + 1, index.column()).isValid()) { - // The very first setlog cannot be joint - menu.addAction(m_pJoinWithPreviousAction); - } - if (playlistId == m_playlistId) { - // Todays playlists can change ! - m_pStartNewPlaylist->setEnabled(m_playlistDao.tracksInPlaylist(m_playlistId) > 0); - menu.addAction(m_pStartNewPlaylist); + if (playlistId == m_placeholderId) { + // this is a YEAR item + menu.addAction(m_pDeleteAllChildPlaylists); + // TODO(ronso0) Allow un/locking all child playlists? + // This is handy if you want to either bulk-secure setlogs, or prepare + // further cleanup / rename + } else { + // this is a playlist + bool locked = m_playlistDao.isPlaylistLocked(playlistId); + m_pDeletePlaylistAction->setEnabled(!locked); + m_pRenamePlaylistAction->setEnabled(!locked); + m_pJoinWithPreviousAction->setEnabled(!locked); + + m_pLockPlaylistAction->setText(locked ? tr("Unlock") : tr("Lock")); + menu.addAction(m_pAddToAutoDJAction); + menu.addAction(m_pAddToAutoDJTopAction); + menu.addSeparator(); + menu.addAction(m_pRenamePlaylistAction); + if (playlistId != m_playlistId) { + // Todays playlist should not be locked or deleted + menu.addAction(m_pDeletePlaylistAction); + menu.addAction(m_pLockPlaylistAction); + } + if (index.sibling(index.row() + 1, index.column()).isValid()) { + // The very first (oldest) setlog cannot be joint + menu.addAction(m_pJoinWithPreviousAction); + } + if (playlistId == m_playlistId) { + // Todays playlists can change ! + m_pStartNewPlaylist->setEnabled( + m_playlistDao.tracksInPlaylist(m_playlistId) > 0); + menu.addAction(m_pStartNewPlaylist); + } + menu.addSeparator(); + menu.addAction(m_pExportPlaylistAction); } - menu.addSeparator(); - menu.addAction(m_pExportPlaylistAction); menu.exec(globalPos); } @@ -381,6 +404,54 @@ void SetlogFeature::slotJoinWithPrevious() { } } +void SetlogFeature::slotDeleteAllChildPlaylists() { + if (!m_lastRightClickedIndex.isValid()) { + return; + } + qWarning() << "slotDeleteAllChildPlaylists of" << m_lastRightClickedIndex.data().toString(); + // Ask for confirmation: "Delete all playlists in this group, including locked?" + TreeItem* item = static_cast(m_lastRightClickedIndex.internalPointer()); + if (!item) { + return; + } + const QList yearChildren = item->children(); + if (yearChildren.isEmpty()) { + return; + } + + QMessageBox::StandardButton btn = QMessageBox::question(nullptr, + tr("Confirm Deletion"), + tr("Do you really want to delete all playlist in %1?

" + "Note: this also includes locked playlists!") + .arg(m_lastRightClickedIndex.data().toString()), + QMessageBox::Yes | QMessageBox::No, + QMessageBox::No); + if (btn == QMessageBox::No) { + return; + } + // Double-check, this is a weighty decision + btn = QMessageBox::warning(nullptr, + tr("Confirm Deletion"), + tr("Deleting all playlist in %1 including locked playlists.

" + "Are you sure?") + .arg(m_lastRightClickedIndex.data().toString()), + QMessageBox::Yes | QMessageBox::No, + QMessageBox::No); + if (btn == QMessageBox::No) { + return; + } + + QStringList ids; + for (auto* child : qAsConst(yearChildren)) { + bool ok = false; + int childId = child->getData().toInt(&ok); + if (ok && childId != kInvalidPlaylistId) { + ids.append(child->getData().toString()); + } + } + m_playlistDao.deletePlaylists(ids, PlaylistDAO::PLHT_SET_LOG); +} + void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { if (!currentPlayingTrack) { return; diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 422b3ec74bc..ec2839ef2da 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -42,6 +42,7 @@ class SetlogFeature : public BasePlaylistFeature { void slotPlaylistContentChanged(QSet playlistIds) override; void slotPlaylistTableLockChanged(int playlistId) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; + void slotDeleteAllChildPlaylists(); private: void deleteAllUnlockedPlaylistsWithFewerTracks(); @@ -51,6 +52,8 @@ class SetlogFeature : public BasePlaylistFeature { std::list m_recentTracks; QAction* m_pJoinWithPreviousAction; QAction* m_pStartNewPlaylist; + QAction* m_pDeleteAllChildPlaylists; + int m_playlistId; int m_placeholderId; diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index c74b065eb9c..060a61c97e0 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -336,10 +336,6 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { qDebug() << "invalid sidebar index"; return; } - if (isExpanded(selIndex)) { - // Root views / knots can not be deleted - return; - } emit deleteItem(selIndex); return; } From 42ce1d4f227a4c3d7923456bfcd2486f3dd2e161 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 9 Feb 2023 22:22:19 +0100 Subject: [PATCH 16/44] PlaylistDAO: add setPlaylistsLocked --- src/library/dao/playlistdao.cpp | 28 ++++++++++++++++++++++++++++ src/library/dao/playlistdao.h | 1 + 2 files changed, 29 insertions(+) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index e0247d5b0fe..5010ceca414 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -313,6 +313,34 @@ bool PlaylistDAO::setPlaylistLocked(const int playlistId, const bool locked) { return true; } +int PlaylistDAO::setPlaylistsLocked(const QStringList& idStringList, const bool lock) { + if (idStringList.isEmpty()) { + return 0; + } + if (lock) { + qInfo() << "Locking" << idStringList.size() << "playlists"; + } else { + qInfo() << "Unlocking" << idStringList.size() << "playlists"; + } + + QString idString = idStringList.join(","); + + QSqlQuery query(m_database); + query.prepare(QStringLiteral( + "UPDATE Playlists SET locked = :lock WHERE id IN (%1)") + .arg(idString)); + // SQLite3 doesn't support boolean value. Using integer instead. + int iLock = lock ? 1 : 0; + query.bindValue(":lock", iLock); + + if (!query.exec()) { + LOG_FAILED_QUERY(query); + return -1; + } + + return idStringList.length(); +} + bool PlaylistDAO::isPlaylistLocked(const int playlistId) const { QSqlQuery query(m_database); query.prepare(QStringLiteral( diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index f4e6a84af59..9250d0bbc07 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -69,6 +69,7 @@ class PlaylistDAO : public QObject, public virtual DAO { void renamePlaylist(const int playlistId, const QString& newName); // Lock or unlock a playlist bool setPlaylistLocked(const int playlistId, const bool locked); + int setPlaylistsLocked(const QStringList& idStringList, const bool lock); // Find out the state of a playlist lock bool isPlaylistLocked(const int playlistId) const; // Append a list of tracks to a playlist From b046caeb433755fea48cabb4c7d7f60c52648bc5 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 9 Feb 2023 22:23:17 +0100 Subject: [PATCH 17/44] History: allow un/locking all playlists in a YEAR folder --- src/library/trackset/baseplaylistfeature.cpp | 5 ++ src/library/trackset/setlogfeature.cpp | 60 +++++++++++++++++++- src/library/trackset/setlogfeature.h | 5 ++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 9acb7411831..2da0442b879 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -738,6 +738,11 @@ void BasePlaylistFeature::htmlLinkClicked(const QUrl& link) { void BasePlaylistFeature::updateChildModel(int playlistId) { // qDebug() << "BasePlaylistFeature::updateChildModel:" << playlistId; + // TODO(ronso0) refactor for efficiency by using a QSet to avoid + // countless TreeItem iterations when bulk-locking + // See slotPlaylistContentChanged() + // This would also allow updating the tree on lockChanged signal, instead of + // rebuilding it (collapsed/expanded states are reset). if (playlistId == kInvalidPlaylistId) { return; } diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 20f0908daea..57d9acbd20c 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -72,6 +72,18 @@ SetlogFeature::SetlogFeature( this, &SetlogFeature::slotGetNewPlaylist); + m_pLockAllChildPlaylists = new QAction(tr("Lock all child playlists"), this); + connect(m_pLockAllChildPlaylists, + &QAction::triggered, + this, + &SetlogFeature::slotLockAllChildPlaylists); + + m_pUnlockAllChildPlaylists = new QAction(tr("Unlock all child playlists"), this); + connect(m_pUnlockAllChildPlaylists, + &QAction::triggered, + this, + &SetlogFeature::slotUnlockAllChildPlaylists); + m_pDeleteAllChildPlaylists = new QAction(tr("Delete all child playlists"), this); connect(m_pDeleteAllChildPlaylists, &QAction::triggered, @@ -162,10 +174,10 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex QMenu menu(m_pSidebarWidget); if (playlistId == m_placeholderId) { // this is a YEAR item + menu.addAction(m_pLockAllChildPlaylists); + menu.addAction(m_pUnlockAllChildPlaylists); + menu.addSeparator(); menu.addAction(m_pDeleteAllChildPlaylists); - // TODO(ronso0) Allow un/locking all child playlists? - // This is handy if you want to either bulk-secure setlogs, or prepare - // further cleanup / rename } else { // this is a playlist bool locked = m_playlistDao.isPlaylistLocked(playlistId); @@ -404,6 +416,48 @@ void SetlogFeature::slotJoinWithPrevious() { } } +void SetlogFeature::slotLockAllChildPlaylists() { + lockOrUnlockAllChildPlaylists(true); +} + +void SetlogFeature::slotUnlockAllChildPlaylists() { + lockOrUnlockAllChildPlaylists(false); +} + +void SetlogFeature::lockOrUnlockAllChildPlaylists(bool lock) { + if (!m_lastRightClickedIndex.isValid()) { + return; + } + if (lock) { + qWarning() << "lock all child playlists of" << m_lastRightClickedIndex.data().toString(); + } else { + qWarning() << "unlock all child playlists of" << m_lastRightClickedIndex.data().toString(); + } + // Ask for confirmation: "Delete all playlists in this group, including locked?" + TreeItem* item = static_cast(m_lastRightClickedIndex.internalPointer()); + if (!item) { + return; + } + const QList yearChildren = item->children(); + if (yearChildren.isEmpty()) { + return; + } + + QStringList ids; + for (auto* child : qAsConst(yearChildren)) { + bool ok = false; + int childId = child->getData().toInt(&ok); + if (ok && childId != kInvalidPlaylistId) { + ids.append(child->getData().toString()); + } + } + m_playlistDao.setPlaylistsLocked(ids, lock); + + // refresh sidebar model manually since PlaylistDAO::setPlaylistsLocked doesn't + // emit a signal we can connect to slotPlaylistTableLockChanged() + slotPlaylistTableChanged(kInvalidPlaylistId); +} + void SetlogFeature::slotDeleteAllChildPlaylists() { if (!m_lastRightClickedIndex.isValid()) { return; diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index ec2839ef2da..c1217eef372 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -27,6 +27,8 @@ class SetlogFeature : public BasePlaylistFeature { void onRightClick(const QPoint& globalPos) override; void onRightClickChild(const QPoint& globalPos, const QModelIndex& index) override; void slotJoinWithPrevious(); + void slotLockAllChildPlaylists(); + void slotUnlockAllChildPlaylists(); void slotDeletePlaylist() override; void slotGetNewPlaylist(); void activate() override; @@ -47,11 +49,14 @@ class SetlogFeature : public BasePlaylistFeature { private: void deleteAllUnlockedPlaylistsWithFewerTracks(); void reloadChildModel(int playlistId); + void lockOrUnlockAllChildPlaylists(bool lock); QString getRootViewHtml() const override; std::list m_recentTracks; QAction* m_pJoinWithPreviousAction; QAction* m_pStartNewPlaylist; + QAction* m_pLockAllChildPlaylists; + QAction* m_pUnlockAllChildPlaylists; QAction* m_pDeleteAllChildPlaylists; int m_playlistId; From 302215e9d5f8ee7d6a373be037a0c4c80af6a0b2 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 26 Jan 2023 15:40:59 +0100 Subject: [PATCH 18/44] PlaylistDAO: add getNextPlaylist(), to be used for History feature --- src/library/dao/playlistdao.cpp | 20 ++++++++++++++++++++ src/library/dao/playlistdao.h | 3 +++ 2 files changed, 23 insertions(+) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 5010ceca414..74faadfc864 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -810,6 +810,26 @@ int PlaylistDAO::getPreviousPlaylist(const int currentPlaylistId, HiddenType hid return kInvalidPlaylistId; } +int PlaylistDAO::getNextPlaylist(const int currentPlaylistId, HiddenType hidden) const { + QSqlQuery query(m_database); + query.prepare(QStringLiteral( + "SELECT max(id) as id FROM Playlists " + "WHERE id > :id AND hidden = :hidden")); + query.bindValue(":id", currentPlaylistId); + query.bindValue(":hidden", hidden); + + if (!query.exec()) { + LOG_FAILED_QUERY(query); + return kInvalidPlaylistId; + } + + // Get the id of the lowest playlist + if (query.next()) { + return query.value(query.record().indexOf("id")).toInt(); + } + return kInvalidPlaylistId; +} + bool PlaylistDAO::copyPlaylistTracks(const int sourcePlaylistID, const int targetPlaylistID) { // Start the transaction ScopedTransaction transaction(m_database); diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index 9250d0bbc07..687cfabfe28 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -114,6 +114,9 @@ class PlaylistDAO : public QObject, public virtual DAO { // Get the preceding playlist of currentPlaylistId with the HiddenType // hidden. Returns -1 if no such playlist exists. int getPreviousPlaylist(const int currentPlaylistId, HiddenType hidden) const; + // Get the following playlist of currentPlaylistId with the HiddenType + // hidden. Returns -1 if no such playlist exists. + int getNextPlaylist(const int currentPlaylistId, HiddenType hidden) const; // Append all the tracks in the source playlist to the target playlist. bool copyPlaylistTracks(const int sourcePlaylistID, const int targetPlaylistID); // Returns the number of tracks in the given playlist. From 2aebd75d7a92e5a5d9f3f8c299c0ed1dca2a4cde Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 28 Jan 2023 02:26:52 +0100 Subject: [PATCH 19/44] Library sidebar: handle item selection in DAO callback slots ... after the playlist model changed. This is easier to maintain and more robust than doing it in the action action slots, i.e. it gives consistent right-click & select UX --- src/library/trackset/baseplaylistfeature.cpp | 26 +--- src/library/trackset/crate/cratefeature.cpp | 36 +++--- src/library/trackset/playlistfeature.cpp | 23 +++- src/library/trackset/setlogfeature.cpp | 120 ++++++++++++++----- src/library/trackset/setlogfeature.h | 1 - 5 files changed, 132 insertions(+), 74 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 2da0442b879..08f4ed4de68 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -325,11 +325,8 @@ void BasePlaylistFeature::slotDuplicatePlaylist() { int newPlaylistId = m_playlistDao.createPlaylist(name); - if (newPlaylistId != kInvalidPlaylistId && - m_playlistDao.copyPlaylistTracks(oldPlaylistId, newPlaylistId)) { - // Note: this assumes the sidebar model was already updated by slotPlaylisttableChanged - // and the sidebar scrolled to the new playlist - activatePlaylist(oldPlaylistId); + if (newPlaylistId != kInvalidPlaylistId) { + m_playlistDao.copyPlaylistTracks(oldPlaylistId, newPlaylistId); } } @@ -420,21 +417,12 @@ void BasePlaylistFeature::slotDeletePlaylist() { return; } - // we will switch to the sibling if the deleted playlist is currently active - bool wasActive = m_pPlaylistTableModel->getPlaylist() == playlistId; - - VERIFY_OR_DEBUG_ASSERT(playlistId >= 0) { - return; - } - bool locked = m_playlistDao.isPlaylistLocked(playlistId); if (locked) { qDebug() << "Skipping playlist deletion because playlist" << playlistId << "is locked."; return; } - int siblingId = getSiblingPlaylistIdOf(m_lastRightClickedIndex); - QMessageBox::StandardButton btn = QMessageBox::question(nullptr, tr("Confirm Deletion"), tr("Do you really want to delete playlist %1?") @@ -446,15 +434,6 @@ void BasePlaylistFeature::slotDeletePlaylist() { } m_playlistDao.deletePlaylist(playlistId); - - if (siblingId == kInvalidPlaylistId) { - return; - } - if (wasActive) { - activatePlaylist(siblingId); - } else if (m_pSidebarWidget) { - m_pSidebarWidget->selectChildIndex(indexFromPlaylistId(siblingId), false); - } } void BasePlaylistFeature::slotImportPlaylist() { @@ -770,6 +749,7 @@ void BasePlaylistFeature::updateChildModel(int playlistId) { /// Clears the child model dynamically, but the invisible root item remains void BasePlaylistFeature::clearChildModel() { + m_lastClickedIndex = QModelIndex(); m_lastRightClickedIndex = QModelIndex(); m_pSidebarModel->removeRows(0, m_pSidebarModel->rowCount()); } diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 718f51b7043..872ad49d408 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -301,13 +301,14 @@ void CrateFeature::activate() { } void CrateFeature::activateChild(const QModelIndex& index) { - //qDebug() << "CrateFeature::activateChild()" << index; + qDebug() << " CrateFeature::activateChild()" << index; CrateId crateId(crateIdFromIndex(index)); VERIFY_OR_DEBUG_ASSERT(crateId.isValid()) { return; } m_lastClickedIndex = index; m_lastRightClickedIndex = QModelIndex(); + m_prevSiblingCrate = CrateId(); emit saveModelState(); m_crateTableModel.selectCrate(crateId); emit showTrackModel(&m_crateTableModel); @@ -323,9 +324,10 @@ bool CrateFeature::activateCrate(CrateId crateId) { VERIFY_OR_DEBUG_ASSERT(index.isValid()) { return false; } - emit saveModelState(); m_lastClickedIndex = index; m_lastRightClickedIndex = QModelIndex(); + m_prevSiblingCrate = CrateId(); + emit saveModelState(); m_crateTableModel.selectCrate(crateId); emit showTrackModel(&m_crateTableModel); emit enableCoverArtDisplay(true); @@ -434,7 +436,11 @@ void CrateFeature::slotDeleteCrate() { 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); + m_prevSiblingCrate = CrateId(); + if (isChildIndexSelectedInSidebar(m_lastRightClickedIndex)) { + storePrevSiblingCrateId(crateId); + } + QMessageBox::StandardButton btn = QMessageBox::question(nullptr, tr("Confirm Deletion"), tr("Do you really want to delete crate %1?") @@ -509,13 +515,10 @@ void CrateFeature::slotDuplicateCrate() { .duplicateCrate(crate); if (newCrateId.isValid()) { qDebug() << "Duplicate crate" << crate << ", new crate:" << newCrateId; - // expand Crates and scroll to new crate - m_pSidebarWidget->selectChildIndex(indexFromCrateId(newCrateId), false); - activateCrate(crate.getId()); + return; } - } else { - qDebug() << "Failed to duplicate selected crate"; } + qDebug() << "Failed to duplicate selected crate"; } void CrateFeature::slotToggleCrateLock() { @@ -856,15 +859,14 @@ void CrateFeature::storePrevSiblingCrateId(CrateId crateId) { } void CrateFeature::slotCrateTableChanged(CrateId crateId) { - if (m_lastRightClickedIndex.isValid() && - (crateIdFromIndex(m_lastRightClickedIndex) == crateId)) { - // 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 + Q_UNUSED(crateId); + if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { + // If the previously selected crate was loaded to the tracks table and + // selected in the sidebar try to activate that or a sibling + rebuildChildModel(); + if (!activateCrate(m_crateTableModel.selectedCrate())) { + // probably last clicked crate was deleted, try to + // select the stored sibling activateCrate(m_prevSiblingCrate); } } else { diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index c6ac580a765..eea83959fc5 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -278,8 +278,29 @@ void PlaylistFeature::slotPlaylistTableChanged(int playlistId) { enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); if (type == PlaylistDAO::PLHT_NOT_HIDDEN || type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist + // Store current selection + int selectedPlaylistId = kInvalidPlaylistId; + if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { + if (playlistId == playlistIdFromIndex(m_lastClickedIndex) && + type == PlaylistDAO::PLHT_UNKNOWN) { + // if the selected playlist was deleted, find a sibling to select + selectedPlaylistId = getSiblingPlaylistIdOf(m_lastClickedIndex); + } else { + // just restore the current selection + selectedPlaylistId = playlistIdFromIndex(m_lastClickedIndex); + } + } + clearChildModel(); - m_lastRightClickedIndex = constructChildModel(playlistId); + QModelIndex newIndex = constructChildModel(selectedPlaylistId); + if (newIndex.isValid()) { + // If a child index was selected and we got a new valid index select that. + // Else (root item was selected or for some reason no index could be created) + // there's nothing to do: either no child was selected earlier, or the root + // was selected and will remain selected after the child model was rebuilt. + activateChild(newIndex); + emit featureSelect(this, newIndex); + } } } diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 57d9acbd20c..6fd548f3f7e 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -233,6 +233,8 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { int idColumn = record.indexOf("id"); int createdColumn = record.indexOf("date_created"); + // Nice to have: restore previous expanded/collapsed state of YEAR items + clearChildModel(); QMap groups; std::vector> itemList; // Generous estimate (number of years the db is used ;)) @@ -253,16 +255,20 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { .toDateTime(); // Create the TreeItem whose parent is the invisible root item - // Show only [kNumToplevelHistoryEntries -1] recent playlists at the top - // level before grouping them by year. + // Show only [kNumToplevelHistoryEntries] recent playlists at the top level + // before grouping them by year. if (row >= kNumToplevelHistoryEntries) { + // group by year int yearCreated = dateCreated.date().year(); auto i = groups.find(yearCreated); TreeItem* pGroupItem; - if (i != groups.end()) { + if (i != groups.end() && i.key() == yearCreated) { + // get YEAR item the playlist will sorted into pGroupItem = i.value(); } else { + // create YEAR item the playlist will sorted into + // store id of empty placeholder playlist auto pNewGroupItem = std::make_unique( QString::number(yearCreated), m_placeholderId); pGroupItem = pNewGroupItem.get(); @@ -274,6 +280,7 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { pItem->setBold(m_playlistIdsOfSelectedTrack.contains(id)); decorateChild(pItem, id); } else { + // add most recent top-level playlist auto pItem = std::make_unique(name, id); pItem->setBold(m_playlistIdsOfSelectedTrack.contains(id)); decorateChild(pItem.get(), id); @@ -285,10 +292,7 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { // Append all the newly created TreeItems in a dynamic way to the childmodel m_pSidebarModel->insertTreeItemRows(std::move(itemList), 0); - if (selectedId) { - return indexFromPlaylistId(selectedId); - } - return QModelIndex(); + return indexFromPlaylistId(selectedId); } QString SetlogFeature::fetchPlaylistLabel(int playlistId) { @@ -405,11 +409,7 @@ void SetlogFeature::slotJoinWithPrevious() { << " previous:" << previousPlaylistId; if (m_playlistDao.copyPlaylistTracks( currentPlaylistId, previousPlaylistId)) { - m_lastRightClickedIndex = constructChildModel(previousPlaylistId); m_playlistDao.deletePlaylist(currentPlaylistId); - reloadChildModel(previousPlaylistId); // For moving selection - emit showTrackModel(m_pPlaylistTableModel); - activatePlaylist(previousPlaylistId); } } } @@ -582,16 +582,65 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { } void SetlogFeature::slotPlaylistTableChanged(int playlistId) { - reloadChildModel(playlistId); -} - -void SetlogFeature::reloadChildModel(int playlistId) { //qDebug() << "updateChildModel() playlistId:" << playlistId; PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); - if (type == PlaylistDAO::PLHT_SET_LOG || - type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist - clearChildModel(); - m_lastRightClickedIndex = constructChildModel(playlistId); + if (type != PlaylistDAO::PLHT_SET_LOG && + type != PlaylistDAO::PLHT_UNKNOWN) { // deleted Playlist + return; + } + + // save currently selected History sidebar item (if any) + int selectedYearIndexRow = -1; + int selectedPlaylistId = kInvalidPlaylistId; + bool rootWasSelected = false; + if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { + // a child index was selected (actual playlist or YEAR item) + int lastClickedPlaylistId = m_pPlaylistTableModel->getPlaylist(); + if (lastClickedPlaylistId == m_placeholderId) { + // a YEAR item was selected + selectedYearIndexRow = m_lastClickedIndex.row(); + } else if (playlistId == lastClickedPlaylistId && + type == PlaylistDAO::PLHT_UNKNOWN) { + // selected playlist was deleted, find a sibling. + // prev/next works here because history playlists are always + // sorted by date of creation. + selectedPlaylistId = m_playlistDao.getPreviousPlaylist( + lastClickedPlaylistId, + PlaylistDAO::PLHT_SET_LOG); + if (selectedPlaylistId == kInvalidPlaylistId) { + // no previous playlist, try to get the next playlist + selectedPlaylistId = m_playlistDao.getNextPlaylist( + lastClickedPlaylistId, + PlaylistDAO::PLHT_SET_LOG); + } + } else { + selectedPlaylistId = lastClickedPlaylistId; + } + } else { + rootWasSelected = m_pSidebarWidget && + m_pSidebarWidget->isFeatureRootIndexSelected(this); + } + + QModelIndex newIndex = constructChildModel(selectedPlaylistId); + + // restore selection + if (selectedYearIndexRow != -1) { + // if row is valid this means newIndex is invalid anyway + newIndex = m_pSidebarModel->index(selectedYearIndexRow, 0); + if (!newIndex.isValid()) { + // seems like we deleted the oldest (bottom) YEAR node while it was + // selected. Try to pick the row above + newIndex = m_pSidebarModel->index(selectedYearIndexRow - 1, 0); + } + } + if (newIndex.isValid()) { + emit featureSelect(this, newIndex); + activateChild(newIndex); + return; + } else if (rootWasSelected) { + // calling featureSelect with invalid index will select the root item + emit featureSelect(this, newIndex); + activate(); // to reload the new current playlist } } @@ -620,7 +669,7 @@ void SetlogFeature::slotPlaylistTableRenamed(int playlistId, const QString& newN void SetlogFeature::activate() { // The root item was clicked, so actuvate the current playlist. - m_lastClickedIndex = QModelIndex(); + m_lastClickedIndex = m_pSidebarModel->getRootIndex(); activatePlaylist(m_playlistId); } @@ -630,18 +679,25 @@ void SetlogFeature::activatePlaylist(int playlistId) { return; } QModelIndex index = indexFromPlaylistId(playlistId); - if (index.isValid()) { - emit saveModelState(); - m_pPlaylistTableModel->setTableModel(playlistId); - emit showTrackModel(m_pPlaylistTableModel); - emit enableCoverArtDisplay(true); - // Update sidebar selection only if this is a child, incl. current playlist. - // indexFromPlaylistId() can't be used because, in case the root item was - // selected, that would switch to the 'current' child. - if (m_lastClickedIndex.isValid()) { - emit featureSelect(this, index); - activateChild(index); - } + VERIFY_OR_DEBUG_ASSERT(index.isValid()) { + return; + } + emit saveModelState(); + m_pPlaylistTableModel->setTableModel(playlistId); + emit showTrackModel(m_pPlaylistTableModel); + emit enableCoverArtDisplay(true); + // Update sidebar selection only if this is a child, incl. current playlist. + // indexFromPlaylistId() can't be used because, in case the root item was + // selected, that would switch to the 'current' child. + if (m_lastClickedIndex != m_pSidebarModel->getRootIndex()) { + m_lastClickedIndex = index; + emit featureSelect(this, index); + // redundant + // activateChild(index); + + // TODO(ronso0) Disable search for YEAR items + // emit disableSearch(); + // emit enableCoverArtDisplay(false); } } diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index c1217eef372..ff601eae61c 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -48,7 +48,6 @@ class SetlogFeature : public BasePlaylistFeature { private: void deleteAllUnlockedPlaylistsWithFewerTracks(); - void reloadChildModel(int playlistId); void lockOrUnlockAllChildPlaylists(bool lock); QString getRootViewHtml() const override; From 8740178cdd55524e0cdb04a6f50867a7d24d8393 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Mon, 6 Feb 2023 02:46:19 +0100 Subject: [PATCH 20/44] Library sidebar: add helper to get the (first) selected index --- src/widget/wlibrarysidebar.cpp | 71 ++++++++++++++-------------------- src/widget/wlibrarysidebar.h | 1 + 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 060a61c97e0..50459cfdd1e 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -178,9 +178,8 @@ void WLibrarySidebar::dropEvent(QDropEvent * event) { } void WLibrarySidebar::toggleSelectedItem() { - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.size() > 0) { - QModelIndex index = selectedIndices.at(0); + QModelIndex index = selectedIndex(); + if (index.isValid()) { // Activate the item so its content shows in the main library. emit pressed(index); // Expand or collapse the item as necessary. @@ -189,9 +188,8 @@ void WLibrarySidebar::toggleSelectedItem() { } bool WLibrarySidebar::isLeafNodeSelected() { - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.size() > 0) { - QModelIndex index = selectedIndices.at(0); + QModelIndex index = selectedIndex(); + if (index.isValid()) { if(!index.model()->hasChildren(index)) { return true; } @@ -205,9 +203,8 @@ bool WLibrarySidebar::isLeafNodeSelected() { bool WLibrarySidebar::isChildIndexSelected(const QModelIndex& index) { // qDebug() << "WLibrarySidebar::isChildIndexSelected" << index; - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.size() <= 0) { - // qWarning() << " >> no selection"; + QModelIndex selIndex = selectedIndex(); + if (!selIndex.isValid()) { return false; } SidebarModel* sidebarModel = qobject_cast(model()); @@ -220,13 +217,13 @@ bool WLibrarySidebar::isChildIndexSelected(const QModelIndex& index) { // qDebug() << " >> index can't be translated"; return false; } - return translated == selectedIndices.first(); + return translated == selIndex; } bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { // qDebug() << "WLibrarySidebar::isFeatureRootIndexSelected"; - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.size() <= 0) { + QModelIndex selIndex = selectedIndex(); + if (!selIndex.isValid()) { return false; } SidebarModel* sidebarModel = qobject_cast(model()); @@ -234,7 +231,7 @@ bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { return false; } const QModelIndex rootIndex = sidebarModel->getFeatureRootIndex(pFeature); - return rootIndex == selectedIndices.first(); + return rootIndex == selIndex; } /// Invoked by actual keypresses (requires widget focus) and emulated keypresses @@ -260,13 +257,8 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { QTreeView::keyPressEvent(event); // After the selection changed force-activate (click) the newly selected // item to save us from having to push "Enter". - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.isEmpty()) { - return; - } - QModelIndex selIndex = selectedIndices.first(); - VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { - qDebug() << "invalid sidebar index"; + QModelIndex selIndex = selectedIndex(); + if (!selIndex.isValid()) { return; } // Ensure the new selection is visible even if it was already selected/ @@ -282,7 +274,7 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { return; } // If an expanded item is selected let QTreeView collapse it - QModelIndex selIndex = selectedIndices.first(); + QModelIndex selIndex = selectedIndex(); VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { qDebug() << "invalid sidebar index"; return; @@ -306,11 +298,7 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { return; case kRenameSidebarItemShortcutKey: { // F2 // Rename crate or playlist (internal, external, history) - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.isEmpty()) { - return; - } - QModelIndex selIndex = selectedIndices.first(); + QModelIndex selIndex = selectedIndex(); VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { qDebug() << "invalid sidebar index"; return; @@ -327,13 +315,8 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* event) { if (event->modifiers() != kHideRemoveShortcutModifier) { return; } - QModelIndexList selectedIndices = selectionModel()->selectedRows(); - if (selectedIndices.isEmpty()) { - return; - } - QModelIndex selIndex = selectedIndices.first(); - VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { - qDebug() << "invalid sidebar index"; + QModelIndex selIndex = selectedIndex(); + if (!selIndex.isValid()) { return; } emit deleteItem(selIndex); @@ -406,21 +389,25 @@ void WLibrarySidebar::selectChildIndex(const QModelIndex& index, bool selectItem scrollTo(translated, EnsureVisible); } -/// Refocus the selected item after right-click -void WLibrarySidebar::focusSelectedIndex() { - // After the context menu was activated (and closed, with or without clicking - // an action), the currentIndex is the right-clicked item. - // If if the currentIndex is not selected, make the selection the currentIndex +QModelIndex WLibrarySidebar::selectedIndex() { QModelIndexList selectedIndices = selectionModel()->selectedRows(); if (selectedIndices.isEmpty()) { - // QTreeView will handle this, i.e. select an index in keyPressEvent() - return; + return QModelIndex(); } QModelIndex selIndex = selectedIndices.first(); VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { - return; + return QModelIndex(); } - if (selIndex != selectionModel()->currentIndex()) { + return selIndex; +} + +/// Refocus the selected item after right-click +void WLibrarySidebar::focusSelectedIndex() { + // After the context menu was activated (and closed, with or without clicking + // an action), the currentIndex is the right-clicked item. + // If if the currentIndex is not selected, make the selection the currentIndex + QModelIndex selIndex = selectedIndex(); + if (selIndex.isValid() && selIndex != selectionModel()->currentIndex()) { setCurrentIndex(selIndex); } } diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index 555833772c7..ece49c6fca9 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -50,6 +50,7 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { private: void focusSelectedIndex(); + QModelIndex selectedIndex(); QBasicTimer m_expandTimer; QModelIndex m_hoverIndex; From bf28a0726b9be05ad14371d66587db5beeb8b341 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Mon, 6 Feb 2023 02:47:51 +0100 Subject: [PATCH 21/44] Library sidebar: toggleItem() emits click(), not delayed pressed() --- src/widget/wlibrarysidebar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 50459cfdd1e..6b25a0f16de 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -181,7 +181,7 @@ void WLibrarySidebar::toggleSelectedItem() { QModelIndex index = selectedIndex(); if (index.isValid()) { // Activate the item so its content shows in the main library. - emit pressed(index); + emit clicked(index); // Expand or collapse the item as necessary. setExpanded(index, !isExpanded(index)); } From 693f8a4c98750cba53ee3ce0113bc4fa2863d43a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 10 Feb 2023 00:36:14 +0100 Subject: [PATCH 22/44] BasePlaylistFeature::updateChildModel: use QSet to reduce tree iterations for example, updateChildModel would iterate over the tree model for each playlist id received from PlaylistDAO::tracksChanged. With a QSet the sidebar can now simply by updated (renamed, decorated) after bulk-un/lock in History which previously required to rebuild the model and thereby reset the expanded/collapsed states of nodes. --- src/library/dao/playlistdao.cpp | 17 ++++++--- src/library/dao/playlistdao.h | 4 +- src/library/trackset/baseplaylistfeature.cpp | 40 ++++++++++---------- src/library/trackset/baseplaylistfeature.h | 5 +-- src/library/trackset/playlistfeature.cpp | 16 +++----- src/library/trackset/playlistfeature.h | 3 +- src/library/trackset/setlogfeature.cpp | 25 +++++------- src/library/trackset/setlogfeature.h | 3 +- 8 files changed, 53 insertions(+), 60 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 74faadfc864..b2b07646d4d 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -309,20 +309,24 @@ bool PlaylistDAO::setPlaylistLocked(const int playlistId, const bool locked) { LOG_FAILED_QUERY(query); return false; } - emit lockChanged(playlistId); + emit lockChanged(QSet{playlistId}); return true; } -int PlaylistDAO::setPlaylistsLocked(const QStringList& idStringList, const bool lock) { - if (idStringList.isEmpty()) { +int PlaylistDAO::setPlaylistsLocked(const QSet& playlistIds, const bool lock) { + if (playlistIds.isEmpty()) { return 0; } if (lock) { - qInfo() << "Locking" << idStringList.size() << "playlists"; + qInfo() << "Locking" << playlistIds.size() << "playlists"; } else { - qInfo() << "Unlocking" << idStringList.size() << "playlists"; + qInfo() << "Unlocking" << playlistIds.size() << "playlists"; } + QStringList idStringList; + for (int id : qAsConst(playlistIds)) { + idStringList.append(QString::number(id)); + } QString idString = idStringList.join(","); QSqlQuery query(m_database); @@ -338,7 +342,8 @@ int PlaylistDAO::setPlaylistsLocked(const QStringList& idStringList, const bool return -1; } - return idStringList.length(); + emit lockChanged(playlistIds); + return playlistIds.size(); } bool PlaylistDAO::isPlaylistLocked(const int playlistId) const { diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index 687cfabfe28..c08786ac993 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -69,7 +69,7 @@ class PlaylistDAO : public QObject, public virtual DAO { void renamePlaylist(const int playlistId, const QString& newName); // Lock or unlock a playlist bool setPlaylistLocked(const int playlistId, const bool locked); - int setPlaylistsLocked(const QStringList& idStringList, const bool lock); + int setPlaylistsLocked(const QSet& playlistIds, const bool lock); // Find out the state of a playlist lock bool isPlaylistLocked(const int playlistId) const; // Append a list of tracks to a playlist @@ -136,7 +136,7 @@ class PlaylistDAO : public QObject, public virtual DAO { void added(int playlistId); void deleted(int playlistId); void renamed(int playlistId, const QString& newName); - void lockChanged(int playlistId); + void lockChanged(const QSet& playlistIds); void trackAdded(int playlistId, TrackId trackId, int position); void trackRemoved(int playlistId, TrackId trackId, int position); void tracksChanged(const QSet& playlistIds); // added/removed/reordered diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 08f4ed4de68..5d912cbaf8d 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -152,7 +152,7 @@ void BasePlaylistFeature::connectPlaylistDAO() { connect(&m_playlistDao, &PlaylistDAO::lockChanged, this, - &BasePlaylistFeature::slotPlaylistTableLockChanged); + &BasePlaylistFeature::slotPlaylistContentOrLockChanged); connect(&m_playlistDao, &PlaylistDAO::deleted, this, @@ -160,7 +160,7 @@ void BasePlaylistFeature::connectPlaylistDAO() { connect(&m_playlistDao, &PlaylistDAO::tracksChanged, this, - &BasePlaylistFeature::slotPlaylistContentChanged); + &BasePlaylistFeature::slotPlaylistContentOrLockChanged); connect(&m_playlistDao, &PlaylistDAO::renamed, this, @@ -715,18 +715,15 @@ void BasePlaylistFeature::htmlLinkClicked(const QUrl& link) { } } -void BasePlaylistFeature::updateChildModel(int playlistId) { - // qDebug() << "BasePlaylistFeature::updateChildModel:" << playlistId; - // TODO(ronso0) refactor for efficiency by using a QSet to avoid - // countless TreeItem iterations when bulk-locking - // See slotPlaylistContentChanged() - // This would also allow updating the tree on lockChanged signal, instead of - // rebuilding it (collapsed/expanded states are reset). - if (playlistId == kInvalidPlaylistId) { +void BasePlaylistFeature::updateChildModel(const QSet& playlistIds) { + // qDebug() << "BasePlaylistFeature::updateChildModel"; + if (playlistIds.isEmpty()) { return; } - QString playlistLabel = fetchPlaylistLabel(playlistId); - QVariant variantId = QVariant(playlistId); + + int id = kInvalidPlaylistId; + QString label; + bool ok = false; for (int row = 0; row < m_pSidebarModel->rowCount(); ++row) { QModelIndex index = m_pSidebarModel->index(row, 0); @@ -734,15 +731,20 @@ void BasePlaylistFeature::updateChildModel(int playlistId) { DEBUG_ASSERT(pTreeItem != nullptr); if (pTreeItem->hasChildren()) { for (TreeItem* pChild : qAsConst(pTreeItem->children())) { - if (pChild->getData() == variantId) { - pTreeItem = pChild; - break; + id = pChild->getData().toInt(&ok); + if (ok && id != kInvalidPlaylistId && playlistIds.contains(id)) { + label = m_playlistDao.getPlaylistName(id); + pChild->setLabel(label); + decorateChild(pChild, id); } } - } - if (pTreeItem->getData() == variantId) { - pTreeItem->setLabel(playlistLabel); - decorateChild(pTreeItem, playlistId); + } else { + id = pTreeItem->getData().toInt(&ok); + if (ok && id != kInvalidPlaylistId && playlistIds.contains(id)) { + label = m_playlistDao.getPlaylistName(id); + pTreeItem->setLabel(label); + decorateChild(pTreeItem, id); + } } } } diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index cadbb915f2a..3eb0829e6cb 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -54,9 +54,8 @@ class BasePlaylistFeature : public BaseTrackSetFeature { slotPlaylistTableChanged(playlistId); selectPlaylistInSidebar(playlistId, false); }; - virtual void slotPlaylistTableLockChanged(int playlistId) = 0; + virtual void slotPlaylistContentOrLockChanged(const QSet& playlistIds) = 0; virtual void slotPlaylistTableRenamed(int playlistId, const QString& newName) = 0; - virtual void slotPlaylistContentChanged(QSet playlistIds) = 0; void slotCreatePlaylist(); void renameItem(const QModelIndex& index) override; void deleteItem(const QModelIndex& index) override; @@ -83,7 +82,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature { QString label; }; - virtual void updateChildModel(int selected_id); + virtual void updateChildModel(const QSet& playlistIds); virtual void clearChildModel(); virtual QString fetchPlaylistLabel(int playlistId) = 0; diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index eea83959fc5..647fbef6d69 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -304,19 +304,15 @@ void PlaylistFeature::slotPlaylistTableChanged(int playlistId) { } } -void PlaylistFeature::slotPlaylistContentChanged(QSet playlistIds) { +void PlaylistFeature::slotPlaylistContentOrLockChanged(const QSet& playlistIds) { + // qDebug() << "slotPlaylistContentOrLockChanged() playlistId:" << playlistId; + QSet idsToBeUpdated; for (const auto playlistId : qAsConst(playlistIds)) { if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { - updateChildModel(playlistId); + idsToBeUpdated.insert(playlistId); } } -} - -void PlaylistFeature::slotPlaylistTableLockChanged(int playlistId) { - // qDebug() << "slotPlaylistTableLockChanged() playlistId:" << playlistId; - if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { - updateChildModel(playlistId); - } + updateChildModel(idsToBeUpdated); } void PlaylistFeature::slotPlaylistTableRenamed( @@ -324,7 +320,7 @@ void PlaylistFeature::slotPlaylistTableRenamed( Q_UNUSED(newName); //qDebug() << "slotPlaylistTableChanged() playlistId:" << playlistId; if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { - updateChildModel(playlistId); + updateChildModel(QSet{playlistId}); } } diff --git a/src/library/trackset/playlistfeature.h b/src/library/trackset/playlistfeature.h index 162f6a6cfbe..9f4d04db7dc 100644 --- a/src/library/trackset/playlistfeature.h +++ b/src/library/trackset/playlistfeature.h @@ -37,8 +37,7 @@ class PlaylistFeature : public BasePlaylistFeature { private slots: void slotPlaylistTableChanged(int playlistId) override; - void slotPlaylistContentChanged(QSet playlistIds) override; - void slotPlaylistTableLockChanged(int playlistId) override; + void slotPlaylistContentOrLockChanged(const QSet& playlistIds) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; protected: diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 6fd548f3f7e..a030cffc800 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -297,6 +297,7 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { QString SetlogFeature::fetchPlaylistLabel(int playlistId) { // Setup the sidebar playlist model + // TODO(ronso0) Why not m_playlistDao.getPlaylistName(id) ?? QSqlTableModel playlistTableModel(this, m_pLibrary->trackCollectionManager()->internalCollection()->database()); playlistTableModel.setTable("Playlists"); @@ -443,19 +444,15 @@ void SetlogFeature::lockOrUnlockAllChildPlaylists(bool lock) { return; } - QStringList ids; + QSet ids; for (auto* child : qAsConst(yearChildren)) { bool ok = false; int childId = child->getData().toInt(&ok); if (ok && childId != kInvalidPlaylistId) { - ids.append(child->getData().toString()); + ids.insert(childId); } } m_playlistDao.setPlaylistsLocked(ids, lock); - - // refresh sidebar model manually since PlaylistDAO::setPlaylistsLocked doesn't - // emit a signal we can connect to slotPlaylistTableLockChanged() - slotPlaylistTableChanged(kInvalidPlaylistId); } void SetlogFeature::slotDeleteAllChildPlaylists() { @@ -644,26 +641,22 @@ void SetlogFeature::slotPlaylistTableChanged(int playlistId) { } } -void SetlogFeature::slotPlaylistContentChanged(QSet playlistIds) { +void SetlogFeature::slotPlaylistContentOrLockChanged(const QSet& playlistIds) { + // qDebug() << "slotPlaylistContentOrLockChanged() playlistId:" << playlistId; + QSet idsToBeUpdated; for (const auto playlistId : qAsConst(playlistIds)) { if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { - updateChildModel(playlistId); + idsToBeUpdated.insert(playlistId); } } -} - -void SetlogFeature::slotPlaylistTableLockChanged(int playlistId) { - // qDebug() << "slotPlaylistTableLockChanged() playlistId:" << playlistId; - if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { - updateChildModel(playlistId); - } + updateChildModel(idsToBeUpdated); } void SetlogFeature::slotPlaylistTableRenamed(int playlistId, const QString& newName) { Q_UNUSED(newName); //qDebug() << "slotPlaylistTableRenamed() playlistId:" << playlistId; if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { - updateChildModel(playlistId); + updateChildModel(QSet{playlistId}); } } diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index ff601eae61c..1ed07e4b41a 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -41,8 +41,7 @@ class SetlogFeature : public BasePlaylistFeature { private slots: void slotPlayingTrackChanged(TrackPointer currentPlayingTrack); void slotPlaylistTableChanged(int playlistId) override; - void slotPlaylistContentChanged(QSet playlistIds) override; - void slotPlaylistTableLockChanged(int playlistId) override; + void slotPlaylistContentOrLockChanged(const QSet& playlistIds) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; void slotDeleteAllChildPlaylists(); From 013cbc3280e83510d9da71948e48718eabd1fc38 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 10 Feb 2023 11:22:29 +0100 Subject: [PATCH 23/44] skins: add focus indicator (right-click) to library sidebar items --- res/skins/Deere/style.qss | 11 +++++++++-- res/skins/LateNight/style_classic.qss | 11 ++++++++--- res/skins/Shade/style.qss | 11 +++++++++-- res/skins/Tango/style.qss | 8 ++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/res/skins/Deere/style.qss b/res/skins/Deere/style.qss index bfe72df5aff..f2d06b87589 100644 --- a/res/skins/Deere/style.qss +++ b/res/skins/Deere/style.qss @@ -229,6 +229,14 @@ WLibrarySidebar::item:selected, color: #D6D6D6; background-color: #006596; } +WLibrarySidebar::item:selected:focus { + outline: none; + border: 0px; +} +WLibrarySidebar::item:!selected:focus { + outline: none; + border: 1px solid white; +} /* Use the native focus decoration */ /* This is for all cells including Played and Location */ WTrackTableView, @@ -412,8 +420,7 @@ WSearchLineEdit { qproperty-layoutSpacing: 0; } -WLibrarySidebar, -WLibrarySidebar::item:focus { +WLibrarySidebar { outline: none; /* Spacing between treeview and preview deck/search bar */ margin: 0px; diff --git a/res/skins/LateNight/style_classic.qss b/res/skins/LateNight/style_classic.qss index cc7399d87b1..69025df113e 100644 --- a/res/skins/LateNight/style_classic.qss +++ b/res/skins/LateNight/style_classic.qss @@ -2071,6 +2071,14 @@ WTrackTableView::item:selected, color: #fff; background-color: #5e4507; } +WLibrarySidebar::item:selected:focus { + outline: none; + border: 0px; +} +WLibrarySidebar::item:!selected:focus { + outline: none; + border: 1px solid white; +} /* Use the native focus decoration */ /* This is for all cells including Played and Location */ @@ -2088,9 +2096,6 @@ WTrackTableView { WLibrarySidebar { show-decoration-selected: 0; -} -WLibrarySidebar, -WLibrarySidebar::item:focus { outline: none; } diff --git a/res/skins/Shade/style.qss b/res/skins/Shade/style.qss index ae60e8a8c9f..0f6f4c3c636 100644 --- a/res/skins/Shade/style.qss +++ b/res/skins/Shade/style.qss @@ -470,9 +470,16 @@ WLibrarySidebar::branch:selected, WSearchLineEdit::indicator { width: 0; } -WLibrarySidebar, -WLibrarySidebar::item:focus { +WLibrarySidebar { + outline: none; +} +WLibrarySidebar::item:selected:focus { + outline: none; + border: 0px; +} +WLibrarySidebar::item:!selected:focus { outline: none; + border: 1px solid white; } /* Use the native focus decoration */ diff --git a/res/skins/Tango/style.qss b/res/skins/Tango/style.qss index 496fb41eb09..2570e895eb5 100644 --- a/res/skins/Tango/style.qss +++ b/res/skins/Tango/style.qss @@ -2650,6 +2650,14 @@ WLibrarySidebar::item:!selected { background-color: #0f0f0f; color: #999; } +WLibrarySidebar::item:selected:focus { + outline: none; + border: 0px; +} +WLibrarySidebar::item:!selected:focus { + outline: none; + border: 1px solid white; +} WTrackTableViewHeader { /* Don't set a font size to pick up the system font size. */ From 291888451e223e7d98ab251df33df7c153b92bb5 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 00:23:28 +0100 Subject: [PATCH 24/44] BasePlaylistFeature: improve playlist import comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel Schürmann --- src/library/trackset/baseplaylistfeature.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 5d912cbaf8d..454e3852c5e 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -466,8 +466,10 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlistFile, // folder. We don't need access to this file on a regular basis so we do not // register a security bookmark. - // Create a new table model since the main one might have another playlist - // selected which is not the playlist that received the right-click. + // Create a temporary PlaylistTableModel for the Playlist the entries shall be imported to. + // This is used as a proxy object to write to the database. + // We cannot use m_pPlaylistTableModel since it might have another playlist selected which + // is not the playlist that received the right-click. QScopedPointer pPlaylistTableModel( new PlaylistTableModel(this, m_pLibrary->trackCollectionManager(), From 35a0905074aaa0b34c44dcfd2529322e748eabbf Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 00:24:25 +0100 Subject: [PATCH 25/44] simplify WLibrarySidebar::selectedIndex return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel Schürmann --- src/widget/wlibrarysidebar.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 6b25a0f16de..6bcbef9584f 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -395,9 +395,7 @@ QModelIndex WLibrarySidebar::selectedIndex() { return QModelIndex(); } QModelIndex selIndex = selectedIndices.first(); - VERIFY_OR_DEBUG_ASSERT(selIndex.isValid()) { - return QModelIndex(); - } + DEBUG_ASSERT(selIndex.isValid()); return selIndex; } From 4fe9156f16d6feb8de1f1638e6701d15b67a9f4d Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 00:27:39 +0100 Subject: [PATCH 26/44] playlist features: remove obsolete qAsConst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel Schürmann --- src/library/trackset/baseplaylistfeature.cpp | 2 +- src/library/trackset/setlogfeature.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 454e3852c5e..575d8d98e7d 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -732,7 +732,7 @@ void BasePlaylistFeature::updateChildModel(const QSet& playlistIds) { TreeItem* pTreeItem = m_pSidebarModel->getItem(index); DEBUG_ASSERT(pTreeItem != nullptr); if (pTreeItem->hasChildren()) { - for (TreeItem* pChild : qAsConst(pTreeItem->children())) { + for (TreeItem* pChild : pTreeItem->children()) { id = pChild->getData().toInt(&ok); if (ok && id != kInvalidPlaylistId && playlistIds.contains(id)) { label = m_playlistDao.getPlaylistName(id); diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index a030cffc800..286e7f1111e 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -445,9 +445,9 @@ void SetlogFeature::lockOrUnlockAllChildPlaylists(bool lock) { } QSet ids; - for (auto* child : qAsConst(yearChildren)) { + for (const auto& pChild : yearChildren) { bool ok = false; - int childId = child->getData().toInt(&ok); + int childId = pChild->getData().toInt(&ok); if (ok && childId != kInvalidPlaylistId) { ids.insert(childId); } @@ -493,11 +493,11 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { } QStringList ids; - for (auto* child : qAsConst(yearChildren)) { + for (const auto& pChild : yearChildren) { bool ok = false; - int childId = child->getData().toInt(&ok); + int childId = pChild->getData().toInt(&ok); if (ok && childId != kInvalidPlaylistId) { - ids.append(child->getData().toString()); + ids.append(pChild->getData().toString()); } } m_playlistDao.deletePlaylists(ids, PlaylistDAO::PLHT_SET_LOG); From 40dd91aec8f9907854668bd13b793819f1aca8fa Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 00:59:54 +0100 Subject: [PATCH 27/44] PlaylistDAO: use kInvalidPlaylistId --- src/library/dao/playlistdao.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index b2b07646d4d..a30c6e2d22a 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -250,7 +250,7 @@ int PlaylistDAO::deletePlaylists(const QStringList& idStringList, PlaylistDAO::H return -1; } - emit deleted(-1); + emit deleted(kInvalidPlaylistId); return idStringList.length(); } From 5c2b1f12601d83ef81682dd28e2a6ffac8474896 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 01:00:40 +0100 Subject: [PATCH 28/44] PlaylistDAO: simplify string building for Sql query --- src/library/dao/playlistdao.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index a30c6e2d22a..df80980f471 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -323,16 +323,17 @@ int PlaylistDAO::setPlaylistsLocked(const QSet& playlistIds, const bool loc qInfo() << "Unlocking" << playlistIds.size() << "playlists"; } - QStringList idStringList; - for (int id : qAsConst(playlistIds)) { - idStringList.append(QString::number(id)); + QString idsString; + for (int id : playlistIds) { + idsString += QString::number(id) + ','; } - QString idString = idStringList.join(","); + // strip the last comma + idsString.chop(1); QSqlQuery query(m_database); query.prepare(QStringLiteral( "UPDATE Playlists SET locked = :lock WHERE id IN (%1)") - .arg(idString)); + .arg(idsString)); // SQLite3 doesn't support boolean value. Using integer instead. int iLock = lock ? 1 : 0; query.bindValue(":lock", iLock); From 6c56d35e7ab70769f626f675bcd4cb23822d36c2 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 01:01:25 +0100 Subject: [PATCH 29/44] CrateFeature: use existing table model for importing when crate is selected --- src/library/trackset/crate/cratefeature.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 872ad49d408..96c97c49dab 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -675,13 +675,18 @@ void CrateFeature::slotImportPlaylistFile(const QString& playlistFile, CrateId c return; } - // Create a new table model since the main one might have another crate - // selected which is not the crate that received the right-click. - QScopedPointer pCrateTableModel( - new CrateTableModel(this, m_pLibrary->trackCollectionManager())); - pCrateTableModel->selectCrate(crateId); - pCrateTableModel->select(); - pCrateTableModel->addTracks(QModelIndex(), locations); + if (crateId == m_crateTableModel.selectedCrate()) { + // Add tracks directly to the model + m_crateTableModel.addTracks(QModelIndex(), locations); + } else { + // Create a temporary table model since the main one might have another + // crate selected which is not the crate that received the right-click. + QScopedPointer pCrateTableModel( + new CrateTableModel(this, m_pLibrary->trackCollectionManager())); + pCrateTableModel->selectCrate(crateId); + pCrateTableModel->select(); + pCrateTableModel->addTracks(QModelIndex(), locations); + } } void CrateFeature::slotCreateImportCrate() { From 2bc3884eb9db40608b96992af86324206ab3c42e Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 01:02:44 +0100 Subject: [PATCH 30/44] PlaylistFeature: early return for inedaquate playlist type --- src/library/trackset/playlistfeature.cpp | 48 ++++++++++++------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index 647fbef6d69..460eaa99525 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -276,32 +276,34 @@ void PlaylistFeature::decorateChild(TreeItem* item, int playlistId) { void PlaylistFeature::slotPlaylistTableChanged(int playlistId) { //qDebug() << "slotPlaylistTableChanged() playlistId:" << playlistId; enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); - if (type == PlaylistDAO::PLHT_NOT_HIDDEN || - type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist - // Store current selection - int selectedPlaylistId = kInvalidPlaylistId; - if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { - if (playlistId == playlistIdFromIndex(m_lastClickedIndex) && - type == PlaylistDAO::PLHT_UNKNOWN) { - // if the selected playlist was deleted, find a sibling to select - selectedPlaylistId = getSiblingPlaylistIdOf(m_lastClickedIndex); - } else { - // just restore the current selection - selectedPlaylistId = playlistIdFromIndex(m_lastClickedIndex); - } - } + if (type != PlaylistDAO::PLHT_NOT_HIDDEN && // not a regular playlist + type != PlaylistDAO::PLHT_UNKNOWN) { // not a deleted playlist + return; + } - clearChildModel(); - QModelIndex newIndex = constructChildModel(selectedPlaylistId); - if (newIndex.isValid()) { - // If a child index was selected and we got a new valid index select that. - // Else (root item was selected or for some reason no index could be created) - // there's nothing to do: either no child was selected earlier, or the root - // was selected and will remain selected after the child model was rebuilt. - activateChild(newIndex); - emit featureSelect(this, newIndex); + // Store current selection + int selectedPlaylistId = kInvalidPlaylistId; + if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { + if (playlistId == playlistIdFromIndex(m_lastClickedIndex) && + type == PlaylistDAO::PLHT_UNKNOWN) { + // if the selected playlist was deleted, find a sibling to select + selectedPlaylistId = getSiblingPlaylistIdOf(m_lastClickedIndex); + } else { + // just restore the current selection + selectedPlaylistId = playlistIdFromIndex(m_lastClickedIndex); } } + + clearChildModel(); + QModelIndex newIndex = constructChildModel(selectedPlaylistId); + if (newIndex.isValid()) { + // If a child index was selected and we got a new valid index select that. + // Else (root item was selected or for some reason no index could be created) + // there's nothing to do: either no child was selected earlier, or the root + // was selected and will remain selected after the child model was rebuilt. + activateChild(newIndex); + emit featureSelect(this, newIndex); + } } void PlaylistFeature::slotPlaylistContentOrLockChanged(const QSet& playlistIds) { From 4a2be9b3f26b66321b9b55d8cebd1126d4592f0b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 01:03:39 +0100 Subject: [PATCH 31/44] playlist features: fix debug string --- src/library/trackset/playlistfeature.cpp | 2 +- src/library/trackset/setlogfeature.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index 460eaa99525..ec39a748c91 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -320,7 +320,7 @@ void PlaylistFeature::slotPlaylistContentOrLockChanged(const QSet& playlist void PlaylistFeature::slotPlaylistTableRenamed( int playlistId, const QString& newName) { Q_UNUSED(newName); - //qDebug() << "slotPlaylistTableChanged() playlistId:" << playlistId; + // qDebug() << "slotPlaylistTableRenamed() playlistId:" << playlistId; if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { updateChildModel(QSet{playlistId}); } diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 286e7f1111e..e2e7d634bb3 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -667,7 +667,7 @@ void SetlogFeature::activate() { } void SetlogFeature::activatePlaylist(int playlistId) { - //qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId; + // qDebug() << "SetlogFeature::activatePlaylist()" << playlistId; if (playlistId == kInvalidPlaylistId) { return; } From 150a116dea68cdb333b7f57edb762cf9ba779823 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 10:18:30 +0100 Subject: [PATCH 32/44] PlaylistDAO: make delete functions return bool --- src/library/dao/playlistdao.cpp | 14 +++++++------- src/library/dao/playlistdao.h | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index df80980f471..458583827fd 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -227,9 +227,9 @@ void PlaylistDAO::deletePlaylist(const int playlistId) { } } -int PlaylistDAO::deletePlaylists(const QStringList& idStringList, PlaylistDAO::HiddenType type) { +bool PlaylistDAO::deletePlaylists(const QStringList& idStringList, PlaylistDAO::HiddenType type) { if (idStringList.isEmpty()) { - return 0; + return false; } QString idString = idStringList.join(","); @@ -240,21 +240,21 @@ int PlaylistDAO::deletePlaylists(const QStringList& idStringList, PlaylistDAO::H QString("DELETE FROM PlaylistTracks WHERE playlist_id IN (%1)") .arg(idString)); if (!deleteTracks.execPrepared()) { - return -1; + return false; } // delete the playlists auto deletePlaylists = FwdSqlQuery(m_database, QString("DELETE FROM Playlists WHERE id IN (%1)").arg(idString)); if (!deletePlaylists.execPrepared()) { - return -1; + return false; } emit deleted(kInvalidPlaylistId); - return idStringList.length(); + return true; } -int PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( +bool PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( PlaylistDAO::HiddenType type, int minNumberOfTracks) { VERIFY_OR_DEBUG_ASSERT(minNumberOfTracks > 0) { return 0; // nothing to do, probably unintended invocation @@ -270,7 +270,7 @@ int PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( query.bindValue(":length", minNumberOfTracks); if (!query.exec()) { LOG_FAILED_QUERY(query); - return -1; + return false; } QStringList idStringList; diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index c08786ac993..ebf265eadf6 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -58,12 +58,12 @@ class PlaylistDAO : public QObject, public virtual DAO { // Delete a playlist void deletePlaylist(const int playlistId); // Delete a set of playlists. for now "type" is just for the logger - int deletePlaylists(const QStringList& idStringList, + bool deletePlaylists(const QStringList& idStringList, const HiddenType type = PLHT_NOT_HIDDEN); /// Delete Playlists with fewer entries then "minNumberOfTracks" /// Needs to be called inside a transaction. - /// @return number of deleted playlists, -1 on error - int deleteAllUnlockedPlaylistsWithFewerTracks(const PlaylistDAO::HiddenType type, + /// @return true on success, flase on error + bool deleteAllUnlockedPlaylistsWithFewerTracks(const PlaylistDAO::HiddenType type, int minNumberOfTracks); // Rename a playlist void renamePlaylist(const int playlistId, const QString& newName); From 59be421e8d30a9425ca9b89cf141f8a6d2669eca Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 10:20:42 +0100 Subject: [PATCH 33/44] PlaylistDAO: remove type input from deletePlaylists() --- src/library/dao/playlistdao.cpp | 6 +++--- src/library/dao/playlistdao.h | 5 ++--- src/library/trackset/setlogfeature.cpp | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 458583827fd..1428510b2d8 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -227,13 +227,13 @@ void PlaylistDAO::deletePlaylist(const int playlistId) { } } -bool PlaylistDAO::deletePlaylists(const QStringList& idStringList, PlaylistDAO::HiddenType type) { +bool PlaylistDAO::deletePlaylists(const QStringList& idStringList) { if (idStringList.isEmpty()) { return false; } QString idString = idStringList.join(","); - qInfo() << "Deleting" << idStringList.size() << "playlists of type" << type; + qInfo() << "Deleting" << idStringList.size() << "playlists"; // delete tracks assigned to these playlists auto deleteTracks = FwdSqlQuery(m_database, @@ -280,7 +280,7 @@ bool PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( qInfo() << "Prepared deletion of" << idStringList.size() << "playlists of type" << type << "that contain fewer than" << minNumberOfTracks << "tracks"; - return deletePlaylists(idStringList, type); + return deletePlaylists(idStringList); } void PlaylistDAO::renamePlaylist(const int playlistId, const QString& newName) { diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index ebf265eadf6..38d1cdff7e9 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -57,9 +57,8 @@ class PlaylistDAO : public QObject, public virtual DAO { int createUniquePlaylist(QString* pName, const HiddenType type = PLHT_NOT_HIDDEN); // Delete a playlist void deletePlaylist(const int playlistId); - // Delete a set of playlists. for now "type" is just for the logger - bool deletePlaylists(const QStringList& idStringList, - const HiddenType type = PLHT_NOT_HIDDEN); + // Delete a set of playlists. + bool deletePlaylists(const QStringList& idStringList); /// Delete Playlists with fewer entries then "minNumberOfTracks" /// Needs to be called inside a transaction. /// @return true on success, flase on error diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index e2e7d634bb3..ad42cd62165 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -500,7 +500,7 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { ids.append(pChild->getData().toString()); } } - m_playlistDao.deletePlaylists(ids, PlaylistDAO::PLHT_SET_LOG); + m_playlistDao.deletePlaylists(ids); } void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { From 6e3cc8e56b08d827c7642f59f81003a64f7121e7 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 10:27:32 +0100 Subject: [PATCH 34/44] CrateFeature: explicit cast to int ('possible loss of data' warning) --- src/library/trackset/crate/cratefeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 96c97c49dab..1c3dbd39a5f 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -565,7 +565,7 @@ QModelIndex CrateFeature::rebuildChildModel(CrateId selectedCrateId) { modelRows.push_back(newTreeItemForCrateSummary(crateSummary)); if (selectedCrateId == crateSummary.getId()) { // save index for selection - selectedRow = modelRows.size() - 1; + selectedRow = static_cast(modelRows.size()) - 1; } } From f89e17019b6463fafc6064b81f73bda72c604cb7 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 10:44:38 +0100 Subject: [PATCH 35/44] SetlogFeature: add translator comments, re/move debug messages --- src/library/trackset/setlogfeature.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index ad42cd62165..4d3f168a2b6 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -434,7 +434,6 @@ void SetlogFeature::lockOrUnlockAllChildPlaylists(bool lock) { } else { qWarning() << "unlock all child playlists of" << m_lastRightClickedIndex.data().toString(); } - // Ask for confirmation: "Delete all playlists in this group, including locked?" TreeItem* item = static_cast(m_lastRightClickedIndex.internalPointer()); if (!item) { return; @@ -459,8 +458,6 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { if (!m_lastRightClickedIndex.isValid()) { return; } - qWarning() << "slotDeleteAllChildPlaylists of" << m_lastRightClickedIndex.data().toString(); - // Ask for confirmation: "Delete all playlists in this group, including locked?" TreeItem* item = static_cast(m_lastRightClickedIndex.internalPointer()); if (!item) { return; @@ -469,12 +466,16 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { if (yearChildren.isEmpty()) { return; } + QString year = m_lastRightClickedIndex.data().toString(); QMessageBox::StandardButton btn = QMessageBox::question(nullptr, tr("Confirm Deletion"), + //: %1 is the year + //: + are used to make the text in between bold in the popup + //:
is a linebreak tr("Do you really want to delete all playlist in %1?

" "Note: this also includes locked playlists!") - .arg(m_lastRightClickedIndex.data().toString()), + .arg(year), QMessageBox::Yes | QMessageBox::No, QMessageBox::No); if (btn == QMessageBox::No) { @@ -483,9 +484,12 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { // Double-check, this is a weighty decision btn = QMessageBox::warning(nullptr, tr("Confirm Deletion"), + //: %1 is the year + //: + are used to make the text in between bold in the popup + //:
is a linebreak tr("Deleting all playlist in %1 including locked playlists.

" "Are you sure?") - .arg(m_lastRightClickedIndex.data().toString()), + .arg(year), QMessageBox::Yes | QMessageBox::No, QMessageBox::No); if (btn == QMessageBox::No) { @@ -500,6 +504,7 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { ids.append(pChild->getData().toString()); } } + qDebug() << "Setlog: deleting all playlists of" << year; m_playlistDao.deletePlaylists(ids); } From 7b84778088d3fc111df72c3869ec7f8fc94af6c5 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 23 Mar 2023 12:08:44 +0100 Subject: [PATCH 36/44] History: bulk-delete only unlocked playlists --- src/library/dao/playlistdao.cpp | 25 ++++++++++++++++++++++++- src/library/dao/playlistdao.h | 1 + src/library/trackset/setlogfeature.cpp | 17 ++++++++--------- src/library/trackset/setlogfeature.h | 2 +- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 1428510b2d8..89f73df154e 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -231,7 +231,7 @@ bool PlaylistDAO::deletePlaylists(const QStringList& idStringList) { if (idStringList.isEmpty()) { return false; } - QString idString = idStringList.join(","); + const QString idString = idStringList.join(","); qInfo() << "Deleting" << idStringList.size() << "playlists"; @@ -254,6 +254,29 @@ bool PlaylistDAO::deletePlaylists(const QStringList& idStringList) { return true; } +bool PlaylistDAO::deleteUnlockedPlaylists(QStringList& idStringList) { + if (idStringList.isEmpty()) { + return false; + } + + idStringList.removeDuplicates(); + const QString idString = idStringList.join(","); + QSqlQuery query(m_database); + // select locked playlists + query.prepare(QStringLiteral( + "SELECT id FROM Playlists WHERE id IN (%1) AND locked = 1") + .arg(idString)); + if (!query.exec()) { + LOG_FAILED_QUERY(query); + return false; + } + // remove locked playlists from list + while (query.next()) { + idStringList.removeOne(query.value(0).toString()); + } + return deletePlaylists(idStringList); +} + bool PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( PlaylistDAO::HiddenType type, int minNumberOfTracks) { VERIFY_OR_DEBUG_ASSERT(minNumberOfTracks > 0) { diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index 38d1cdff7e9..8b08abeaa74 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -59,6 +59,7 @@ class PlaylistDAO : public QObject, public virtual DAO { void deletePlaylist(const int playlistId); // Delete a set of playlists. bool deletePlaylists(const QStringList& idStringList); + bool deleteUnlockedPlaylists(QStringList& idStringList); /// Delete Playlists with fewer entries then "minNumberOfTracks" /// Needs to be called inside a transaction. /// @return true on success, flase on error diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 4d3f168a2b6..c111226099b 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -84,11 +84,11 @@ SetlogFeature::SetlogFeature( this, &SetlogFeature::slotUnlockAllChildPlaylists); - m_pDeleteAllChildPlaylists = new QAction(tr("Delete all child playlists"), this); + m_pDeleteAllChildPlaylists = new QAction(tr("Delete all unlocked child playlists"), this); connect(m_pDeleteAllChildPlaylists, &QAction::triggered, this, - &SetlogFeature::slotDeleteAllChildPlaylists); + &SetlogFeature::slotDeleteAllUnlockedChildPlaylists); // initialized in a new generic slot(get new history playlist purpose) slotGetNewPlaylist(); @@ -136,7 +136,7 @@ void SetlogFeature::slotDeletePlaylist() { return; } else if (playlistId == m_placeholderId) { // this is a YEAR node - slotDeleteAllChildPlaylists(); + slotDeleteAllUnlockedChildPlaylists(); } else { // regular setlog, call the base implementation BasePlaylistFeature::slotDeletePlaylist(); @@ -454,7 +454,7 @@ void SetlogFeature::lockOrUnlockAllChildPlaylists(bool lock) { m_playlistDao.setPlaylistsLocked(ids, lock); } -void SetlogFeature::slotDeleteAllChildPlaylists() { +void SetlogFeature::slotDeleteAllUnlockedChildPlaylists() { if (!m_lastRightClickedIndex.isValid()) { return; } @@ -473,8 +473,7 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { //: %1 is the year //: + are used to make the text in between bold in the popup //:
is a linebreak - tr("Do you really want to delete all playlist in %1?

" - "Note: this also includes locked playlists!") + tr("Do you really want to delete all unlocked playlist in %1?

") .arg(year), QMessageBox::Yes | QMessageBox::No, QMessageBox::No); @@ -487,7 +486,7 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { //: %1 is the year //: + are used to make the text in between bold in the popup //:
is a linebreak - tr("Deleting all playlist in %1 including locked playlists.

" + tr("Deleting all unlokced playlist in %1.

" "Are you sure?") .arg(year), QMessageBox::Yes | QMessageBox::No, @@ -504,8 +503,8 @@ void SetlogFeature::slotDeleteAllChildPlaylists() { ids.append(pChild->getData().toString()); } } - qDebug() << "Setlog: deleting all playlists of" << year; - m_playlistDao.deletePlaylists(ids); + qDebug() << "History: deleting all unlocked playlists of" << year; + m_playlistDao.deleteUnlockedPlaylists(ids); } void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 1ed07e4b41a..9d6c622920b 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -43,7 +43,7 @@ class SetlogFeature : public BasePlaylistFeature { void slotPlaylistTableChanged(int playlistId) override; void slotPlaylistContentOrLockChanged(const QSet& playlistIds) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; - void slotDeleteAllChildPlaylists(); + void slotDeleteAllUnlockedChildPlaylists(); private: void deleteAllUnlockedPlaylistsWithFewerTracks(); From be08c7d8d9eb1c198cba34332cdc663d785f159b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 23 Apr 2023 22:45:34 +0200 Subject: [PATCH 37/44] PlaylistDAO: std::move QStringList to deleteUnlockedPlaylists() --- src/library/dao/playlistdao.cpp | 2 +- src/library/dao/playlistdao.h | 2 +- src/library/trackset/setlogfeature.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 89f73df154e..806fa32f9f5 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -254,7 +254,7 @@ bool PlaylistDAO::deletePlaylists(const QStringList& idStringList) { return true; } -bool PlaylistDAO::deleteUnlockedPlaylists(QStringList& idStringList) { +bool PlaylistDAO::deleteUnlockedPlaylists(QStringList&& idStringList) { if (idStringList.isEmpty()) { return false; } diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index 8b08abeaa74..b32048a79ea 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -59,7 +59,7 @@ class PlaylistDAO : public QObject, public virtual DAO { void deletePlaylist(const int playlistId); // Delete a set of playlists. bool deletePlaylists(const QStringList& idStringList); - bool deleteUnlockedPlaylists(QStringList& idStringList); + bool deleteUnlockedPlaylists(QStringList&& idStringList); /// Delete Playlists with fewer entries then "minNumberOfTracks" /// Needs to be called inside a transaction. /// @return true on success, flase on error diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index c111226099b..7f7af4cbac2 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -504,7 +504,7 @@ void SetlogFeature::slotDeleteAllUnlockedChildPlaylists() { } } qDebug() << "History: deleting all unlocked playlists of" << year; - m_playlistDao.deleteUnlockedPlaylists(ids); + m_playlistDao.deleteUnlockedPlaylists(std::move(ids)); } void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { From cdc0bf6892b1b4eb3459cd910756db5baaac4f75 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 23 Apr 2023 22:49:13 +0200 Subject: [PATCH 38/44] PlaylistDAO: correct return value of deleteAllUnlockedPlaylistsWithFewerTracks --- src/library/dao/playlistdao.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 806fa32f9f5..5d69ff19cd5 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -280,7 +280,7 @@ bool PlaylistDAO::deleteUnlockedPlaylists(QStringList&& idStringList) { bool PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks( PlaylistDAO::HiddenType type, int minNumberOfTracks) { VERIFY_OR_DEBUG_ASSERT(minNumberOfTracks > 0) { - return 0; // nothing to do, probably unintended invocation + return false; // nothing to do, probably unintended invocation } QSqlQuery query(m_database); From 223c610d7f7d11adbf81dab6ee9390805693107e Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 23 Apr 2023 22:58:58 +0200 Subject: [PATCH 39/44] History feature: improve 'Delete unlocked year playlists' dialogs --- src/library/trackset/setlogfeature.cpp | 34 ++++++++++++++------------ 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 7f7af4cbac2..54f1c4d5a9f 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -473,36 +473,38 @@ void SetlogFeature::slotDeleteAllUnlockedChildPlaylists() { //: %1 is the year //: + are used to make the text in between bold in the popup //:
is a linebreak - tr("Do you really want to delete all unlocked playlist in %1?

") + tr("Do you really want to delete all unlocked playlist from %1?

") .arg(year), QMessageBox::Yes | QMessageBox::No, QMessageBox::No); - if (btn == QMessageBox::No) { - return; - } - // Double-check, this is a weighty decision - btn = QMessageBox::warning(nullptr, - tr("Confirm Deletion"), - //: %1 is the year - //: + are used to make the text in between bold in the popup - //:
is a linebreak - tr("Deleting all unlokced playlist in %1.

" - "Are you sure?") - .arg(year), - QMessageBox::Yes | QMessageBox::No, - QMessageBox::No); - if (btn == QMessageBox::No) { + if (btn != QMessageBox::Yes) { return; } QStringList ids; + int count = 0; for (const auto& pChild : yearChildren) { bool ok = false; int childId = pChild->getData().toInt(&ok); if (ok && childId != kInvalidPlaylistId) { ids.append(pChild->getData().toString()); + count++; } } + // Double-check, this is a weighty decision + btn = QMessageBox::warning(nullptr, + tr("Confirm Deletion"), + //: %1 is the number of playlists to be deleted + //: %2 is the year + //: + are used to make the text in between bold in the popup + //:
is a linebreak + tr("Deleting %1 playlists from %2.

") + .arg(QString::number(count), year), + QMessageBox::Ok | QMessageBox::Cancel, + QMessageBox::Cancel); + if (btn != QMessageBox::Ok) { + return; + } qDebug() << "History: deleting all unlocked playlists of" << year; m_playlistDao.deleteUnlockedPlaylists(std::move(ids)); } From 850ecddffb71f54aa973732c63e69fdf906891c7 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Mon, 24 Apr 2023 10:04:36 +0200 Subject: [PATCH 40/44] PlaylistDAO: remove unnecessary/confusing comments --- src/library/dao/playlistdao.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 5d69ff19cd5..d9adb4aab42 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -832,7 +832,6 @@ int PlaylistDAO::getPreviousPlaylist(const int currentPlaylistId, HiddenType hid return kInvalidPlaylistId; } - // Get the id of the highest playlist if (query.next()) { return query.value(query.record().indexOf("id")).toInt(); } @@ -852,7 +851,6 @@ int PlaylistDAO::getNextPlaylist(const int currentPlaylistId, HiddenType hidden) return kInvalidPlaylistId; } - // Get the id of the lowest playlist if (query.next()) { return query.value(query.record().indexOf("id")).toInt(); } From f01647fb0ddf3fa500ceaaf962252f10092a8a24 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 00:15:10 +0200 Subject: [PATCH 41/44] BasePlaylistFeature: don't wipe track count & duration when selecting --- src/library/trackset/baseplaylistfeature.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 575d8d98e7d..ec0a44a2690 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -735,7 +735,7 @@ void BasePlaylistFeature::updateChildModel(const QSet& playlistIds) { for (TreeItem* pChild : pTreeItem->children()) { id = pChild->getData().toInt(&ok); if (ok && id != kInvalidPlaylistId && playlistIds.contains(id)) { - label = m_playlistDao.getPlaylistName(id); + label = fetchPlaylistLabel(id); pChild->setLabel(label); decorateChild(pChild, id); } @@ -743,7 +743,7 @@ void BasePlaylistFeature::updateChildModel(const QSet& playlistIds) { } else { id = pTreeItem->getData().toInt(&ok); if (ok && id != kInvalidPlaylistId && playlistIds.contains(id)) { - label = m_playlistDao.getPlaylistName(id); + label = fetchPlaylistLabel(id); pTreeItem->setLabel(label); decorateChild(pTreeItem, id); } From 89b3aff750a8462d56db3badc1fdc520c3e3acc0 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 00:30:15 +0200 Subject: [PATCH 42/44] Playlists: restore resorting sidebar items after playlist rename --- src/library/trackset/baseplaylistfeature.cpp | 2 ++ src/library/trackset/playlistfeature.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index ec0a44a2690..46da3f86197 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -164,6 +164,8 @@ void BasePlaylistFeature::connectPlaylistDAO() { connect(&m_playlistDao, &PlaylistDAO::renamed, this, + // In "History") just the item is renamed, while in "Playlists" the + // entire sidebar model is rebuilt to resort items by name &BasePlaylistFeature::slotPlaylistTableRenamed); } diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index ec39a748c91..435442a31f4 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -322,7 +322,7 @@ void PlaylistFeature::slotPlaylistTableRenamed( Q_UNUSED(newName); // qDebug() << "slotPlaylistTableRenamed() playlistId:" << playlistId; if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { - updateChildModel(QSet{playlistId}); + slotPlaylistTableChanged(playlistId); } } From 4cdc0776a74581e71ef59039325f74ab5ae4e63d Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 01:53:16 +0200 Subject: [PATCH 43/44] Crates: restore resorting sidebar items after playlist rename --- src/library/trackset/crate/cratefeature.cpp | 7 +------ src/library/trackset/crate/cratefeature.h | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 1c3dbd39a5f..27a901bef84 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -171,7 +171,7 @@ void CrateFeature::connectTrackCollection() { connect(m_pTrackCollection, // renamed, un/locked, toggled AutoDJ source &TrackCollection::crateUpdated, this, - &CrateFeature::slotUpdateCrateLabel); + &CrateFeature::slotCrateTableChanged); connect(m_pTrackCollection, &TrackCollection::crateDeleted, this, @@ -886,11 +886,6 @@ void CrateFeature::slotCrateContentChanged(CrateId crateId) { updateChildModel(updatedCrateIds); } -void CrateFeature::slotUpdateCrateLabel(CrateId updatedCrateId) { - QSet updatedCrateIds = {updatedCrateId}; - updateChildModel(updatedCrateIds); -} - void CrateFeature::slotUpdateCrateLabels(const QSet& updatedCrateIds) { updateChildModel(updatedCrateIds); } diff --git a/src/library/trackset/crate/cratefeature.h b/src/library/trackset/crate/cratefeature.h index 5f392cd6091..b7e746d201b 100644 --- a/src/library/trackset/crate/cratefeature.h +++ b/src/library/trackset/crate/cratefeature.h @@ -74,7 +74,6 @@ class CrateFeature : public BaseTrackSetFeature { void htmlLinkClicked(const QUrl& link); void slotTrackSelected(TrackId trackId); void slotResetSelectedTrack(); - void slotUpdateCrateLabel(CrateId updatedCrateId); void slotUpdateCrateLabels(const QSet& updatedCrateIds); private: From 877f1127f6795b71527c495e77ea9f9572a502af Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 01:54:29 +0200 Subject: [PATCH 44/44] CrateFeature::activateCrate: just return false if crate doesn't exist --- src/library/trackset/crate/cratefeature.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/library/trackset/crate/cratefeature.cpp b/src/library/trackset/crate/cratefeature.cpp index 27a901bef84..f4f46fba506 100644 --- a/src/library/trackset/crate/cratefeature.cpp +++ b/src/library/trackset/crate/cratefeature.cpp @@ -320,6 +320,11 @@ bool CrateFeature::activateCrate(CrateId crateId) { VERIFY_OR_DEBUG_ASSERT(crateId.isValid()) { return false; } + if (!m_pTrackCollection->crates().readCrateSummaryById(crateId)) { + // this may happen if called by slotCrateTableChanged() + // and the crate has just been deleted + return false; + } QModelIndex index = indexFromCrateId(crateId); VERIFY_OR_DEBUG_ASSERT(index.isValid()) { return false;