From fd3451e20ed46ee3f933e54aab4bc59a96c4be57 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 23 Mar 2025 13:46:19 +0100 Subject: [PATCH 01/13] SidebarModel/TreeItemModel: cleanup, early return, opti, refine comments, p prefix for pointers --- src/library/sidebarmodel.cpp | 177 +++++++++++++++++----------------- src/library/sidebarmodel.h | 3 +- src/library/treeitem.h | 5 +- src/library/treeitemmodel.cpp | 38 ++++---- 4 files changed, 109 insertions(+), 114 deletions(-) diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 189e4270029b..e538dc9e405c 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -25,7 +25,7 @@ constexpr bool kDebug = false; SidebarModel::SidebarModel( QObject* parent) : QAbstractItemModel(parent), - m_iDefaultSelectedIndex(0), + m_iDefaultSelectedIndex(0), // Tracks / MixxxLibraryFeature m_pressedUntilClickedTimer(new QTimer(this)) { m_pressedUntilClickedTimer->setSingleShot(true); connect(m_pressedUntilClickedTimer, @@ -110,10 +110,9 @@ QModelIndex SidebarModel::index(int row, int column, } if (parent.isValid()) { - /* If we have selected the root of a library feature at position 'row' - * its internal pointer is the current sidebar object model - * we return its associated childmodel - */ + // If we have selected the root of a library feature at position 'row', + // its internal pointer is a pointer to the current sidebarmodel object. + // We return its associated childmodel index. if (parent.internalPointer() == this) { const QAbstractItemModel* childModel = m_sFeatures[parent.row()]->sidebarModel(); QModelIndex childIndex = childModel->index(row, column); @@ -124,7 +123,7 @@ QModelIndex SidebarModel::index(int row, int column, return QModelIndex(); } } else { - // We have selected an item within the childmodel + // We have selected an item within the childmodel. // This item has always an internal pointer of (sub)type TreeItem TreeItem* pTreeItem = static_cast(parent.internalPointer()); if (row < pTreeItem->childRows()) { @@ -146,14 +145,11 @@ QModelIndex SidebarModel::getFeatureRootIndex(LibraryFeature* pFeature) { if constexpr (kDebug) { 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; - } + int featureRow = m_sFeatures.indexOf(pFeature); + VERIFY_OR_DEBUG_ASSERT(featureRow != -1) { + return {}; } - return ind; + return index(featureRow, 0); } void SidebarModel::clear(const QModelIndex& index) { @@ -178,42 +174,43 @@ QModelIndex SidebarModel::parent(const QModelIndex& index) const { if constexpr (kDebug) { qDebug() << "SidebarModel::parent index=" << index; } - if (index.isValid()) { - // If we have selected the root of a library feature - // its internal pointer is the current sidebar object model - // A root library feature has no parent and thus we return - // an invalid QModelIndex - if (index.internalPointer() == this) { - return QModelIndex(); - } else { - TreeItem* pTreeItem = static_cast(index.internalPointer()); - if (pTreeItem == nullptr) { - return QModelIndex(); - } - TreeItem* pTreeItemParent = pTreeItem->parent(); - // if we have selected an item at the first level of a childnode - - if (pTreeItemParent) { - if (pTreeItemParent->isRoot()) { - LibraryFeature* pFeature = pTreeItem->feature(); - for (int i = 0; i < m_sFeatures.size(); ++i) { - if (pFeature == m_sFeatures[i]) { - // create a ModelIndex for parent 'this' having a - // library feature at position 'i' - // `this` is const, but the function expects a - // non-const pointer. - // TODO: Check if we can get rid of this const cast - // somehow. - return createIndex(i, 0, const_cast(this)); - } - } - } - // if we have selected an item at some deeper level of a childnode - return createIndex(pTreeItemParent->parentRow(), 0, pTreeItemParent); - } + if (!index.isValid()) { + return {}; + } + + // If we have selected the root of a library feature, + // its internal pointer is the current sidebar object model. + // A root library feature has no parent and thus we return + // an invalid QModelIndex. + if (index.internalPointer() == this) { + return {}; + } + + TreeItem* pTreeItem = static_cast(index.internalPointer()); + VERIFY_OR_DEBUG_ASSERT(pTreeItem != nullptr) { + return {}; + } + TreeItem* pParentItem = pTreeItem->parent(); + VERIFY_OR_DEBUG_ASSERT(pParentItem != nullptr) { + return {}; + } + if (pParentItem->isRoot()) { + // If we have selected an item at the first level of a childnode, + // Create a ModelIndex for parent 'this' having a + // library feature at position 'i'. + // `this` is const, but the function expects a + // non-const pointer. + // TODO: Check if we can get rid of this const cast + // somehow. + LibraryFeature* pFeature = pTreeItem->feature(); + int featureRow = m_sFeatures.indexOf(pFeature); + VERIFY_OR_DEBUG_ASSERT(featureRow != -1) { + return {}; } + return createIndex(featureRow, 0, const_cast(this)); } - return QModelIndex(); + // If we have selected an item at some deeper level of a childnode + return createIndex(pParentItem->parentRow(), 0, pParentItem); } int SidebarModel::rowCount(const QModelIndex& parent) const { @@ -274,7 +271,7 @@ QVariant SidebarModel::data(const QModelIndex& index, int role) const { } if (index.internalPointer() == this) { - //If it points to SidebarModel + // If it points to SidebarModel this is a root item. switch (role) { case Qt::DisplayRole: return m_sFeatures[index.row()]->title(); @@ -370,30 +367,35 @@ 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()) { - if (index.internalPointer() == this) { - return; - } else { - TreeItem* pTreeItem = static_cast(index.internalPointer()); - if (pTreeItem) { - LibraryFeature* pFeature = pTreeItem->feature(); - pFeature->onLazyChildExpandation(index); - } + if (!index.isValid()) { + return; + } + if (index.internalPointer() == this) { + // Index is a root index and QTreeView already did expand it, so ther's + // nothing to do for us. + return; + } else { + TreeItem* pTreeItem = static_cast(index.internalPointer()); + VERIFY_OR_DEBUG_ASSERT(pTreeItem) { + return; } + LibraryFeature* pFeature = pTreeItem->feature(); + pFeature->onLazyChildExpandation(index); } } void SidebarModel::rightClicked(const QPoint& globalPos, const QModelIndex& index) { stopPressedUntilClickedTimer(); - if (index.isValid()) { - if (index.internalPointer() == this) { - m_sFeatures[index.row()]->onRightClick(globalPos); - } else { - TreeItem* pTreeItem = static_cast(index.internalPointer()); - if (pTreeItem) { - LibraryFeature* pFeature = pTreeItem->feature(); - pFeature->onRightClickChild(globalPos, index); - } + if (!index.isValid()) { + return; + } + if (index.internalPointer() == this) { + m_sFeatures[index.row()]->onRightClick(globalPos); + } else { + TreeItem* pTreeItem = static_cast(index.internalPointer()); + if (pTreeItem) { + LibraryFeature* pFeature = pTreeItem->feature(); + pFeature->onRightClickChild(globalPos, index); } } } @@ -489,19 +491,16 @@ bool SidebarModel::dragMoveAccept(const QModelIndex& index, const QList& u /// 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 - * handled by the slot functions. - - * For child models, this always the child models itself - */ - - const QAbstractItemModel* model = qobject_cast(sender()); - VERIFY_OR_DEBUG_ASSERT(model != nullptr) { + // These method is called from the slot functions below. + // QObject::sender() return the object which emitted the signal + // handled by the slot functions. + // For child models, this always the child models itself + const QAbstractItemModel* pModel = qobject_cast(sender()); + VERIFY_OR_DEBUG_ASSERT(pModel != nullptr) { return QModelIndex(); } - return translateIndex(index, model); + return translateIndex(index, pModel); } QModelIndex SidebarModel::translateIndex( @@ -522,22 +521,21 @@ QModelIndex SidebarModel::translateIndex( } void SidebarModel::slotDataChanged(const QModelIndex& topLeft, const QModelIndex& bottomRight) { - // qDebug() << "slotDataChanged topLeft:" << topLeft << "bottomRight:" << bottomRight; + // qDebug() << "slotDataChanged topLeft:" << topLeft; + // qDebug() << " bottomRight:" << bottomRight; QModelIndex topLeftTranslated = translateSourceIndex(topLeft); QModelIndex bottomRightTranslated = translateSourceIndex(bottomRight); emit dataChanged(topLeftTranslated, bottomRightTranslated); } void SidebarModel::slotRowsAboutToBeInserted(const QModelIndex& parent, int start, int end) { - //qDebug() << "slotRowsABoutToBeInserted" << parent << start << end; - + // qDebug() << "slotRowsABoutToBeInserted" << parent << start << end; QModelIndex newParent = translateSourceIndex(parent); beginInsertRows(newParent, start, end); } void SidebarModel::slotRowsAboutToBeRemoved(const QModelIndex& parent, int start, int end) { - //qDebug() << "slotRowsABoutToBeRemoved" << parent << start << end; - + // qDebug() << "slotRowsABoutToBeRemoved" << parent << start << end; QModelIndex newParent = translateSourceIndex(parent); beginRemoveRows(newParent, start, end); } @@ -555,24 +553,24 @@ void SidebarModel::slotRowsRemoved(const QModelIndex& parent, int start, int end Q_UNUSED(parent); Q_UNUSED(start); Q_UNUSED(end); - //qDebug() << "slotRowsRemoved" << parent << start << end; - //QModelIndex newParent = translateSourceIndex(parent); + // qDebug() << "slotRowsRemoved" << parent << start << end; + // QModelIndex newParent = translateSourceIndex(parent); endRemoveRows(); } void SidebarModel::slotModelAboutToBeReset() { + // qDebug() << "slotModelAboutToBeReset"; beginResetModel(); } void SidebarModel::slotModelReset() { + // qDebug() << "slotModelReset"; endResetModel(); } -/* - * Call this slot whenever the title of the feature has changed. - * See RhythmboxFeature for an example, in which the title becomes '(loading) Rhythmbox' - * If selectFeature is true, the feature is selected when the title change occurs. - */ +// Call this slot whenever the title of the feature has changed. +// See RhythmboxFeature for an example, in which the title becomes '(loading) Rhythmbox' +// If selectFeature is true, the feature is selected when the title change occurs. void SidebarModel::slotFeatureIsLoading(LibraryFeature* pFeature, bool selectFeature) { featureRenamed(pFeature); if (selectFeature) { @@ -580,9 +578,6 @@ void SidebarModel::slotFeatureIsLoading(LibraryFeature* pFeature, bool selectFea } } -/* Tobias: This slot is somewhat redundant but I decided - * to leave it for code readability reasons - */ void SidebarModel::slotFeatureLoadingFinished(LibraryFeature* pFeature) { featureRenamed(pFeature); slotFeatureSelect(pFeature); diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index 15c04a8acab5..d7294d8812de 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -66,10 +66,9 @@ class SidebarModel : public QAbstractItemModel { // void slotColumnsInserted(const QModelIndex& parent, int start, int end); // void slotColumnsRemoved(const QModelIndex& parent, int start, int end); void slotDataChanged(const QModelIndex& topLeft, const QModelIndex & bottomRight); - //void slotHeaderDataChanged(Qt::Orientation orientation, int first, int last); + // void slotHeaderDataChanged(Qt::Orientation orientation, int first, int last); // void slotLayoutAboutToBeChanged(); // void slotLayoutChanged(); - // void slotModelAboutToBeReset(); // void slotModelReset(); void slotRowsAboutToBeInserted(const QModelIndex& parent, int start, int end); void slotRowsAboutToBeRemoved(const QModelIndex& parent, int start, int end); diff --git a/src/library/treeitem.h b/src/library/treeitem.h index df80de26b26f..02d792cb23e7 100644 --- a/src/library/treeitem.h +++ b/src/library/treeitem.h @@ -135,8 +135,9 @@ class TreeItem final { void initFeatureRecursively(LibraryFeature* pFeature); // The library feature is inherited from the parent. - // For all child items this is just a shortcut to the - // library feature of the root item! + // For all child items this is done via + // insertChildren() -> initFeatureRecursively() and is just + // a shortcut to the library feature of the root item! LibraryFeature* m_pFeature; TreeItem* m_pParent; diff --git a/src/library/treeitemmodel.cpp b/src/library/treeitemmodel.cpp index 36e40096fdc6..8d1aa9d7b798 100644 --- a/src/library/treeitemmodel.cpp +++ b/src/library/treeitemmodel.cpp @@ -11,7 +11,7 @@ // 1. argument represents a name shown in the sidebar view later on // 2. argument represents the absolute path of this tree item // 3. argument is a library feature object. -// This is necessary because in sidebar.cpp we handle 'activateChid' events +// This is necessary because in wlibrarysidebar.cpp we handle 'activateChild' events // 4. the parent TreeItem object // The constructor does not add this TreeItem object to the parent's child list // @@ -45,16 +45,16 @@ QVariant TreeItemModel::data(const QModelIndex &index, int role) const { return QVariant(); } - TreeItem *item = static_cast(index.internalPointer()); + TreeItem* pItem = static_cast(index.internalPointer()); // We use Qt::UserRole to ask for the data. switch (role) { case Qt::DisplayRole: - return item->getLabel(); + return pItem->getLabel(); case kDataRole: - return item->getData(); + return pItem->getData(); case kBoldRole: - return item->isBold(); + return pItem->isBold(); default: return QVariant(); } @@ -63,7 +63,7 @@ QVariant TreeItemModel::data(const QModelIndex &index, int role) const { bool TreeItemModel::setData(const QModelIndex &a_rIndex, const QVariant &a_rValue, int a_iRole) { // Get the item referred to by this index. - TreeItem *pItem = static_cast(a_rIndex.internalPointer()); + TreeItem* pItem = static_cast(a_rIndex.internalPointer()); if (pItem == nullptr) { return false; } @@ -107,16 +107,16 @@ QModelIndex TreeItemModel::index(int row, int column, const QModelIndex &parent) return QModelIndex(); } - TreeItem *parentItem; + TreeItem* pParentItem; if (parent.isValid()) { - parentItem = static_cast(parent.internalPointer()); + pParentItem = static_cast(parent.internalPointer()); } else { - parentItem = getRootItem(); + pParentItem = getRootItem(); } - TreeItem *childItem = parentItem->child(row); - if (childItem) { - return createIndex(row, column, childItem); + TreeItem* pChildItem = pParentItem->child(row); + if (pChildItem) { + return createIndex(row, column, pChildItem); } else { return QModelIndex(); } @@ -127,14 +127,14 @@ QModelIndex TreeItemModel::parent(const QModelIndex& index) const { return QModelIndex(); } - TreeItem *childItem = static_cast(index.internalPointer()); - TreeItem *parentItem = childItem->parent(); - if (!parentItem) { + TreeItem* pChildItem = static_cast(index.internalPointer()); + TreeItem* pParentItem = pChildItem->parent(); + if (!pParentItem) { return QModelIndex(); - } else if (parentItem == getRootItem()) { + } else if (pParentItem == getRootItem()) { return createIndex(0, 0, getRootItem()); } else { - return createIndex(parentItem->parentRow(), 0, parentItem); + return createIndex(pParentItem->parentRow(), 0, pParentItem); } } @@ -190,10 +190,10 @@ bool TreeItemModel::removeRows(int position, int rows, const QModelIndex &parent if (rows == 0) { return true; } - TreeItem *parentItem = getItem(parent); + TreeItem* pParentItem = getItem(parent); beginRemoveRows(parent, position, position + rows - 1); - parentItem->removeChildren(position, rows); + pParentItem->removeChildren(position, rows); endRemoveRows(); return true; From dc3227dc16d032e18a6b2427f39a4d49867fd641 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 27 Mar 2025 11:16:10 +0100 Subject: [PATCH 02/13] SetlogFeature: optimize a bit, remove obsolete comments --- src/library/trackset/setlogfeature.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index a548c075e926..eff73a22d7f4 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -164,15 +164,12 @@ void SetlogFeature::onRightClick(const QPoint& globalPos) { Q_UNUSED(globalPos); m_lastRightClickedIndex = QModelIndex(); - // Create the right-click menu - // QMenu menu(NULL); - // menu.addAction(m_pCreatePlaylistAction); + // There is no action associated with the root item // TODO(DASCHUER) add something like disable logging - // menu.exec(globalPos); } void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex& index) { - //Save the model index so we can get it in the action slots... + // Save the model index so we can get it in the action slots... m_lastRightClickedIndex = index; int playlistId = playlistIdFromIndex(index); @@ -253,7 +250,6 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { " GROUP BY Playlists.id") .arg(m_countsDurationTableName, QString::number(PlaylistDAO::PLHT_SET_LOG)); - ; queryString.append( mixxx::DbConnection::collateLexicographically( " ORDER BY sort_name")); @@ -359,14 +355,11 @@ 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 !"; + // qDebug() << "slotGetNewPlaylist() successfully triggered !"; // create a new playlist for today - QString set_log_name_format; - QString set_log_name; - - set_log_name = QDate::currentDate().toString(Qt::ISODate); - set_log_name_format = set_log_name + " #%1"; + QString set_log_name = QDate::currentDate().toString(Qt::ISODate); + QString set_log_name_format = set_log_name + " #%1"; int i = 1; // calculate name of the todays setlog @@ -374,7 +367,7 @@ void SetlogFeature::slotGetNewPlaylist() { set_log_name = set_log_name_format.arg(++i); } - //qDebug() << "Creating session history playlist name:" << set_log_name; + // qDebug() << "Creating session history playlist name:" << set_log_name; m_currentPlaylistId = m_playlistDao.createPlaylist( set_log_name, PlaylistDAO::PLHT_SET_LOG); From c6cf78e65d4ab789599bac7f579c591a57407136 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 25 Mar 2025 11:15:20 +0100 Subject: [PATCH 03/13] Library sidebar: store model pointer as member --- src/widget/wlibrarysidebar.cpp | 77 +++++++++++++--------------------- src/widget/wlibrarysidebar.h | 4 ++ 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index ebd74d94d178..0a0ec278bdc7 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -13,6 +13,7 @@ WLibrarySidebar::WLibrarySidebar(QWidget* parent) : QTreeView(parent), WBaseWidget(this), + m_pSidebarModel(nullptr), m_hoverExpandDelay(mixxx::library::prefs::kSidebarHoverExpandDelayDefault), m_lastDragMoveAccepted(false) { qRegisterMetaType("FocusWidget"); @@ -31,6 +32,13 @@ WLibrarySidebar::WLibrarySidebar(QWidget* parent) header()->setHorizontalScrollMode(QAbstractItemView::ScrollPerPixel); } +void WLibrarySidebar::setModel(QAbstractItemModel* pModel) { + SidebarModel* pSidebarModel = qobject_cast(pModel); + DEBUG_ASSERT(pSidebarModel); + m_pSidebarModel = pSidebarModel; + QTreeView::setModel(pSidebarModel); +} + void WLibrarySidebar::contextMenuEvent(QContextMenuEvent* pEvent) { // if (pEvent->state() & Qt::RightButton) { //Dis shiz don werk on windowze QModelIndex clickedIndex = indexAt(pEvent->pos()); @@ -116,13 +124,7 @@ void WLibrarySidebar::dragMoveEvent(QDragMoveEvent* pEvent) { return; } - SidebarModel* pSidebarModel = qobject_cast(model()); - VERIFY_OR_DEBUG_ASSERT(pSidebarModel) { - m_lastDragMoveAccepted = false; - pEvent->ignore(); - return; - } - if (pSidebarModel->dragMoveAccept(index, urls)) { + if (m_pSidebarModel->dragMoveAccept(index, urls)) { m_lastDragMoveAccepted = true; pEvent->acceptProposedAction(); } else { @@ -166,11 +168,6 @@ void WLibrarySidebar::dropEvent(QDropEvent* pEvent) { // track table widget onto the sidebar. // Reset the selected items (if you had anything highlighted, it clears it) // this->selectionModel()->clear(); - SidebarModel* pSidebarModel = qobject_cast(model()); - VERIFY_OR_DEBUG_ASSERT(pSidebarModel) { - pEvent->ignore(); - return; - } #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) QPoint pos = pEvent->position().toPoint(); #else @@ -181,7 +178,7 @@ void WLibrarySidebar::dropEvent(QDropEvent* pEvent) { // pEvent->source() will return NULL if something is dropped from // a different application const QList urls = pEvent->mimeData()->urls(); - if (pSidebarModel->dropAccept(destIndex, urls, pEvent->source())) { + if (m_pSidebarModel->dropAccept(destIndex, urls, pEvent->source())) { pEvent->acceptProposedAction(); } else { pEvent->ignore(); @@ -226,31 +223,28 @@ void WLibrarySidebar::toggleSelectedItem() { } bool WLibrarySidebar::isLeafNodeSelected() { - QModelIndex index = selectedIndex(); - if (index.isValid()) { - if(!index.model()->hasChildren(index)) { - return true; - } - const SidebarModel* pSidebarModel = qobject_cast(index.model()); - if (pSidebarModel) { - return pSidebarModel->hasTrackTable(index); - } + const QModelIndex index = selectedIndex(); + if (!index.isValid()) { + return false; + } + + if (!index.model()->hasChildren(index)) { + return true; + } + const SidebarModel* pSidebarModel = qobject_cast(index.model()); + if (pSidebarModel) { + return pSidebarModel->hasTrackTable(index); } return false; } bool WLibrarySidebar::isChildIndexSelected(const QModelIndex& index) { // qDebug() << "WLibrarySidebar::isChildIndexSelected" << index; - QModelIndex selIndex = selectedIndex(); + const QModelIndex selIndex = selectedIndex(); if (!selIndex.isValid()) { return false; } - SidebarModel* pSidebarModel = qobject_cast(model()); - VERIFY_OR_DEBUG_ASSERT(pSidebarModel) { - // qDebug() << " >> model() is not SidebarModel"; - return false; - } - QModelIndex translated = pSidebarModel->translateChildIndex(index); + const QModelIndex translated = m_pSidebarModel->translateChildIndex(index); if (!translated.isValid()) { // qDebug() << " >> index can't be translated"; return false; @@ -260,15 +254,11 @@ bool WLibrarySidebar::isChildIndexSelected(const QModelIndex& index) { bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { // qDebug() << "WLibrarySidebar::isFeatureRootIndexSelected"; - QModelIndex selIndex = selectedIndex(); + const QModelIndex selIndex = selectedIndex(); if (!selIndex.isValid()) { return false; } - SidebarModel* pSidebarModel = qobject_cast(model()); - VERIFY_OR_DEBUG_ASSERT(pSidebarModel) { - return false; - } - const QModelIndex rootIndex = pSidebarModel->getFeatureRootIndex(pFeature); + const QModelIndex rootIndex = m_pSidebarModel->getFeatureRootIndex(pFeature); return rootIndex == selIndex; } @@ -277,11 +267,9 @@ bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { void WLibrarySidebar::keyPressEvent(QKeyEvent* pEvent) { // 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. - - SidebarModel* pSidebarModel = qobject_cast(model()); - QModelIndex selIndex = selectedIndex(); - if (pSidebarModel && selIndex.isValid() && pEvent->matches(QKeySequence::Paste)) { - pSidebarModel->paste(selIndex); + const QModelIndex selIndex = selectedIndex(); + if (selIndex.isValid() && pEvent->matches(QKeySequence::Paste)) { + m_pSidebarModel->paste(selIndex); return; } @@ -408,18 +396,13 @@ void WLibrarySidebar::selectIndex(const QModelIndex& index, bool scrollToIndex) /// Selects a child index from a feature and ensures visibility void WLibrarySidebar::selectChildIndex(const QModelIndex& index, bool selectItem) { - SidebarModel* pSidebarModel = qobject_cast(model()); - VERIFY_OR_DEBUG_ASSERT(pSidebarModel) { - qDebug() << "model() is not SidebarModel"; - return; - } - QModelIndex translated = pSidebarModel->translateChildIndex(index); + const QModelIndex translated = m_pSidebarModel->translateChildIndex(index); if (!translated.isValid()) { return; } if (selectItem) { - auto* pModel = new QItemSelectionModel(pSidebarModel); + auto* pModel = new QItemSelectionModel(m_pSidebarModel); pModel->select(translated, QItemSelectionModel::Select); if (selectionModel()) { selectionModel()->deleteLater(); diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index 55e0baaf6892..2553a62ef6b3 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -8,6 +8,7 @@ #include "widget/wbasewidget.h" class LibraryFeature; +class SidebarModel; class QPoint; class WLibrarySidebar : public QTreeView, public WBaseWidget { @@ -15,6 +16,8 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { public: explicit WLibrarySidebar(QWidget* parent = nullptr); + void setModel(QAbstractItemModel* pModel) override; + void contextMenuEvent(QContextMenuEvent* pEvent) override; void dragMoveEvent(QDragMoveEvent* pEvent) override; void dragEnterEvent(QDragEnterEvent* pEvent) override; @@ -53,6 +56,7 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void toggleDragHoverPropertyAndUpdateStyle(bool enabled); void resetHoverIndexAndDragMoveResult(); + SidebarModel* m_pSidebarModel; QBasicTimer m_expandTimer; int m_hoverExpandDelay; QModelIndex m_hoverIndex; From 663e88ebc80ec4bdf0e66abefa63f783b6c1ad39 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 28 Mar 2025 00:56:48 +0100 Subject: [PATCH 04/13] PlaylistDAO: also emit type after adding or deleting playlists ... On playlist addition/deletion playlist features now also receive the playlist type and can act on their own playlist types only. Previously, deleting a playlist triggered a child (tree) model rebuild in both PlaylistFeature and Setlogfeature. Both would invoke rowsDeleted() and rowsInserted() in SidebarModel almost simultaneously. Ruling this out is required to avoid concurrent access to the SidebarBookmark list in SidebarModel. Regular (thread) lock/block/wait mechanisms can not be used because caller and receiver are in the same thread. --- src/library/dao/playlistdao.cpp | 17 +++++++++---- src/library/dao/playlistdao.h | 10 +++++--- src/library/trackset/baseplaylistfeature.h | 14 +++++------ src/library/trackset/playlistfeature.cpp | 10 ++++---- src/library/trackset/playlistfeature.h | 2 +- src/library/trackset/setlogfeature.cpp | 28 ++++++++++++---------- src/library/trackset/setlogfeature.h | 4 +++- 7 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/library/dao/playlistdao.cpp b/src/library/dao/playlistdao.cpp index 477c1047ee76..df1a0944be5d 100644 --- a/src/library/dao/playlistdao.cpp +++ b/src/library/dao/playlistdao.cpp @@ -89,7 +89,7 @@ int PlaylistDAO::createPlaylist(const QString& name, const HiddenType hidden) { int playlistId = query.lastInsertId().toInt(); // Commit the transaction transaction.commit(); - emit added(playlistId); + emit added(playlistId, hidden); return playlistId; } @@ -198,7 +198,8 @@ void PlaylistDAO::deletePlaylist(const int playlistId) { ScopedTransaction transaction(m_database); QSet playedTrackIds; - if (getHiddenType(playlistId) == PLHT_SET_LOG) { + PlaylistDAO::HiddenType type = getHiddenType(playlistId); + if (type == PLHT_SET_LOG) { const QList trackIds = getTrackIds(playlistId); // TODO: QSet::fromList(const QList&) is deprecated and should be @@ -246,7 +247,7 @@ void PlaylistDAO::deletePlaylist(const int playlistId) { } } - emit deleted(playlistId); + emit deleted(playlistId, type); if (!playedTrackIds.isEmpty()) { emit tracksRemovedFromPlayedHistory(playedTrackIds); } @@ -260,6 +261,14 @@ bool PlaylistDAO::deletePlaylists(const QStringList& idStringList) { qInfo() << "Deleting" << idStringList.size() << "playlists"; + PlaylistDAO::HiddenType type = PlaylistDAO::HiddenType::PLHT_UNKNOWN; + // Get playlist type. Assumes playlist batch deletion was invoked + // by the same library feature, ie. all are of same type. + bool ok = false; + int firstId = idStringList.first().toInt(&ok); + if (ok) { + type = getHiddenType(firstId); + } // delete tracks assigned to these playlists auto deleteTracks = FwdSqlQuery(m_database, QString("DELETE FROM PlaylistTracks WHERE playlist_id IN (%1)") @@ -275,7 +284,7 @@ bool PlaylistDAO::deletePlaylists(const QStringList& idStringList) { return false; } - emit deleted(kInvalidPlaylistId); + emit deleted(kInvalidPlaylistId, type); return true; } diff --git a/src/library/dao/playlistdao.h b/src/library/dao/playlistdao.h index f30f89923363..19f36ed52904 100644 --- a/src/library/dao/playlistdao.h +++ b/src/library/dao/playlistdao.h @@ -134,13 +134,17 @@ class PlaylistDAO : public QObject, public virtual DAO { void setAutoDJProcessor(AutoDJProcessor* pAutoDJProcessor); signals: - void added(int playlistId); - void deleted(int playlistId); + // Added/deleted triggers rebuild of the feature's sidebar model. + // Pass the type so receivers (library features) can easily decide + // whether to act or not. + void added(int playlistId, HiddenType type); + void deleted(int playlistId, HiddenType type); void renamed(int playlistId, const QString& newName); void lockChanged(const QSet& playlistIds); void trackAdded(int playlistId, TrackId trackId, int position); void trackRemoved(int playlistId, TrackId trackId, int position); - // added / removed / un/locked. Triggers playlist features to update the sidebar + // Track(s) added/removed or un/locked. Triggers playlist features + // to update the sidebar labels or icons. void playlistContentChanged(const QSet& playlistIds); // Separate signals for PlaylistTableModel void tracksAdded(const QSet& playlistIds); diff --git a/src/library/trackset/baseplaylistfeature.h b/src/library/trackset/baseplaylistfeature.h index 933c391af32d..e859bf0f8711 100644 --- a/src/library/trackset/baseplaylistfeature.h +++ b/src/library/trackset/baseplaylistfeature.h @@ -44,13 +44,13 @@ class BasePlaylistFeature : public BaseTrackSetFeature { virtual void activatePlaylist(int playlistId); virtual void htmlLinkClicked(const QUrl& link); - virtual void slotPlaylistTableChanged(int playlistId) = 0; - void slotPlaylistTableChangedAndSelect(int playlistId) { - slotPlaylistTableChanged(playlistId); - selectPlaylistInSidebar(playlistId); - }; - void slotPlaylistTableChangedAndScrollTo(int playlistId) { - slotPlaylistTableChanged(playlistId); + virtual void slotPlaylistTableChanged( + int playlistId, + PlaylistDAO::HiddenType type) = 0; + void slotPlaylistTableChangedAndScrollTo( + int playlistId, + PlaylistDAO::HiddenType type) { + slotPlaylistTableChanged(playlistId, type); selectPlaylistInSidebar(playlistId, false); }; virtual void slotPlaylistContentOrLockChanged(const QSet& playlistIds) = 0; diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index 0d7e4945622f..036ed06fe7c6 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -384,11 +384,9 @@ void PlaylistFeature::decorateChild(TreeItem* item, int playlistId) { } } -void PlaylistFeature::slotPlaylistTableChanged(int playlistId) { - // qDebug() << "PlaylistFeature::slotPlaylistTableChanged() playlistId:" << playlistId; - enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); - if (type != PlaylistDAO::PLHT_NOT_HIDDEN && // not a regular playlist - type != PlaylistDAO::PLHT_UNKNOWN) { // not a deleted playlist +void PlaylistFeature::slotPlaylistTableChanged(int playlistId, PlaylistDAO::HiddenType type) { + // qDebug() << "PlaylistFeature::slotPlaylistTableChanged() playlistId:" << playlistId << type; + if (type != PlaylistDAO::PLHT_NOT_HIDDEN) { // not a regular playlist return; } @@ -436,7 +434,7 @@ void PlaylistFeature::slotPlaylistTableRenamed(int playlistId, const QString& ne if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_NOT_HIDDEN) { // Maybe we need to re-sort the sidebar items, so call slotPlaylistTableChanged() // in order to rebuild the model, not just updateChildModel() - slotPlaylistTableChanged(playlistId); + slotPlaylistTableChanged(playlistId, PlaylistDAO::PLHT_NOT_HIDDEN); } } diff --git a/src/library/trackset/playlistfeature.h b/src/library/trackset/playlistfeature.h index 1a7772766bc3..17b16c6a0c37 100644 --- a/src/library/trackset/playlistfeature.h +++ b/src/library/trackset/playlistfeature.h @@ -33,7 +33,7 @@ class PlaylistFeature : public BasePlaylistFeature { void onRightClickChild(const QPoint& globalPos, const QModelIndex& index) override; private slots: - void slotPlaylistTableChanged(int playlistId) override; + void slotPlaylistTableChanged(int playlistId, PlaylistDAO::HiddenType type) override; void slotPlaylistContentOrLockChanged(const QSet& playlistIds) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; void slotShufflePlaylist(); diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index eff73a22d7f4..7afccb5c7f04 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -368,6 +368,7 @@ void SetlogFeature::slotGetNewPlaylist() { } // qDebug() << "Creating session history playlist name:" << set_log_name; + int previousPlaylistid = m_currentPlaylistId; m_currentPlaylistId = m_playlistDao.createPlaylist( set_log_name, PlaylistDAO::PLHT_SET_LOG); @@ -380,10 +381,10 @@ void SetlogFeature::slotGetNewPlaylist() { m_playlistDao.setCurrentHistoryPlaylistId(m_currentPlaylistId); } - // reload child model again because the 'added' signal fired by PlaylistDAO - // might have triggered slotPlaylistTableChanged() before m_currentPlaylistId was set, - // which causes the wrong playlist being decorated as 'current' - slotPlaylistTableChanged(m_currentPlaylistId); + // Update child model again because the 'added' signal fired by PlaylistDAO + // might have triggered slotPlaylistTableChanged() before m_currentPlaylistId + // was set, which causes the wrong playlist being decorated as 'current'. + slotPlaylistContentOrLockChanged(QSet{previousPlaylistid, m_currentPlaylistId}); } void SetlogFeature::slotJoinWithPrevious() { @@ -501,9 +502,11 @@ void SetlogFeature::lockOrUnlockAllChildPlaylists(bool lock) { return; } if (lock) { - qWarning() << "lock all child playlists of" << m_lastRightClickedIndex.data().toString(); + qDebug() << "SetlogFeature: locking all child playlists of" + << m_lastRightClickedIndex.data().toString(); } else { - qWarning() << "unlock all child playlists of" << m_lastRightClickedIndex.data().toString(); + qWarning() << "SetlogFeature: unlocking all child playlists of" + << m_lastRightClickedIndex.data().toString(); } TreeItem* item = static_cast(m_lastRightClickedIndex.internalPointer()); if (!item) { @@ -656,11 +659,12 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { } } -void SetlogFeature::slotPlaylistTableChanged(int playlistId) { - // qDebug() << "SetlogFeature::slotPlaylistTableChanged() id:" << playlistId; - PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); - if (type != PlaylistDAO::PLHT_SET_LOG && - type != PlaylistDAO::PLHT_UNKNOWN) { // deleted Playlist +void SetlogFeature::slotPlaylistTableChanged(int playlistId, PlaylistDAO::HiddenType type) { + // qDebug() << "SetlogFeature::slotPlaylistTableChanged() id:" << playlistId << type; + // Note: we only care about PLHT_SET_LOG as that's the only relevant type for + // rebuilding the tree. Type PLHT_UNKNOWN is only used for YEAR placeholder + // playlist and is assigned to items dynamically with YEAR label. + if (type != PlaylistDAO::PLHT_SET_LOG) { return; } @@ -675,7 +679,7 @@ void SetlogFeature::slotPlaylistTableChanged(int playlistId) { // a YEAR item was selected selectedYearIndexRow = m_lastClickedIndex.row(); } else if (playlistId == lastClickedPlaylistId && - type == PlaylistDAO::PLHT_UNKNOWN) { + m_playlistDao.getHiddenType(lastClickedPlaylistId) == PlaylistDAO::PLHT_UNKNOWN) { // selected playlist was deleted, find a sibling. // prev/next works here because history playlists are always // sorted by date of creation. diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 143641738717..2eca52d4c3ad 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -40,7 +40,9 @@ class SetlogFeature : public BasePlaylistFeature { private slots: void slotPlayingTrackChanged(TrackPointer currentPlayingTrack); - void slotPlaylistTableChanged(int playlistId) override; + void slotPlaylistTableChanged( + int playlistId, + PlaylistDAO::HiddenType type) override; void slotPlaylistContentOrLockChanged(const QSet& playlistIds) override; void slotPlaylistTableRenamed(int playlistId, const QString& newName) override; void slotDeleteAllUnlockedChildPlaylists(); From 59938de697f0e1ff584530aeb8df2180e392aaa3 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 27 Mar 2025 14:23:43 +0100 Subject: [PATCH 05/13] TreeItem: add helper isItemDataUnique() This allows to check if an item has unique data. Default is true. SetlogFeature for example uses the same playlist id for its YEAR items, and for now that's the only feature that overrides the default implementation. --- src/library/libraryfeature.h | 4 ++++ src/library/trackset/setlogfeature.h | 3 +++ src/library/treeitem.cpp | 5 +++++ src/library/treeitem.h | 1 + 4 files changed, 13 insertions(+) diff --git a/src/library/libraryfeature.h b/src/library/libraryfeature.h index 4e8390a02e2c..d1a865e8753d 100644 --- a/src/library/libraryfeature.h +++ b/src/library/libraryfeature.h @@ -87,6 +87,10 @@ class LibraryFeature : public QObject { virtual bool hasTrackTable() { return false; } + virtual bool isItemDataUnique(const QVariant& data) const { + Q_UNUSED(data); + return true; + } protected: QStringList getPlaylistFiles() const { diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 2eca52d4c3ad..4d0980a0bb8b 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -21,6 +21,9 @@ class SetlogFeature : public BasePlaylistFeature { void bindLibraryWidget(WLibrary* libraryWidget, KeyboardEventFilter* keyboard) override; void activatePlaylist(int playlistId) override; + bool isItemDataUnique(const QVariant& data) const override { + return data != QVariant(m_yearNodeId); + } public slots: void onRightClick(const QPoint& globalPos) override; diff --git a/src/library/treeitem.cpp b/src/library/treeitem.cpp index a6bd742005a1..91ca87c0f580 100644 --- a/src/library/treeitem.cpp +++ b/src/library/treeitem.cpp @@ -1,5 +1,6 @@ #include "library/treeitem.h" +#include "library/libraryfeature.h" #include "util/make_const_iterator.h" /* @@ -110,3 +111,7 @@ void TreeItem::removeChildren(int row, int count) { qDeleteAll(m_children.constBegin() + row, m_children.constBegin() + (row + count)); constErase(&m_children, m_children.constBegin() + row, m_children.constBegin() + (row + count)); } + +bool TreeItem::isDataUniqueInFeature() const { + return feature()->isItemDataUnique(m_data); +} diff --git a/src/library/treeitem.h b/src/library/treeitem.h index 02d792cb23e7..75c0c24c3442 100644 --- a/src/library/treeitem.h +++ b/src/library/treeitem.h @@ -111,6 +111,7 @@ class TreeItem final { const QVariant& getData() const { return m_data; } + bool isDataUniqueInFeature() const; void setIcon(const QIcon& icon) { m_icon = icon; From 0516af5eaf4769c8bb57817d4b5952bef03c588a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 12 Dec 2025 15:25:57 +0100 Subject: [PATCH 06/13] Library sidebar: add bookmarks + navigation controls Alt+B toggles bookmark flag Alt+Up/Down jumps to prev/next (highlighted, not activated) Enter to activate --- res/skins/LateNight/style_palemoon.qss | 29 ++- src/library/libraryfeature.cpp | 5 +- src/library/sidebarmodel.cpp | 287 ++++++++++++++++++++++++- src/library/sidebarmodel.h | 96 +++++++++ src/library/treeitem.h | 10 + src/widget/wlibrarysidebar.cpp | 92 +++++++- src/widget/wlibrarysidebar.h | 5 +- 7 files changed, 508 insertions(+), 16 deletions(-) diff --git a/res/skins/LateNight/style_palemoon.qss b/res/skins/LateNight/style_palemoon.qss index 36e463d44243..d9ff4fc60ae4 100644 --- a/res/skins/LateNight/style_palemoon.qss +++ b/res/skins/LateNight/style_palemoon.qss @@ -3019,15 +3019,26 @@ WTrackTableView::item:selected, color: #fff; } -WLibrarySidebar::item:selected:focus { - outline: none; -} -WLibrarySidebar::item:!selected:focus, -/* This, in conjunction with the c++ counterpart, styles only items hovered while dragging */ -WLibrarySidebar[dragHover="true"]::item:hover { - outline: none; - border: 1px solid white; -} +WLibrarySidebar::item { + /* add a dummy border so item text is not shifted when item is focused */ + border-left: 1px solid transparent; + /* add right border so text is not elided when focus border is added */ + border-right: 1px solid transparent; + } + /* subtle highlight if item is selected */ + WLibrarySidebar::item:focus { + outline: none; + border: 1px solid #999; + } + WLibrarySidebar::item:selected:focus { + outline: none; + } + WLibrarySidebar::item:!selected:focus, + /* This, in conjunction with the c++ counterpart, styles only items hovered while dragging */ + WLibrarySidebar[dragHover="true"]::item:hover { + outline: none; + border: 1px solid white; + } WTrackTableView:focus, WLibrarySidebar:focus, diff --git a/src/library/libraryfeature.cpp b/src/library/libraryfeature.cpp index 7c88fbf93bdc..06638e3dac90 100644 --- a/src/library/libraryfeature.cpp +++ b/src/library/libraryfeature.cpp @@ -27,9 +27,10 @@ LibraryFeature::LibraryFeature( m_pLibrary(pLibrary), m_pConfig(pConfig), m_iconName(iconName) { - if (!m_iconName.isEmpty()) { - m_icon = QIcon(kIconPath.arg(m_iconName)); + VERIFY_OR_DEBUG_ASSERT(!m_iconName.isEmpty()) { + return; } + m_icon = QIcon(kIconPath.arg(m_iconName)); } void LibraryFeature::selectAndActivate(const QModelIndex& index) { diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index e538dc9e405c..c651199b9f89 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -545,8 +545,9 @@ void SidebarModel::slotRowsInserted(const QModelIndex& parent, int start, int en Q_UNUSED(start); Q_UNUSED(end); // qDebug() << "slotRowsInserted" << parent << start << end; - // QModelIndex newParent = translateSourceIndex(parent); + QModelIndex newParent = translateSourceIndex(parent); endInsertRows(); + maybeUpdateBookmarkIndices(newParent); } void SidebarModel::slotRowsRemoved(const QModelIndex& parent, int start, int end) { @@ -609,3 +610,287 @@ void SidebarModel::slotFeatureSelect(LibraryFeature* pFeature, } emit selectIndex(ind, scrollTo); } + +void SidebarModel::toggleBookmarkByIndex(const QModelIndex& index) { + qWarning() << " toggleBookmarkByIndex" << index; + if (!index.isValid()) { + return; + } + + SidebarBookmark bookmark = createBookmarkFromIndex(index); + if (m_bookmarks.contains(bookmark)) { + // Remove bookmark and index + m_bookmarks.removeOne(bookmark); + qWarning() << "--- removed" << bookmark; + } else { + m_bookmarks.append(bookmark); + qWarning() << "+++ added" << bookmark; + } + m_bookmarkIndices = sortBookmarksUpdateIndices(&m_bookmarks); +} + +QModelIndexList SidebarModel::sortBookmarksUpdateIndices(QList* pBookmarks) { + // Sort by position in the tree so getNextPrevBookmarkIndex() + // switches to bookmark below/above in a predictable manner: + // feature row -> child level -> parent row + std::sort(pBookmarks->begin(), pBookmarks->end()); + // Update indices. Only add valid indices -- invalid means the bookmark + // wasn't found after last child model update. + QModelIndexList newBookmarkIndices; + for (const auto& bm : std::as_const(*pBookmarks)) { + if (bm.index.isValid()) { + newBookmarkIndices.append(bm.index); + } + } + return newBookmarkIndices; + qDebug() << "Sidebar bookmarks updated"; +} + +QModelIndex SidebarModel::getNextPrevBookmarkIndex(const QModelIndex& currIndex, int direction) { + if (!currIndex.isValid() || direction == 0) { + qWarning() << "SidebarModel::getNextPrevBookmarkIndex: invalid index " + "or dir == 0" + << currIndex; + return {}; + } + if (m_bookmarks.isEmpty()) { + qWarning() << "SidebarModel::getNextPrevBookmarkIndex: no bookmarks stored"; + return {}; + } + + if (m_bookmarkIndices.size() == 1 && currIndex == m_bookmarkIndices[0]) { + // We already have the only bookmark selected. + // Return invalid index so the sidebar does not reload the current view. + return {}; + } + + // Do single steps regardless the input. + direction = direction > 0 ? 1 : -1; + qWarning("SidebarModel::getNextPrevBookmarkIndex, dir: %d", direction); + + int currPos = 0; + bool forward = direction > 0; + const int numItems = m_bookmarks.size(); + // Create a bookmark from the current position + const SidebarBookmark tempBM = createBookmarkFromIndex(currIndex); + if (m_bookmarks.contains(tempBM)) { + currPos = m_bookmarks.indexOf(tempBM); + // qWarning(" we're on bookmark, curr pos: %d", currPos); + } else { + // We're not on a bookmark. + // In order to get true prev/next (up/down) behavior from the current + // position, we clone m_bookmarks, add the temporary bookmark, + // sort and get the list index of the current position. + auto tempBookmarks = m_bookmarks; + tempBookmarks.append(tempBM); + sortBookmarksUpdateIndices(&tempBookmarks); + currPos = tempBookmarks.indexOf(tempBM); + // qWarning(" not on a bookmark, creates temp bookmark, curr pos: %d", currPos); + if (forward) { + currPos--; + } + // qWarning(" -> adjusted curr pos to %d", currPos); + } + // Now we have a valid start position in the bookmark list. + // Calculate the target position. + int targetPos = currPos + direction; + // qWarning(" targetPos orig: %d | numItems: %d", targetPos, numItems); + + SidebarBookmark targetBM; + QModelIndex targetIndex; + int attempt = 0; + while (attempt < numItems) { + // qWarning(" find targetBm -- attempt %d | targetPos: %d", attempt, targetPos); + // Check if we need to warp around + if (targetPos < 0 || targetPos >= numItems) { + forward ? targetPos = 0 : targetPos = numItems - 1; + // qWarning(" targetPos < 0 || >= numItems, adjust to %d", targetPos); + } + targetBM = m_bookmarks[targetPos]; + // qWarning() << " targetBm:" << targetBM; + if (targetBM.isValid()) { + // qWarning() << " -> targetBm valid"; + if (targetBM.childLevel == 0) { + // If this is root item we can simply create an index, no need + // for findBookmarkIndex(). + // qWarning(" bookmark is root item %d", targetBM.featureRow); + targetIndex = index(targetBM.featureRow, 0); + } else { + // Else we look up the bookmark inside the child model. + // This may fail, eg. when an item for a bookmark restored from + // config is not present (yet), like Computer items in branches + // that have not been built, yet. + // qWarning(" bookmark is child at level %d", targetBM.childLevel); + const auto bmIndex = findBookmarkIndex(targetBM); + // qWarning() << " bmIndex:" << bmIndex; + targetIndex = translateChildIndex(bmIndex); + // qWarning() << " > translateChIdx:" << targetIndex; + } + } else { + // qWarning() << " -> targetBm NOT valid, continue"; + } + if (targetIndex.isValid()) { + break; + } + forward ? targetPos++ : targetPos--; + attempt++; + } + + if (targetIndex != targetBM.index) { + // Lookup bookmark. + // If found, replace with fresh bookmark, ie. update all properties at once. + // (just in case the index changed and we didn't notice) + // Else just invalidate it's index so we know that we should skip it when + // updating the index list. + if (targetIndex.isValid()) { + targetBM = createBookmarkFromIndex(translateSourceIndex(targetIndex)); + } else { + targetBM.index = QModelIndex(); + } + sortBookmarksUpdateIndices(&m_bookmarks); + } + return targetIndex; +} + +/// Invoked by rowsInserted(). A range of sidebar indices of a childmodel has been +/// rebuilt. Some stored indices may now be invalid so we need to reassociate +/// bookmarks with new indices and update the quick-lookup index list. +/// Happens when playlists or crates are added/removedm, when a History playlist +/// has been moved into a YEAR group or when the BrowseFeature tree is rebuilt. +// +// TODO(ronso0) Implement some lock/wait mechanism to avoid concurrent access to +// m_bookmarks/m_bookmarkIndices. Caller is in same thread so QMutex won't work. +// Reason: previously, deleting a playlist caused both PlaylistFeature and SetlogFeauture +// to rebuilt their child models, even though only one of them can be affected. +// This is now fixed, but I didn't check with other features, hence can't guarantee +// singular/safe access. +void SidebarModel::maybeUpdateBookmarkIndices(const QModelIndex& parentIndex) { + // qWarning() << "maybeUpdateBookmarkIndices" << parentIndex; + if (m_bookmarkIndices.isEmpty()) { + return; + } + // Collect the start parameters for findBookmarkIndex() + int featureRow = -1; + if (parentIndex.internalPointer() == this) { + featureRow = parentIndex.row(); + } else { + const auto* pTreeItem = static_cast(parentIndex.internalPointer()); + VERIFY_OR_DEBUG_ASSERT(pTreeItem) { + return; + } + featureRow = m_sFeatures.indexOf(pTreeItem->feature()); + } + + bool needsUpdate = false; + for (auto& bm : m_bookmarks) { + if (bm.featureRow != featureRow) { + continue; + } + needsUpdate = true; + // Lookup bookmark. + // If found, replace with fresh bookmark, ie. update all properties at once. + // Else just invalidate it's index so we know that we should skip it when + // updating the index list. + const auto bmIndex = findBookmarkIndex(bm); + if (bmIndex.isValid()) { + bm = createBookmarkFromIndex(translateSourceIndex(bmIndex)); + // qWarning() << " >> updated" << bm << "in" << pFeature->title().toString(); + } else { + // qWarning() << " >> no match for" << bm << "in" << pFeature->title().toString(); + bm.index = QModelIndex(); + } + } + + if (!needsUpdate) { + // no hit for affected feature, nothing to do + return; + } + + // Note: Don't remove missing bookmarks. + // BrowseFeature bookmarks may be 'missing' after collapsing and + // re-expanding a directory tree one or more levels above a bookmark + // because expanding an item only rebuilds the next sublevel, so + // bookmarked items on lower levels are simply not there, yet. + + m_bookmarkIndices = sortBookmarksUpdateIndices(&m_bookmarks); +} + +/// Try to find the matching TreeItem in a feature's childmodel. +/// Return its index when found, else return invalid QModelIndex(). +/// Scans the entire tree recursively, either by item data or label. +// TODO Do we need to store an index list at all when using match()? +// Compare performance of list vs. match() for each next/prev move. +QModelIndex SidebarModel::findBookmarkIndex(const SidebarBookmark& bookmark) { + qWarning() << " findBookmarkIndex" << bookmark; + VERIFY_OR_DEBUG_ASSERT(bookmark.isValid()) { + return {}; + } + LibraryFeature* pFeature = m_sFeatures[bookmark.featureRow]; + TreeItemModel* pChildModel = pFeature->sidebarModel(); + DEBUG_ASSERT(pChildModel); + const QModelIndex rootIndex = index(bookmark.featureRow, 0); + QModelIndexList results; + if (bookmark.data.isValid() && pFeature->isItemDataUnique(bookmark.data)) { + // Search for matching data + results = pChildModel->match( + rootIndex, + TreeItemModel::kDataRole, + bookmark.data, + 1, + Qt::MatchWrap | Qt::MatchExactly | Qt::MatchRecursive); + } else { + // Search for label match. + // This covers root items, Tracks Missing/Hidden, AutoDJ Crates + // and History's YEAR nodes. + results = pChildModel->match( + rootIndex, + Qt::DisplayRole, + bookmark.label, + 1, + Qt::MatchWrap | Qt::MatchExactly | Qt::MatchRecursive); + } + + if (!results.isEmpty()) { + return results.front(); + } + return {}; +} + +SidebarBookmark SidebarModel::createBookmarkFromIndex(const QModelIndex& index) { + if (!index.isValid()) { + return {}; + } + + // qWarning() << " >> getBookmarkFromIndex" << index; + SidebarBookmark bookmark; + if (index.internalPointer() == this) { + LibraryFeature* pFeature = m_sFeatures[index.row()]; + bookmark = SidebarBookmark( + index.row(), + 0, + 0, + QVariant(), + // pFeature->title() is not const at runtime, hence not suitable as identifier! + // For example "Tracks (numTracks)" or dynamically loading features + pFeature->iconName(), + index); + } else { + TreeItem* pTreeItem = static_cast(index.internalPointer()); + VERIFY_OR_DEBUG_ASSERT(pTreeItem) { + return {}; + } + bookmark.featureRow = m_sFeatures.indexOf(pTreeItem->feature()); + bookmark.childLevel = pTreeItem->childLevel(); + bookmark.parentRow = pTreeItem->parentRow(); + const auto& data = pTreeItem->getData(); + if (data.isValid() && pTreeItem->isDataUniqueInFeature()) { + bookmark.data = data; + } else { + bookmark.label = pTreeItem->getLabel(); + } + bookmark.index = index; + } + // qWarning() << " >> created" << bookmark; + DEBUG_ASSERT(bookmark.isValid()); + return bookmark; +} diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index d7294d8812de..c7f5b2b8e0bb 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -7,6 +7,90 @@ class LibraryFeature; class QTimer; +class TreeItem; + +struct SidebarBookmark { + SidebarBookmark( + int row = -1, + int cLevel = -1, + int pRow = -1, + const QVariant& datavar = QVariant(), + const QString& sLabel = QString(), + const QModelIndex& dIndex = QModelIndex()) + : featureRow(row), + childLevel(cLevel), + parentRow(pRow), + data(datavar), + label(sLabel), + index(dIndex) { + } + bool isValid() const { + // qDebug() << " SidebarBookmark isValid()"; + return featureRow >= 0 && + childLevel >= 0 && + parentRow >= 0 && + (data.isValid() || !label.isEmpty()) && + index.isValid(); + } + bool operator==(const SidebarBookmark& other) const { + // qDebug() << " SidebarBookmark==op"; + if (featureRow != other.featureRow) { + // qDebug() << " -> featureRow != other.featureRow"; + return false; + } + if (data.isValid() && other.data.isValid()) { + // qDebug() << " -> data.isValid() && other.data.isValid()"; + // qDebug() << " -> return (data" << data << other.data; + return data == other.data; + } + // qDebug() << " -> else -> return (label" << label << other.label; + return label == other.label; + } + bool operator<(const SidebarBookmark& other) const { + if (featureRow == other.featureRow) { + if (childLevel == other.childLevel) { + return parentRow < other.parentRow; + } + return childLevel < other.childLevel; + } + return featureRow < other.featureRow; + } + + // Store feature row, child level and row number relative to first parent in + // order to allow sorting bookmarks by their position in the tree. We need + // this because QModelIndex operator<() doesn#t work for our multi-level trees. + // With the feature row we can also quickly ignore irrelevant items when + // searching for a bookmark a rebuilt child model. + int featureRow; + int childLevel; + int parentRow; + // The TreeItem data. CrateId, int playlist id or directory path / special + // node identifier in BrowseFeature. + QVariant data; + // For child items that have invalid data (Tracks Missing|Hidden and + // AutoDJ Crates) or when the LibraryFeature says the data is not unique + // (common playlist id of YEAR nodes). + QString label; + // The associated sidebar index. This is used to build the index lookup list + // for getNextPrevBookmarkIndex(). + // Must be updated when the child model is changed. + QModelIndex index; +}; + +inline QDebug operator<<(QDebug dbg, const SidebarBookmark& bm) { + dbg << "SidebarBookmark" << bm.featureRow << bm.childLevel << bm.parentRow; + if (bm.data.isValid()) { + if (bm.data.canConvert()) { + dbg << bm.data.toString(); + } else if (bm.data.canConvert()) { + dbg << bm.data.toInt(); + } + } + if (!bm.label.isEmpty()) { + dbg << bm.label; + } + return dbg; +} class SidebarModel : public QAbstractItemModel { Q_OBJECT @@ -49,6 +133,10 @@ class SidebarModel : public QAbstractItemModel { void clear(const QModelIndex& index); void paste(const QModelIndex& index); + + void toggleBookmarkByIndex(const QModelIndex& selIndex); + QModelIndex getNextPrevBookmarkIndex(const QModelIndex& selIndex, int direction); + public slots: void pressed(const QModelIndex& index); void clicked(const QModelIndex& index); @@ -96,7 +184,15 @@ class SidebarModel : public QAbstractItemModel { QTimer* const m_pressedUntilClickedTimer; QModelIndex m_pressedIndex; + QList m_bookmarks; + QModelIndexList m_bookmarkIndices; void startPressedUntilClickedTimer(const QModelIndex& pressedIndex); void stopPressedUntilClickedTimer(); + + QModelIndexList sortBookmarksUpdateIndices(QList* pBookmarks); + QModelIndex getBookmarkIndexByPos(int pos); + QModelIndex findBookmarkIndex(const SidebarBookmark& bookmark); + SidebarBookmark createBookmarkFromIndex(const QModelIndex& index); + void maybeUpdateBookmarkIndices(const QModelIndex& index); }; diff --git a/src/library/treeitem.h b/src/library/treeitem.h index 75c0c24c3442..b5843999b353 100644 --- a/src/library/treeitem.h +++ b/src/library/treeitem.h @@ -83,6 +83,16 @@ class TreeItem final { const QList& children() const { return m_children; } + /// Get the tree level of the item. Returns 0 if this is the root. + int childLevel() const { + int level = 0; + TreeItem* pTempItem = const_cast(this); + while (pTempItem->hasParent()) { + level++; + pTempItem = pTempItem->parent(); + } + return level; + } TreeItem* appendChild( QString label, diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 0a0ec278bdc7..499ca3696d84 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -262,6 +262,46 @@ bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { return rootIndex == selIndex; } +void WLibrarySidebar::toggleBookmark() { + const QModelIndex selIndex = selectedIndex(); + // TODO add visual hint, custom qproperty + if (!selIndex.isValid()) { + qWarning() << " ! WLS bookmarkSelectedItem, invalid index" << selIndex; + return; + } + + m_pSidebarModel->toggleBookmarkByIndex(selIndex); + update(); +} + +void WLibrarySidebar::goToNextPrevBookmark(int direction) { + // Don't use selectedIndex(). Selected item may not be the focused item, eg. + // if we focused a bookmark item without activating it. + QModelIndex index = currentIndex(); + if (!index.isValid()) { + qDebug() << "WLibrarySidebar::goToNextPrevBookmark invalid index" << index; + return; + } + + const QModelIndex bookmarkIdx = m_pSidebarModel->getNextPrevBookmarkIndex(index, direction); + if (!bookmarkIdx.isValid() || bookmarkIdx == index) { + // No bookmarks stored or none of them has been found. + // Or, we are already on the only bookmark. In that case we don't reselect because + // that would cause resorting (and reloading tracks for Computer path indices). + return; + } + + // just scroll to and highlight (focus) + scrollTo(bookmarkIdx); + // Use this instead of setCurrentIndex() to keep current selection + selectionModel()->setCurrentIndex(bookmarkIdx, QItemSelectionModel::NoUpdate); + // TODO add control [Library],goToSelectedItem ?? + // Or add wrapper goToItem() that does + // * select & activate focused item if it's not selected + // * expand / collapse + // * jump to tracks if double-tapped +} + /// Invoked by actual keypresses (requires widget focus) and emulated keypresses /// sent by LibraryControl void WLibrarySidebar::keyPressEvent(QKeyEvent* pEvent) { @@ -273,10 +313,27 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* pEvent) { return; } - focusSelectedIndex(); + // Don't focus selection if we receive a modifier-only event, for example + // Alt + B: un/bookmark selected item + // Alt + Up/Down: jump to and highlight next/previous bookmarked item + // Press Enter to activate + if (pEvent->modifiers().testFlag(Qt::AltModifier)) { + if (pEvent->key() == Qt::Key_Down || pEvent->key() == Qt::Key_Up) { + goToNextPrevBookmark(pEvent->key() == Qt::Key_Down ? 1 : -1); + } else if (pEvent->key() == Qt::Key_B) { + toggleBookmark(); + } + // No further Alt, might as well be a system shortcut + return; + } switch (pEvent->key()) { case Qt::Key_Return: + // If the selection is not focused, focus it and scroll to it first. + // Happens when going to bookmark with activating it. + if (selectFocusedIndex()) { + return; + } toggleSelectedItem(); return; case Qt::Key_Down: @@ -285,6 +342,11 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* pEvent) { case Qt::Key_PageUp: case Qt::Key_End: case Qt::Key_Home: { + // If the selection is not focused, focus it and scroll to it first. + // Happens when going to bookmark without activating it. + if (focusSelectedIndex()) { + return; + } // Let the tree view move up and down for us. QTreeView::keyPressEvent(pEvent); // After the selection changed force-activate (click) the newly selected @@ -304,11 +366,17 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* pEvent) { if (pEvent->modifiers() & Qt::ControlModifier) { emit setLibraryFocus(FocusWidget::TracksTable); } else { + if (focusSelectedIndex()) { + return; + } QTreeView::keyPressEvent(pEvent); } return; } case Qt::Key_Left: { + if (focusSelectedIndex()) { + return; + } // If an expanded item is selected let QTreeView collapse it QModelIndex selIndex = selectedIndex(); if (!selIndex.isValid()) { @@ -362,7 +430,7 @@ void WLibrarySidebar::mousePressEvent(QMouseEvent* pEvent) { void WLibrarySidebar::focusInEvent(QFocusEvent* pEvent) { // Clear the current index, i.e. remove the focus indicator - selectionModel()->clearCurrentIndex(); + focusSelectedIndex(); QTreeView::focusInEvent(pEvent); } @@ -430,14 +498,32 @@ QModelIndex WLibrarySidebar::selectedIndex() { } /// Refocus the selected item after right-click -void WLibrarySidebar::focusSelectedIndex() { +bool 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); + return true; } + return false; +} + +bool WLibrarySidebar::selectFocusedIndex() { + const QModelIndex selIndex = selectedIndex(); + const QModelIndex focusIndex = selectionModel()->currentIndex(); + // qDebug() << " -- selected index:" << selIndex; + // qDebug() << " -- focused index: " << focusIndex; + if (focusIndex.isValid() && focusIndex != selIndex) { + // qDebug() << " -- select focused index, scroll to"; + scrollTo(focusIndex); + selectIndex(focusIndex); + emit pressed(focusIndex); + return true; + } + // qDebug() << " -- ! focused index invalid" << focusIndex; + return false; } bool WLibrarySidebar::event(QEvent* pEvent) { diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index 2553a62ef6b3..c67a62407064 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -50,11 +50,14 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { bool event(QEvent* pEvent) override; private: - void focusSelectedIndex(); + bool focusSelectedIndex(); + bool selectFocusedIndex(); QModelIndex selectedIndex(); void toggleDragHoverPropertyAndUpdateStyle(bool enabled); void resetHoverIndexAndDragMoveResult(); + void toggleBookmark(); + void goToNextPrevBookmark(int direction); SidebarModel* m_pSidebarModel; QBasicTimer m_expandTimer; From eeebc5bd6b11f4cd4460b343cbd13cd6cbe63ccc Mon Sep 17 00:00:00 2001 From: ronso0 Date: Mon, 28 Apr 2025 14:10:33 +0200 Subject: [PATCH 07/13] Library sidebar: decorate bookmarks with SidebarItemDelegate color can be set in qss with WLibrarySidebar { qproperty-bookmarkColor: #451231; } --- CMakeLists.txt | 1 + res/skins/LateNight/style_palemoon.qss | 5 ++-- src/library/sidebaritemdelegate.cpp | 36 ++++++++++++++++++++++++++ src/library/sidebaritemdelegate.h | 31 ++++++++++++++++++++++ src/library/sidebarmodel.cpp | 7 +++++ src/library/sidebarmodel.h | 2 ++ src/widget/wlibrarysidebar.cpp | 29 +++++++++++++++++++-- src/widget/wlibrarysidebar.h | 11 ++++++++ 8 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 src/library/sidebaritemdelegate.cpp create mode 100644 src/library/sidebaritemdelegate.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 46547ce6106f..ab26d0c99bdd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1455,6 +1455,7 @@ add_library( src/library/searchqueryparser.cpp src/library/serato/seratofeature.cpp src/library/serato/seratoplaylistmodel.cpp + src/library/sidebaritemdelegate.cpp src/library/sidebarmodel.cpp src/library/starrating.cpp src/library/tabledelegates/bpmdelegate.cpp diff --git a/res/skins/LateNight/style_palemoon.qss b/res/skins/LateNight/style_palemoon.qss index d9ff4fc60ae4..37ed762dc274 100644 --- a/res/skins/LateNight/style_palemoon.qss +++ b/res/skins/LateNight/style_palemoon.qss @@ -2996,6 +2996,8 @@ WLibrarySidebar { } WLibrarySidebar { outline: none; + show-decoration-selected: 0; + qproperty-bookmarkColor: #ff0000; } /* Selected rows in Tree and Tracks table */ WLibrarySidebar::item:selected, @@ -3365,9 +3367,6 @@ QLabel#labelSelectionInfo /* AutoDJ track selection info */ { margin: 4px 5px 5px 1px; } -WLibrarySidebar { - show-decoration-selected: 0; -} /* triangle for closed/opened branches in treeview */ /* closed */ WLibrarySidebar::branch:closed:has-children:!has-siblings:!selected, diff --git a/src/library/sidebaritemdelegate.cpp b/src/library/sidebaritemdelegate.cpp new file mode 100644 index 000000000000..58192ca08940 --- /dev/null +++ b/src/library/sidebaritemdelegate.cpp @@ -0,0 +1,36 @@ +#include "library/sidebaritemdelegate.h" + +#include +#include + +#include "library/sidebarmodel.h" +#include "moc_sidebaritemdelegate.cpp" +#include "util/assert.h" +#include "widget/wlibrarysidebar.h" + +SidebarItemDelegate::SidebarItemDelegate( + WLibrarySidebar* pSidebarWidget, + SidebarModel* pSidebarModel) + : QStyledItemDelegate(pSidebarWidget), + m_pSidebarModel(pSidebarModel) { + DEBUG_ASSERT(m_pSidebarModel); +} + +void SidebarItemDelegate::paint( + QPainter* pPainter, + const QStyleOptionViewItem& option, + const QModelIndex& index) const { + QStyledItemDelegate::paint(pPainter, option, index); + + // If the item is a bookmark, draw the indicator on top of the qss style. + // We draw a rectangle a the left, narrow enough to not cover the label and + // inset by 1px to not cover the focus border. + if (m_bookmarkColor.isValid() && m_pSidebarModel->indexIsBookmark(index)) { + pPainter->fillRect( + option.rect.x(), + option.rect.y() + 1, + 3, // width + option.rect.height() - 2, + m_bookmarkColor); + } +} diff --git a/src/library/sidebaritemdelegate.h b/src/library/sidebaritemdelegate.h new file mode 100644 index 000000000000..c8175c1aa781 --- /dev/null +++ b/src/library/sidebaritemdelegate.h @@ -0,0 +1,31 @@ +#pragma once + +#include +#include + +class SidebarModel; +class WLibrarySidebar; + +class SidebarItemDelegate : public QStyledItemDelegate { + Q_OBJECT + public: + explicit SidebarItemDelegate( + WLibrarySidebar* pSidebarwidget, + SidebarModel* pSidebarModel); + ~SidebarItemDelegate() override = default; + + void paint( + QPainter* pPainter, + const QStyleOptionViewItem& option, + const QModelIndex& index) const override; + + void setBookmarkColor(const QColor& color) { + if (color.isValid()) { + m_bookmarkColor = color; + } + } + + private: + SidebarModel* m_pSidebarModel; // shared_ptr? + QColor m_bookmarkColor; +}; diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index c651199b9f89..190bac18b1b4 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -894,3 +894,10 @@ SidebarBookmark SidebarModel::createBookmarkFromIndex(const QModelIndex& index) DEBUG_ASSERT(bookmark.isValid()); return bookmark; } + +bool SidebarModel::indexIsBookmark(const QModelIndex& index) const { + if (!index.isValid()) { + return false; + } + return m_bookmarkIndices.contains(index); +} diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index c7f5b2b8e0bb..32867cbda7c6 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -137,6 +137,8 @@ class SidebarModel : public QAbstractItemModel { void toggleBookmarkByIndex(const QModelIndex& selIndex); QModelIndex getNextPrevBookmarkIndex(const QModelIndex& selIndex, int direction); + bool indexIsBookmark(const QModelIndex& index) const; + public slots: void pressed(const QModelIndex& index); void clicked(const QModelIndex& index); diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 499ca3696d84..130d772a93e7 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -5,17 +5,26 @@ #include #include "library/library_prefs.h" +#include "library/sidebaritemdelegate.h" #include "library/sidebarmodel.h" #include "moc_wlibrarysidebar.cpp" #include "util/defs.h" #include "util/dnd.h" +namespace { + +const QColor kDefaultBookmarkColor = QColor(Qt::red); + +} // anonymous namespace + WLibrarySidebar::WLibrarySidebar(QWidget* parent) : QTreeView(parent), WBaseWidget(this), m_pSidebarModel(nullptr), + m_pItemDelegate(nullptr), m_hoverExpandDelay(mixxx::library::prefs::kSidebarHoverExpandDelayDefault), - m_lastDragMoveAccepted(false) { + m_lastDragMoveAccepted(false), + m_bookmarkColor(kDefaultBookmarkColor) { qRegisterMetaType("FocusWidget"); //Set some properties setHeaderHidden(true); @@ -37,6 +46,17 @@ void WLibrarySidebar::setModel(QAbstractItemModel* pModel) { DEBUG_ASSERT(pSidebarModel); m_pSidebarModel = pSidebarModel; QTreeView::setModel(pSidebarModel); + // Create the delegate for painting the bookmark indicator + DEBUG_ASSERT(m_pItemDelegate == nullptr); + m_pItemDelegate = new SidebarItemDelegate(this, pSidebarModel); + setItemDelegateForColumn(0, m_pItemDelegate); + m_pItemDelegate->setBookmarkColor(m_bookmarkColor); + // Color can be set in qss via qproperty-bookmarkColor which happens + // when the stylesheet is applied. Push it to delegate. + connect(this, + &WLibrarySidebar::bookmarkColorChanged, + m_pItemDelegate, + &SidebarItemDelegate::setBookmarkColor); } void WLibrarySidebar::contextMenuEvent(QContextMenuEvent* pEvent) { @@ -262,9 +282,14 @@ bool WLibrarySidebar::isFeatureRootIndexSelected(LibraryFeature* pFeature) { return rootIndex == selIndex; } +void WLibrarySidebar::setBookmarkColor(const QColor& color) { + if (color.isValid() && m_pItemDelegate) { + m_pItemDelegate->setBookmarkColor(color); + } +} + void WLibrarySidebar::toggleBookmark() { const QModelIndex selIndex = selectedIndex(); - // TODO add visual hint, custom qproperty if (!selIndex.isValid()) { qWarning() << " ! WLS bookmarkSelectedItem, invalid index" << selIndex; return; diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index c67a62407064..22d5a4eef8ae 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -8,6 +8,7 @@ #include "widget/wbasewidget.h" class LibraryFeature; +class SidebarItemDelegate; class SidebarModel; class QPoint; @@ -16,6 +17,11 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { public: explicit WLibrarySidebar(QWidget* parent = nullptr); + Q_PROPERTY(QColor bookmarkColor + MEMBER m_bookmarkColor + NOTIFY bookmarkColorChanged + DESIGNABLE true); + void setModel(QAbstractItemModel* pModel) override; void contextMenuEvent(QContextMenuEvent* pEvent) override; @@ -33,6 +39,8 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { bool isChildIndexSelected(const QModelIndex& index); bool isFeatureRootIndexSelected(LibraryFeature* pFeature); + void setBookmarkColor(const QColor& color); + public slots: void selectIndex(const QModelIndex& index, bool scrollToIndex = true); void selectChildIndex(const QModelIndex&, bool selectItem = true); @@ -45,6 +53,7 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void deleteItem(const QModelIndex&); FocusWidget setLibraryFocus(FocusWidget newFocus, Qt::FocusReason focusReason = Qt::OtherFocusReason); + void bookmarkColorChanged(QColor m_bookmarkColor); protected: bool event(QEvent* pEvent) override; @@ -60,8 +69,10 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void goToNextPrevBookmark(int direction); SidebarModel* m_pSidebarModel; + SidebarItemDelegate* m_pItemDelegate; QBasicTimer m_expandTimer; int m_hoverExpandDelay; QModelIndex m_hoverIndex; bool m_lastDragMoveAccepted; + QColor m_bookmarkColor; }; From 10e7e64c0148143fed3265d22b52af8f88bf5f11 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 1 Apr 2025 15:17:25 +0200 Subject: [PATCH 08/13] WLibrarySidebar: remove obsolete expand() calls, add note about scrollTo() --- src/widget/wlibrarysidebar.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 130d772a93e7..c7b786c86d06 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -317,6 +317,9 @@ void WLibrarySidebar::goToNextPrevBookmark(int direction) { } // just scroll to and highlight (focus) + // Note: scrollTo() with default hint EnsureVisible will also expand all + // parents, which in turn emits expanded() for each index which invokes + // LibraryFeature::onLazyChildExpandation(). scrollTo(bookmarkIdx); // Use this instead of setCurrentIndex() to keep current selection selectionModel()->setCurrentIndex(bookmarkIdx, QItemSelectionModel::NoUpdate); @@ -469,9 +472,6 @@ void WLibrarySidebar::selectIndex(const QModelIndex& index, bool scrollToIndex) if (selectionModel()) { selectionModel()->deleteLater(); } - if (index.parent().isValid()) { - expand(index.parent()); - } setSelectionModel(pModel); if (!scrollToIndex) { // With auto-scroll enabled, setCurrentIndex() would scroll there. @@ -481,6 +481,9 @@ void WLibrarySidebar::selectIndex(const QModelIndex& index, bool scrollToIndex) } setCurrentIndex(index); if (scrollToIndex) { + // Note: scrollTo() with default hint EnsureVisible will also expand all + // parents, which in turn emits expanded() for each index which invokes + // LibraryFeature::onLazyChildExpandation(). scrollTo(index); } else { setAutoScroll(true); @@ -504,12 +507,10 @@ void WLibrarySidebar::selectChildIndex(const QModelIndex& index, bool selectItem setCurrentIndex(translated); } - QModelIndex parentIndex = translated.parent(); - while (parentIndex.isValid()) { - expand(parentIndex); - parentIndex = parentIndex.parent(); - } - scrollTo(translated, EnsureVisible); + // Note: scrollTo() with default hint EnsureVisible will also expand all + // parents, which in turn emits expanded() for each index which invokes + // LibraryFeature::onLazyChildExpandation(). + scrollTo(translated); } QModelIndex WLibrarySidebar::selectedIndex() { From f0befc7bdf89927126dd6fc6f1d1dc3e1d94235a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Tue, 1 Apr 2025 12:03:43 +0200 Subject: [PATCH 09/13] BrowseFeature fix: don't automatically rebuild tree on each expand This can cause a crash when jumping to a SidebarBookmark in a currently collpased tree (or, with many adjustments in sidebar & tree models, at least not give the desired result). Only build the tree when there are currently no items, e.g. on initial expandation, or rebuild when `forceRebuild` argument is true. Else, if there is a mismatch between current and new items, show a refresh icon on the parent. It's always possible to enforce a tree rebuild with the menu action "Refresh directory tree". --- res/images/library/ic_library_refresh.svg | 29 ++++++++ res/mixxx.qrc | 1 + src/library/browse/browsefeature.cpp | 90 ++++++++++++++++++----- src/library/browse/browsefeature.h | 3 +- src/library/libraryfeature.h | 8 +- src/library/sidebarmodel.cpp | 7 +- 6 files changed, 113 insertions(+), 25 deletions(-) create mode 100644 res/images/library/ic_library_refresh.svg diff --git a/res/images/library/ic_library_refresh.svg b/res/images/library/ic_library_refresh.svg new file mode 100644 index 000000000000..0c4bd2ef3f61 --- /dev/null +++ b/res/images/library/ic_library_refresh.svg @@ -0,0 +1,29 @@ + + Mixxx 1.12+ iconset + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/res/mixxx.qrc b/res/mixxx.qrc index 37479f4e3bbd..4bdf2bfd48d2 100644 --- a/res/mixxx.qrc +++ b/res/mixxx.qrc @@ -25,6 +25,7 @@ images/library/ic_library_preview_pause.svg images/library/ic_library_preview_play.svg images/library/ic_library_recordings.svg + images/library/ic_library_refresh.svg images/library/ic_library_rhythmbox.svg images/library/ic_library_traktor.svg images/library/ic_library_rekordbox.svg diff --git a/src/library/browse/browsefeature.cpp b/src/library/browse/browsefeature.cpp index c2d981099ef3..d740727618fb 100644 --- a/src/library/browse/browsefeature.cpp +++ b/src/library/browse/browsefeature.cpp @@ -339,7 +339,7 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex QString path = pItem->getData().toString(); - if (path == QUICK_LINK_NODE || path == DEVICE_NODE) { + if (path == QUICK_LINK_NODE) { return; } @@ -349,9 +349,8 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex QMenu menu(m_pSidebarWidget); - if (pItem->parent()->getData().toString() == QUICK_LINK_NODE || - m_quickLinkList.contains(path)) { - // This is a QuickLink or path is in the Quick Link list + if (pItem->parent()->getData().toString() == QUICK_LINK_NODE) { + // This is a QuickLink menu.addAction(m_pRemoveQuickLinkAction); } else { menu.addAction(m_pAddQuickLinkAction); @@ -414,7 +413,7 @@ std::vector> createRemovableDevices() { // This is called whenever you double click or use the triangle symbol to expand // the subtree. The method will read the subfolders. -void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) { +void BrowseFeature::onLazyChildExpandation(const QModelIndex& index, bool enforceRebuild) { // Caution: Make sure the passed index still exists in the model. // In case it has been removed or replaced, it is still "valid", but // holds dangling internalPointer() that causes a crash. @@ -433,45 +432,98 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) { // will continue to be valid as long as they can be accessed by the model" QPersistentModelIndex idx(index); if (!idx.isValid()) { - qWarning() << "BrowseFeature::onLazyChildExpandation -> index invalid, abort."; + qWarning() << "BrowseFeature::onLazyChildExpandation -> index invalid, abort" << index; return; } TreeItem* pItem = static_cast(idx.internalPointer()); if (!(pItem && pItem->getData().isValid())) { - qWarning() << "BrowseFeature::onLazyChildExpandation -> no item or no item data, abort."; + qWarning() << "BrowseFeature::onLazyChildExpandation: no item or item data invalid"; return; } - qDebug() << "BrowseFeature::onLazyChildExpandation " << pItem->getLabel() - << " " << pItem->getData(); + qWarning() << "BrowseFeature::onLazyChildExpandation " + << pItem->getLabel() << pItem->getData().toString(); - QString path = pItem->getData().toString(); + const QString path = pItem->getData().toString(); // If the item is a built-in node, e.g., 'QuickLink' return if (path.isEmpty() || path == QUICK_LINK_NODE) { return; } - // Before we populate the subtree, we need to delete old subtrees - m_pSidebarModel->removeRows(0, pItem->childRows(), idx); - // List of subfolders or drive letters std::vector> folders; - // If we are on the special device node if (path == DEVICE_NODE) { + folders = createRemovableDevices(); + } else { + folders = getChildDirectoryItems(path); + } + + // Always remove the childrens' `hasChildren` flag.in order to + // always show the real state for child items that (still) exist. + m_pSidebarModel->removeChildDirsFromCache(QStringList{path}); + + // Build the child tree only when there are currently no folders displayed, + // eg. on initial expand, or when user click the "Refresh directory tree" action. + // If we detect changed child directories, eg. when users un/mounted devices + // or renamed, added or removed directories outside Mixxx, we show an icon + // on the changed parent item and users can refresh with the menu action. + // TODO Check if performance is an issue, and if yes, if it'd be better if + // we can get a path QStringList instead of constructing all TreeItems. + // + // Note(ronso0) The reason to avoid needless rebuild of the tree is that, + // with SidebarBookmarks, jumping to a bookmark in a collapsed tree would + // cause a crash when the tree rebuild triggered by expandation invalidates + // the index we are currently using in WLibrarySidebar. + // Note: ideally we'd reimplement QTreeView's expand() function and maybe + // others, too, in order to allow us to emit different signals for manual + // and programmatic expandation. Though, this is not an option because all + // this is done in QTreeView's private base class and therefore a no-go. + int childRows = pItem->childRows(); + if (!enforceRebuild && childRows > 0) { + bool needsUpdate = false; + if (childRows == static_cast(folders.size())) { + const auto currChildren = pItem->children(); + for (int i = 0; i < childRows; i++) { + if (currChildren[i]->getData() != folders[i].get()->getData()) { + needsUpdate = true; + break; + } + } + if (!needsUpdate) { + // Nothing to do. + // Also update children's `hasChildren` flag? + return; + } + // Else we found a count or path mismatch and rebuild the tree. + } else { + needsUpdate = true; + } + + if (needsUpdate) { + // Show a warning icon on the parent item. Users may then use the context + // menu -> "Refresh directory tree". Or click the icon?? + pItem->setIcon(QIcon(QStringLiteral(":/images/library/ic_library_refresh.svg"))); + return; + } + } + + pItem->setIcon(QIcon()); + + // Before we populate the subtree, we need to delete old subtrees + if (childRows > 0) { + m_pSidebarModel->removeRows(0, childRows, idx); + } #if defined(__LINUX__) + if (path == DEVICE_NODE) { // Tell the model to remove the cached 'hasChildren' states of all sub- // directories when we expand the Device node. // This ensures we show the real dir tree. This is relevant when devices // were unmounted, changed and mounted again. m_pSidebarModel->removeChildDirsFromCache(removableDriveRootPaths()); -#endif - folders = createRemovableDevices(); - } else { - folders = getChildDirectoryItems(path); } - +#endif if (!folders.empty()) { m_pSidebarModel->insertTreeItemRows(std::move(folders), 0, idx); } diff --git a/src/library/browse/browsefeature.h b/src/library/browse/browsefeature.h index d809406a1c0d..927b6a6ef7f1 100644 --- a/src/library/browse/browsefeature.h +++ b/src/library/browse/browsefeature.h @@ -47,7 +47,7 @@ class BrowseFeature : public LibraryFeature { void activate() override; void activateChild(const QModelIndex& index) override; void onRightClickChild(const QPoint& globalPos, const QModelIndex& index) override; - void onLazyChildExpandation(const QModelIndex& index) override; + void onLazyChildExpandation(const QModelIndex& index, bool enforceRebuild = false) override; void slotLibraryScanStarted(); void slotLibraryScanFinished(); void invalidateRightClickIndex(); @@ -82,4 +82,5 @@ class BrowseFeature : public LibraryFeature { TreeItem* m_pQuickLinkItem; QStringList m_quickLinkList; QPointer m_pSidebarWidget; + bool m_forceUpdate; }; diff --git a/src/library/libraryfeature.h b/src/library/libraryfeature.h index d1a865e8753d..0b7c9123d637 100644 --- a/src/library/libraryfeature.h +++ b/src/library/libraryfeature.h @@ -138,9 +138,13 @@ class LibraryFeature : public QObject { Q_UNUSED(index); } // Only implement this, if using incremental or lazy childmodels, see BrowseFeature. - // This method is executed whenever you **double** click child items - virtual void onLazyChildExpandation(const QModelIndex& index) { + // This is called whenever you manually expand the subtree (double click on item + // or click on the triangle icon) and after programmatic expandation with QTreeView's + // expand() or scrollTo() (with EnsureVisible hint the latter auto-expands all + // of the index' parents). + virtual void onLazyChildExpandation(const QModelIndex& index, bool enforceRebuild = false) { Q_UNUSED(index); + Q_UNUSED(enforceRebuild); } signals: void showTrackModel(QAbstractItemModel* model, bool restoreState = true); diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 190bac18b1b4..454f11853250 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -364,15 +364,16 @@ void SidebarModel::clicked(const QModelIndex& index) { } } -/// Invoked by double click and click on tree node expand icons +/// Invoked by double click on child items, click on tree node expand icons and +/// when jumping to a child item in a collapsed branch. void SidebarModel::doubleClicked(const QModelIndex& index) { stopPressedUntilClickedTimer(); if (!index.isValid()) { return; } if (index.internalPointer() == this) { - // Index is a root index and QTreeView already did expand it, so ther's - // nothing to do for us. + // Index is a root index and QTreeView already did expand it, + // so there's nothing to do for us. return; } else { TreeItem* pTreeItem = static_cast(index.internalPointer()); From c2f42bd7fa8b10ee1bcf24eaf4dc89f936d0d6f1 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 2 Apr 2025 13:14:59 +0200 Subject: [PATCH 10/13] Library sidebar: click on Refresh icon in Computer enforces tree rebuild --- src/library/browse/browsefeature.cpp | 11 ++++++--- src/library/sidebaritemdelegate.cpp | 34 ++++++++++++++++++++++++++++ src/library/sidebaritemdelegate.h | 5 ++++ src/library/sidebarmodel.cpp | 17 ++++++++++++++ src/library/sidebarmodel.h | 2 ++ src/library/treeitem.cpp | 11 +++++---- src/library/treeitem.h | 7 ++++++ 7 files changed, 79 insertions(+), 8 deletions(-) diff --git a/src/library/browse/browsefeature.cpp b/src/library/browse/browsefeature.cpp index d740727618fb..a791167564ea 100644 --- a/src/library/browse/browsefeature.cpp +++ b/src/library/browse/browsefeature.cpp @@ -464,8 +464,9 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index, bool enforc // always show the real state for child items that (still) exist. m_pSidebarModel->removeChildDirsFromCache(QStringList{path}); - // Build the child tree only when there are currently no folders displayed, - // eg. on initial expand, or when user click the "Refresh directory tree" action. + // Build the child tree only if there are currently no folders displayed, + // eg. on initial expand, if user clicked the "Refresh directory tree" action + // or left-clicked the Refresh icon. // If we detect changed child directories, eg. when users un/mounted devices // or renamed, added or removed directories outside Mixxx, we show an icon // on the changed parent item and users can refresh with the menu action. @@ -503,13 +504,17 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index, bool enforc if (needsUpdate) { // Show a warning icon on the parent item. Users may then use the context - // menu -> "Refresh directory tree". Or click the icon?? + // menu -> "Refresh directory tree". + // Or click the Refresh icon. This is handled by SidebarItemDelegate pItem->setIcon(QIcon(QStringLiteral(":/images/library/ic_library_refresh.svg"))); + pItem->setNeedsUpdate(true); return; } } + // Reset `needsUpdate` state pItem->setIcon(QIcon()); + pItem->setNeedsUpdate(false); // Before we populate the subtree, we need to delete old subtrees if (childRows > 0) { diff --git a/src/library/sidebaritemdelegate.cpp b/src/library/sidebaritemdelegate.cpp index 58192ca08940..7acb42075f13 100644 --- a/src/library/sidebaritemdelegate.cpp +++ b/src/library/sidebaritemdelegate.cpp @@ -1,5 +1,6 @@ #include "library/sidebaritemdelegate.h" +#include #include #include @@ -16,6 +17,7 @@ SidebarItemDelegate::SidebarItemDelegate( DEBUG_ASSERT(m_pSidebarModel); } +// Used to paint SidebarBookmarks void SidebarItemDelegate::paint( QPainter* pPainter, const QStyleOptionViewItem& option, @@ -34,3 +36,35 @@ void SidebarItemDelegate::paint( m_bookmarkColor); } } + +// Used to catch clicks on TreeItem icons. Implemented only for BrowseFeature +// folder items that need an update. Click does force-rebuild the child tree. +bool SidebarItemDelegate::editorEvent(QEvent* pEvent, + QAbstractItemModel* pModel, + const QStyleOptionViewItem& option, + const QModelIndex& index) { + // Only act on left click + // Note: act on release in case we implement drag'n'drop for items. + if (pEvent->type() == QEvent::MouseButtonPress) { + if (m_pSidebarModel->data(index, SidebarModel::NeedsUpdateRole).toBool()) { + QMouseEvent* pME = static_cast(pEvent); + // Right click should be be handled by WLibrarySidebar and sidebar models! + VERIFY_OR_DEBUG_ASSERT(pME->button() == Qt::LeftButton) { + return false; + } + // Check if it's a click on the icon + QStyleOptionViewItem opt = option; + initStyleOption(&opt, index); + const QWidget* pWidget = opt.widget; + const QRect iconRect = pWidget->style()->subElementRect( + QStyle::SE_ItemViewItemDecoration, + &opt, + pWidget); + if (iconRect.contains(pME->pos())) { + // Enforce update (tree rebuild) + m_pSidebarModel->updateItem(index); + } + } + } + return QStyledItemDelegate::editorEvent(pEvent, pModel, option, index); +} diff --git a/src/library/sidebaritemdelegate.h b/src/library/sidebaritemdelegate.h index c8175c1aa781..a58387087d40 100644 --- a/src/library/sidebaritemdelegate.h +++ b/src/library/sidebaritemdelegate.h @@ -25,6 +25,11 @@ class SidebarItemDelegate : public QStyledItemDelegate { } } + bool editorEvent(QEvent* pEvent, + QAbstractItemModel* pModel, + const QStyleOptionViewItem& option, + const QModelIndex& index) override; + private: SidebarModel* m_pSidebarModel; // shared_ptr? QColor m_bookmarkColor; diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 454f11853250..eb41efeb2f8f 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -309,6 +309,9 @@ QVariant SidebarModel::data(const QModelIndex& index, int role) const { return pTreeItem->getIcon(); case SidebarModel::DataRole: return pTreeItem->getData(); + case SidebarModel::NeedsUpdateRole: + // True only for BrowseFeature items that need an update + return pTreeItem->needsUpdate(); case SidebarModel::IconNameRole: // TODO: Add support for icon names in tree items default: @@ -385,6 +388,20 @@ void SidebarModel::doubleClicked(const QModelIndex& index) { } } +/// Invoked by click on Refresh icon of BrowseFeature items whose child tree +/// is outdated. Triggers force-rebuild of the chidl tree. +void SidebarModel::updateItem(const QModelIndex& index) { + stopPressedUntilClickedTimer(); + if (!index.isValid()) { + return; + } + TreeItem* pTreeItem = static_cast(index.internalPointer()); + if (pTreeItem) { + LibraryFeature* pFeature = pTreeItem->feature(); + pFeature->onLazyChildExpandation(index, true /* enforce rebuild */); + } +} + void SidebarModel::rightClicked(const QPoint& globalPos, const QModelIndex& index) { stopPressedUntilClickedTimer(); if (!index.isValid()) { diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index 32867cbda7c6..bf1d992714db 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -101,6 +101,7 @@ class SidebarModel : public QAbstractItemModel { enum Roles { IconNameRole = Qt::UserRole + 1, + NeedsUpdateRole = Qt::UserRole + 2, DataRole, }; Q_ENUM(Roles); @@ -138,6 +139,7 @@ class SidebarModel : public QAbstractItemModel { QModelIndex getNextPrevBookmarkIndex(const QModelIndex& selIndex, int direction); bool indexIsBookmark(const QModelIndex& index) const; + void updateItem(const QModelIndex& index); public slots: void pressed(const QModelIndex& index); diff --git a/src/library/treeitem.cpp b/src/library/treeitem.cpp index 91ca87c0f580..9e29a86907c8 100644 --- a/src/library/treeitem.cpp +++ b/src/library/treeitem.cpp @@ -32,11 +32,12 @@ TreeItem::TreeItem( LibraryFeature* pFeature, QString label, QVariant data) - : m_pFeature(pFeature), - m_pParent(nullptr), - m_label(std::move(label)), - m_data(std::move(data)), - m_bold(false) { + : m_pFeature(pFeature), + m_pParent(nullptr), + m_label(std::move(label)), + m_data(std::move(data)), + m_bold(false), + m_needsUpdate(false) { } TreeItem::~TreeItem() { diff --git a/src/library/treeitem.h b/src/library/treeitem.h index b5843999b353..7d92ccaa63a6 100644 --- a/src/library/treeitem.h +++ b/src/library/treeitem.h @@ -136,6 +136,12 @@ class TreeItem final { bool isBold() const { return m_bold; } + void setNeedsUpdate(bool needsUpdate) { + m_needsUpdate = needsUpdate; + } + bool needsUpdate() { + return m_needsUpdate; + } private: explicit TreeItem( @@ -159,4 +165,5 @@ class TreeItem final { QVariant m_data; QIcon m_icon; bool m_bold; + bool m_needsUpdate; }; From 478795306e917c70bf2e149a5ec5f63e6a685005 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sat, 5 Apr 2025 01:33:47 +0200 Subject: [PATCH 11/13] Library sidebar: Enter rebuilds outdated BrowseFeature child trees --- src/library/sidebarmodel.cpp | 8 ++++++++ src/library/sidebarmodel.h | 1 + src/widget/wlibrarysidebar.cpp | 18 ++++++++++++------ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index eb41efeb2f8f..8f542eae88ee 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -388,6 +388,14 @@ void SidebarModel::doubleClicked(const QModelIndex& index) { } } +bool SidebarModel::indexNeedsUpdate(const QModelIndex& index) const { + const QVariant updateData = data(index, NeedsUpdateRole); + if (updateData.isValid() && updateData.canConvert()) { + return updateData.toBool(); + } + return false; +} + /// Invoked by click on Refresh icon of BrowseFeature items whose child tree /// is outdated. Triggers force-rebuild of the chidl tree. void SidebarModel::updateItem(const QModelIndex& index) { diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index bf1d992714db..6ddbea4faf6b 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -139,6 +139,7 @@ class SidebarModel : public QAbstractItemModel { QModelIndex getNextPrevBookmarkIndex(const QModelIndex& selIndex, int direction); bool indexIsBookmark(const QModelIndex& index) const; + bool indexNeedsUpdate(const QModelIndex& index) const; void updateItem(const QModelIndex& index); public slots: diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index c7b786c86d06..641ed779d359 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -233,13 +233,19 @@ void WLibrarySidebar::renameSelectedItem() { } void WLibrarySidebar::toggleSelectedItem() { - QModelIndex index = selectedIndex(); - if (index.isValid()) { - // Activate the item so its content shows in the main library. - emit clicked(index); - // Expand or collapse the item as necessary. - setExpanded(index, !isExpanded(index)); + const QModelIndex index = selectedIndex(); + if (!index.isValid()) { + return; + } + // Activate the item so its content shows in the main library. + emit clicked(index); + // Update child tree of BrowseFeature items with outdated tree + if (m_pSidebarModel->indexNeedsUpdate(index)) { + m_pSidebarModel->updateItem(index); + return; } + // Expand or collapse the item as necessary. + setExpanded(index, !isExpanded(index)); } bool WLibrarySidebar::isLeafNodeSelected() { From d1249758729da964ecb8f3a30112e11d4fb114c4 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Wed, 5 Nov 2025 01:25:08 +0100 Subject: [PATCH 12/13] Library Sidebar: store Bookmarks in config + restore TODO polish, optimize data --- src/library/library.cpp | 10 +- src/library/sidebarmodel.cpp | 180 ++++++++++++++++++++++++++++++++++- src/library/sidebarmodel.h | 25 ++++- 3 files changed, 208 insertions(+), 7 deletions(-) diff --git a/src/library/library.cpp b/src/library/library.cpp index 36710fc999cc..79366845cbb1 100644 --- a/src/library/library.cpp +++ b/src/library/library.cpp @@ -267,9 +267,17 @@ Library::Library( m_editMetadataSelectedClick = m_pConfig->getValue( kEditMetadataSelectedClickConfigKey, kEditMetadataSelectedClickDefault); + + if (m_pSidebarModel && m_pConfig) { + m_pSidebarModel->loadBookmarksFromConfig(m_pConfig); + } } -Library::~Library() = default; +Library::~Library() { + if (m_pSidebarModel && m_pConfig) { + m_pSidebarModel->saveBookmarksToConfig(m_pConfig); + } +} TrackCollectionManager* Library::trackCollectionManager() const { // Cannot be implemented inline due to forward declarations diff --git a/src/library/sidebarmodel.cpp b/src/library/sidebarmodel.cpp index 8f542eae88ee..219e3c0d7a58 100644 --- a/src/library/sidebarmodel.cpp +++ b/src/library/sidebarmodel.cpp @@ -638,11 +638,12 @@ void SidebarModel::slotFeatureSelect(LibraryFeature* pFeature, } void SidebarModel::toggleBookmarkByIndex(const QModelIndex& index) { - qWarning() << " toggleBookmarkByIndex" << index; if (!index.isValid()) { + // qWarning() << "toggleBookmarkByIndex ! index invalid" << index; return; } + qWarning() << "toggleBookmarkByIndex" << index; SidebarBookmark bookmark = createBookmarkFromIndex(index); if (m_bookmarks.contains(bookmark)) { // Remove bookmark and index @@ -663,13 +664,17 @@ QModelIndexList SidebarModel::sortBookmarksUpdateIndices(QList* // Update indices. Only add valid indices -- invalid means the bookmark // wasn't found after last child model update. QModelIndexList newBookmarkIndices; + // qWarning() << "Updating Sidebar bookmark indices"; for (const auto& bm : std::as_const(*pBookmarks)) { if (bm.index.isValid()) { + // qWarning() << " valid bm index, added" << bm.index; newBookmarkIndices.append(bm.index); + } else { + // qWarning() << " ! bm index invalid" << bm.index; } } + // qWarning() << "Sidebar bookmark indices updated"; return newBookmarkIndices; - qDebug() << "Sidebar bookmarks updated"; } QModelIndex SidebarModel::getNextPrevBookmarkIndex(const QModelIndex& currIndex, int direction) { @@ -687,6 +692,7 @@ QModelIndex SidebarModel::getNextPrevBookmarkIndex(const QModelIndex& currIndex, if (m_bookmarkIndices.size() == 1 && currIndex == m_bookmarkIndices[0]) { // We already have the only bookmark selected. // Return invalid index so the sidebar does not reload the current view. + qWarning() << "SidebarModel::getNextPrevBookmarkIndex: already on only bookmark"; return {}; } @@ -791,8 +797,9 @@ QModelIndex SidebarModel::getNextPrevBookmarkIndex(const QModelIndex& currIndex, // This is now fixed, but I didn't check with other features, hence can't guarantee // singular/safe access. void SidebarModel::maybeUpdateBookmarkIndices(const QModelIndex& parentIndex) { - // qWarning() << "maybeUpdateBookmarkIndices" << parentIndex; + qWarning() << "maybeUpdateBookmarkIndices" << parentIndex; if (m_bookmarkIndices.isEmpty()) { + qWarning() << " ! bm indices empty"; return; } // Collect the start parameters for findBookmarkIndex() @@ -812,6 +819,7 @@ void SidebarModel::maybeUpdateBookmarkIndices(const QModelIndex& parentIndex) { if (bm.featureRow != featureRow) { continue; } + // qWarning() << " -> look for" << bm; needsUpdate = true; // Lookup bookmark. // If found, replace with fresh bookmark, ie. update all properties at once. @@ -857,6 +865,7 @@ QModelIndex SidebarModel::findBookmarkIndex(const SidebarBookmark& bookmark) { const QModelIndex rootIndex = index(bookmark.featureRow, 0); QModelIndexList results; if (bookmark.data.isValid() && pFeature->isItemDataUnique(bookmark.data)) { + // qWarning() << " -> child data of" << pFeature->title() << "is unique"; // Search for matching data results = pChildModel->match( rootIndex, @@ -865,6 +874,7 @@ QModelIndex SidebarModel::findBookmarkIndex(const SidebarBookmark& bookmark) { 1, Qt::MatchWrap | Qt::MatchExactly | Qt::MatchRecursive); } else { + // qWarning() << " -> child data of" << pFeature->title() << "is NOT unique"; // Search for label match. // This covers root items, Tracks Missing/Hidden, AutoDJ Crates // and History's YEAR nodes. @@ -877,8 +887,10 @@ QModelIndex SidebarModel::findBookmarkIndex(const SidebarBookmark& bookmark) { } if (!results.isEmpty()) { + // qWarning() << " -> result found"; return results.front(); } + // qWarning() << " -> no result found, abort"; return {}; } @@ -887,9 +899,10 @@ SidebarBookmark SidebarModel::createBookmarkFromIndex(const QModelIndex& index) return {}; } - // qWarning() << " >> getBookmarkFromIndex" << index; + qWarning() << "createBookmarkFromIndex" << index; SidebarBookmark bookmark; if (index.internalPointer() == this) { + // qWarning() << "-> int.pointer = this:" << index.internalPointer(); LibraryFeature* pFeature = m_sFeatures[index.row()]; bookmark = SidebarBookmark( index.row(), @@ -901,6 +914,7 @@ SidebarBookmark SidebarModel::createBookmarkFromIndex(const QModelIndex& index) pFeature->iconName(), index); } else { + // qWarning() << "-> int.pointer != this:" << index.internalPointer(); TreeItem* pTreeItem = static_cast(index.internalPointer()); VERIFY_OR_DEBUG_ASSERT(pTreeItem) { return {}; @@ -910,9 +924,11 @@ SidebarBookmark SidebarModel::createBookmarkFromIndex(const QModelIndex& index) bookmark.parentRow = pTreeItem->parentRow(); const auto& data = pTreeItem->getData(); if (data.isValid() && pTreeItem->isDataUniqueInFeature()) { + // qWarning() << "-> data valid:" << data; bookmark.data = data; } else { bookmark.label = pTreeItem->getLabel(); + // qWarning() << "-> data invalid, use label:" << bookmark.label; } bookmark.index = index; } @@ -925,5 +941,161 @@ bool SidebarModel::indexIsBookmark(const QModelIndex& index) const { if (!index.isValid()) { return false; } + if (m_bookmarkIndices.contains(index)) { + // qWarning() << "--* indexIsBookmark?" << index << index.internalPointer(); + } return m_bookmarkIndices.contains(index); } + +void SidebarModel::saveBookmarksToConfig(UserSettingsPointer pConfig) { + if (pConfig == nullptr || m_bookmarks.isEmpty()) { + return; + } + + // Bookmark identifiers are either + // [item] [value] + // int 0 = feature root item + // └> feature row index + // int-int-int string = child item + // | | | └> data of TreeItem, int-based id, or QString + // | | └> data type: 0 data as QVariant(int) + // | | 1 data a QVariant(QString) + // | └> child level + // └> feature row index + + // qWarning() << "."; + // qWarning() << "save bookmarks to config:"; + const QString group = QStringLiteral("[SidebarBookmarks]"); + for (const auto& bm : std::as_const(m_bookmarks)) { + if (bm.childLevel == 0) { // feature root item + // qWarning() << " root item" << bm.label; + pConfig->setValue(ConfigKey(group, QString::number(bm.featureRow)), QString("---")); + } else { // child item + // qWarning() << " child item" << bm; + // check data type of feature + QVariant idxData = data(bm.index, SidebarModel::DataRole); + if (!idxData.isValid() || idxData.isNull()) { + // each child item must have data + // qWarning() << " ! invalid data"; + continue; + } + int dataType = -1; + QStringList dataStrList; + QString data; + if (idxData.canConvert()) { + bool okay = false; + int dataInt = idxData.toInt(&okay); + if (okay) { + // qWarning() << " -> int data:" << dataInt; + dataType = 0; + data = QString::number(dataInt); + } + } + if (dataType == -1) { + if (idxData.canConvert()) { + dataType = 1; + data = idxData.toString(); + // qWarning() << " -> string data:" << data; + } else { + // qWarning() << " ! unknown data type:" << idxData; + continue; + } + } + dataStrList << QString::number(bm.featureRow); + dataStrList << QString::number(bm.childLevel); + dataStrList << QString::number(dataType); + const QString item = dataStrList.join('-'); + // qWarning() << " save bookmark as" << item << data; + pConfig->setValue(ConfigKey(group, item), data); + } + } +} + +void SidebarModel::loadBookmarksFromConfig(UserSettingsPointer pConfig) { + if (pConfig == nullptr) { + return; + } + + // Bookmark identifiers are either + // [item] [value] + // int 0 = feature root item + // └> feature row index + // int-int-int string = child item + // | | | └> data of TreeItem, int-based id, or QString + // | | └> data type: 0 data as QVariant(int) + // | | 1 data a QVariant(QString) + // | └> child level + // └> feature row index + + // qWarning() << "."; + // qWarning() << "read bookmarks from config:"; + + const QList bookmarkKeys = + pConfig->getKeysWithGroup(QStringLiteral("[SidebarBookmarks]")); + for (const auto& bookmarkKey : bookmarkKeys) { + // qWarning() << " " << i << bookmarkKey.item << pConfig->getValueString(bookmarkKey); + const QStringList dataStrList = bookmarkKey.item.split('-', Qt::SkipEmptyParts); + if (dataStrList.size() == 1) { // root item + bool okay = false; + int fRow = dataStrList[0].toInt(&okay); + if (okay && fRow >= 0 && fRow < m_sFeatures.count()) { + toggleBookmarkByIndex(createIndex(fRow, 0, this)); + // Remove now so we don't pile up bookmarks that we removed in session + pConfig->remove(bookmarkKey); + } + } else if (dataStrList.size() == 3) { // child item + bool allInt = false; + int fRow = dataStrList[0].toInt(&allInt); + int chLevel = dataStrList[1].toInt(&allInt); + int dataType = dataStrList[2].toInt(&allInt); + if (!allInt || fRow < 0 || chLevel < 0 || (dataType != 0 && dataType != 1)) { + // invalid, ignore + continue; + } + const QString dataStr = pConfig->getValueString(bookmarkKey).trimmed(); + if (dataStr.isEmpty()) { + continue; + } + SidebarBookmark bm( + fRow, + chLevel, + 0, + QVariant(), + QString()); + if (dataType == 0) { // QVariant(int) + bool okay = false; + int dataInt = dataStr.toInt(&okay); + if (!okay) { + continue; + } + bm.data = QVariant(dataInt); + } else if (dataType == 1) { // QVariant(QSring) + bm.data = QVariant(dataStr); + } + // qWarning() << " created Bookmark" << bm; + // We now have an incomplete Bookmark (index is missing), so we + // do the same round trip like when we update bookmarks after + // the model has changed (rows added, removed) + QModelIndex bmIdx = findBookmarkIndex(bm); + if (!bmIdx.isValid()) { + // This may happen if we bookmarked a Computer folder item which + // has not been added to the tree yet due to lazy tree population + // on expand. + // Let's store the bookmark anyway so we can look it up later on + // and maybe set its index. + m_bookmarks.append(bm); + // qWarning() << " - no index found, skip"; + continue; + } + // qWarning() << " - found Bookmark index:" << bmIdx; + // qWarning() << " - index data:" << data(bmIdx, Qt::DisplayRole); + toggleBookmarkByIndex(translateChildIndex(bmIdx)); + // Remove now so we don't pile up bookmarks that we removed in session + pConfig->remove(bookmarkKey); + } else { + // invalid + continue; + } + } + return; +} diff --git a/src/library/sidebarmodel.h b/src/library/sidebarmodel.h index 6ddbea4faf6b..97b4dbe07c21 100644 --- a/src/library/sidebarmodel.h +++ b/src/library/sidebarmodel.h @@ -5,6 +5,8 @@ #include #include +#include "preferences/usersettings.h" + class LibraryFeature; class QTimer; class TreeItem; @@ -23,14 +25,17 @@ struct SidebarBookmark { data(datavar), label(sLabel), index(dIndex) { + // How to? + // qWarning() << "------- created" << this; } bool isValid() const { // qDebug() << " SidebarBookmark isValid()"; + // Don't validate the index. It may still be invalid, eg. when we + // read bookmarks from config return featureRow >= 0 && childLevel >= 0 && parentRow >= 0 && - (data.isValid() || !label.isEmpty()) && - index.isValid(); + (data.isValid() || !label.isEmpty()); } bool operator==(const SidebarBookmark& other) const { // qDebug() << " SidebarBookmark==op"; @@ -75,6 +80,19 @@ struct SidebarBookmark { // for getNextPrevBookmarkIndex(). // Must be updated when the child model is changed. QModelIndex index; + + // TODO How to store Bookmarks in config? + // [SidebarBookmarks] + // 3 /home/user/Music/SomeAlbum + // [featureRow] [dataOrLabel] + // dataOrLabel can be either int (int for playlist or CrateId which is + // converted to QVariant(int) so I could rework/duplicate + // findBookmarkIndex() and either + // * check data type of a random item of the feature + // -> could work. check History + // * try to convert dataLabel to int, if okay + // * try to find QVariant(intValue) + // * if not okay or int search fails, try QString }; inline QDebug operator<<(QDebug dbg, const SidebarBookmark& bm) { @@ -138,6 +156,9 @@ class SidebarModel : public QAbstractItemModel { void toggleBookmarkByIndex(const QModelIndex& selIndex); QModelIndex getNextPrevBookmarkIndex(const QModelIndex& selIndex, int direction); + void saveBookmarksToConfig(UserSettingsPointer pConfig); + void loadBookmarksFromConfig(UserSettingsPointer pConfig); + bool indexIsBookmark(const QModelIndex& index) const; bool indexNeedsUpdate(const QModelIndex& index) const; void updateItem(const QModelIndex& index); From fed92c91eda3cdb3b44eee45ab42ebf5df612b7a Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 6 Nov 2025 16:38:02 +0100 Subject: [PATCH 13/13] Library sidebar: add Bookmark controls prev/next/select --- src/library/librarycontrol.cpp | 41 ++++++++++++++++++++++++++++++++++ src/library/librarycontrol.h | 4 ++++ src/widget/wlibrarysidebar.cpp | 4 ++-- src/widget/wlibrarysidebar.h | 2 +- 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/library/librarycontrol.cpp b/src/library/librarycontrol.cpp index efac7b356b0b..8f8244746c7e 100644 --- a/src/library/librarycontrol.cpp +++ b/src/library/librarycontrol.cpp @@ -524,6 +524,47 @@ LibraryControl::LibraryControl(Library* pLibrary) this, &LibraryControl::slotLoadSelectedIntoFirstStopped); + m_pBookmarkNext = std::make_unique( + ConfigKey("[Library]", "bookmark_next")); + connect(m_pBookmarkNext.get(), + &ControlPushButton::valueChanged, + this, + [this](double value) { + VERIFY_OR_DEBUG_ASSERT(m_pSidebarWidget) { + return; + } + if (value > 0) { + m_pSidebarWidget->slotGoToNextPrevBookmark(1); + } + }); + m_pBookmarkPrev = std::make_unique( + ConfigKey("[Library]", "bookmark_prev")); + connect(m_pBookmarkPrev.get(), + &ControlPushButton::valueChanged, + this, + [this](double value) { + VERIFY_OR_DEBUG_ASSERT(m_pSidebarWidget) { + return; + } + if (value > 0) { + m_pSidebarWidget->slotGoToNextPrevBookmark(-1); + } + }); + m_pBookmarkSelect = std::make_unique( + ConfigKey("[Library]", "bookmark_selector"), false); + connect(m_pBookmarkSelect.get(), + &ControlEncoder::valueChanged, + this, + [this](double steps) { + VERIFY_OR_DEBUG_ASSERT(m_pSidebarWidget) { + return; + } + int iSteps = static_cast(steps); + if (iSteps) { + m_pSidebarWidget->slotGoToNextPrevBookmark(static_cast(steps)); + } + }); + #ifdef MIXXX_USE_QML if (!CmdlineArgs::Instance().isQml()) #endif diff --git a/src/library/librarycontrol.h b/src/library/librarycontrol.h index ecf6e1370bc9..ac488cc5fa57 100644 --- a/src/library/librarycontrol.h +++ b/src/library/librarycontrol.h @@ -216,6 +216,10 @@ class LibraryControl : public QObject { std::unique_ptr m_pToggleSidebarItem; std::unique_ptr m_pLoadSelectedIntoFirstStopped; + std::unique_ptr m_pBookmarkNext; + std::unique_ptr m_pBookmarkPrev; + std::unique_ptr m_pBookmarkSelect; + // Library widgets WLibrary* m_pLibraryWidget; WLibrarySidebar* m_pSidebarWidget; diff --git a/src/widget/wlibrarysidebar.cpp b/src/widget/wlibrarysidebar.cpp index 641ed779d359..4cd05970d3b2 100644 --- a/src/widget/wlibrarysidebar.cpp +++ b/src/widget/wlibrarysidebar.cpp @@ -305,7 +305,7 @@ void WLibrarySidebar::toggleBookmark() { update(); } -void WLibrarySidebar::goToNextPrevBookmark(int direction) { +void WLibrarySidebar::slotGoToNextPrevBookmark(int direction) { // Don't use selectedIndex(). Selected item may not be the focused item, eg. // if we focused a bookmark item without activating it. QModelIndex index = currentIndex(); @@ -353,7 +353,7 @@ void WLibrarySidebar::keyPressEvent(QKeyEvent* pEvent) { // Press Enter to activate if (pEvent->modifiers().testFlag(Qt::AltModifier)) { if (pEvent->key() == Qt::Key_Down || pEvent->key() == Qt::Key_Up) { - goToNextPrevBookmark(pEvent->key() == Qt::Key_Down ? 1 : -1); + slotGoToNextPrevBookmark(pEvent->key() == Qt::Key_Down ? 1 : -1); } else if (pEvent->key() == Qt::Key_B) { toggleBookmark(); } diff --git a/src/widget/wlibrarysidebar.h b/src/widget/wlibrarysidebar.h index 22d5a4eef8ae..f150620e27f8 100644 --- a/src/widget/wlibrarysidebar.h +++ b/src/widget/wlibrarysidebar.h @@ -46,6 +46,7 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void selectChildIndex(const QModelIndex&, bool selectItem = true); void slotSetFont(const QFont& font); void slotSetExpandOnHoverDelay(int delay); + void slotGoToNextPrevBookmark(int direction); signals: void rightClicked(const QPoint&, const QModelIndex&); @@ -66,7 +67,6 @@ class WLibrarySidebar : public QTreeView, public WBaseWidget { void toggleDragHoverPropertyAndUpdateStyle(bool enabled); void resetHoverIndexAndDragMoveResult(); void toggleBookmark(); - void goToNextPrevBookmark(int direction); SidebarModel* m_pSidebarModel; SidebarItemDelegate* m_pItemDelegate;