(feat) add Library sidebar bookmarks

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

res/mixxx.qrc

@@ -25,6 +25,7 @@
         <file>images/library/ic_library_preview_pause.svg</file>
         <file>images/library/ic_library_preview_play.svg</file>
         <file>images/library/ic_library_recordings.svg</file>
+        <file>images/library/ic_library_refresh.svg</file>
         <file>images/library/ic_library_rhythmbox.svg</file>
         <file>images/library/ic_library_traktor.svg</file>
         <file>images/library/ic_library_rekordbox.svg</file>

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,
@@ -3019,15 +3021,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,
@@ -3354,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,

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<std::unique_ptr<TreeItem>> 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,103 @@ 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<TreeItem*>(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<std::unique_ptr<TreeItem>> 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 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.
+    // 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<int>(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 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) {
+        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);
     }

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<WLibrarySidebar> m_pSidebarWidget;
+    bool m_forceUpdate;
 };

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<TrackId> playedTrackIds;
-    if (getHiddenType(playlistId) == PLHT_SET_LOG) {
+    PlaylistDAO::HiddenType type = getHiddenType(playlistId);
+    if (type == PLHT_SET_LOG) {
         const QList<TrackId> trackIds = getTrackIds(playlistId);
 
         // TODO: QSet<T>::fromList(const QList<T>&) 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;
 }
 

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<int>& 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<int>& playlistIds);
     // Separate signals for PlaylistTableModel
     void tracksAdded(const QSet<int>& playlistIds);

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