(feat) add Library sidebar bookmarks#14508
Conversation
d00f19b to
85479f9
Compare
85479f9 to
3c73c1f
Compare
| results = pChildModel->match( | ||
| pChildModel->getRootIndex(), | ||
| TreeItemModel::kDataRole, | ||
| bookmark.data, | ||
| 1, | ||
| Qt::MatchWrap | Qt::MatchExactly | Qt::MatchRecursive); |
There was a problem hiding this comment.
I discovered by chance QAbstractItemModel::match() after I wrote my own recursive match() function that checked label and/or data 😬 🤦♂️ 😆
3c73c1f to
874604f
Compare
eb7fc7c to
cac2cdc
Compare
cac2cdc to
16537f6
Compare
|
I think it's now safe to say this is ready for real-life testing. |
b3a2170 to
3776151
Compare
3776151 to
da0ed8f
Compare
|
Yeah, ready for review! |
da0ed8f to
1a778fd
Compare
|
Bookmarks are now persistent (I really wanted to avoid the redundant steps when starting a session). |
d4d29c0 to
546e26d
Compare
|
I have not tested yet, I just stumbled about the refresh icon in two ways. Maybe because I have not fully understand the workflow.
|
|
It's been a while since I did this, so I'm not entirely sure. I tried to explain the main issue with recalling bookmarks in Computer here
I compare the child count only (actual on disk vs. currently in tree), if mismatch show the icon/button. That's one of the situations where I tried to improve Computer, or at least hack around its flaws. My design and UI decisions may not be the best, happy to receive proposals. And after all, this is about the Computer library view, so if there are file / dir changes you'd need to rescan anyway. And, in my experience at least, significant file / directory modifications are very rare while playing. |
|
Btw #15571 or similar is required to make this feature work satisfactorily. |
|
Nice, I managed to also restore Computer bookmarks. Will update this branch soon. |
546e26d to
aada682
Compare
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.
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.
Alt+B toggles bookmark flag Alt+Up/Down jumps to prev/next (highlighted, not activated) Enter to activate
color can be set in qss with
WLibrarySidebar {
qproperty-bookmarkColor: #451231;
}
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".
a4ab825 to
b3e75a6
Compare
b3e75a6 to
fed92c9
Compare
Just recently I needed this again. I was doing a b2b session with a guest who had his music in various directories on a thumb drive. Switching between my and his tracks views was annoying.
Now it's just
Alt+Up/Down😁The bookmark indicator alone is helpful if you want to only visually mark sidebar items, eg. some favorite History playlist or some directory in a huge list, without the next/prev functions.
Related feature request is #9912
Note: for easy testing I have included all fixes from #14556.
How to use
Alt+Bto bookmark the selected sidebar item (or remove bookmark)Alt+Up/Downto go to and highlight the previous/next bookmark.bookmarks are sorted by position in the tree, so Down really jumps to the next bookmark below.
(note: missing bookmark items are skipped, but not removed from the list)
-> this does not activate (load) the bookmark view
-> useful for scrolling to a bookmark in order to drag tracks from the current view / from decks onto it
-> press
Enterto activate itQuick demo, with and without activating the bookmark

(edit: auto-activation has now been disabled, see above)
Implementation
New struct `SidebarBookmark` holds all info to identify item: * data/label to find it in the tree * feature row, child level and parent row to sort bookmarks by position in the tree * feature row is also used to check only indices of the changed feature when the sidebar is updatedAll bookmarked indices are stored QModelIndexList. This functions as quick lookup list for SidebarModel to paint the bookmarks.
Bookmarks and this index list are updated when the sidebar changed, eg. when playlists were added, removed etc. (when child tree models are rebuilt). By boomark data, we try to find the item in the updated tree, then store the new index and update the index list.
-- btw that was the real challenge here. If the sidebar is unchanged (ie. only labels (track count) are changed) it works by simply maintaining a QModelIndexList, but with potentially changing indices we need to store items' metadata to identify them later on and get their new indices.
New
SidebarItemDelegatedoes the painting, and also detects left-click on Refresh icons.Some new
TreeItemmembers to help withBrowseFeatureupdates etc.New extra feature / fix / slightly disruptive UI change
Recalling bookmarks in currently collapsed Computer trees causes a crash because the directory trees are rebuilt on expandation 1.

Now, the child trees are only build on initial expandation. On next re-expandation we check if child directories changed.
If yes, a refresh icon is added to indicate changes.
Trees can be rebuild on demand, either by clicking the Refresh icon or with the existing "Refresh directory tree" context menu action.
Note that this is noticeable only if the if child directories changed, so it's actually not that disruptive IMO
TODO
Known Issues
Nice to have
I'd go with the config. I'm lazy and a
[Bookmarks]group would do, keys are the [feature] names and values are comma-separated ids/paths 🤷like hotcue buttons, click to set/recall, right-click to clear
requires a Bookmark row or something..
Footnotes
When switching to a bookmark in a collapsed tree, WLibrarySidebar calls
QTreeView::scrollTo(index). This expands all the index' parents from current to top level. While doing this, QTreeView'sexpandedsignal triggersBrowseFeature::onLazyChildExpandationwhich concurrently rebuilds the child tree, which emits signals to update the tree view and its model.Apparently we run into an QModelIndex or its TreeItem pointer in
SidebarModel::parentthat becomes invalid while we process it.Here's the backtrace ↩