Per crate sorting #2046
Per crate sorting #2046
Conversation
There was a problem hiding this comment.
I think this is still work in progress, right.
Please add the [WIP] prefix to the PR title.
I am a worrying about all the additional emit calls required. How do we make sure that we don't forget one?
It would be great if we can find a single place deeper in the call stack where it can be called. Did you already had a look for it?
Alternative we may introduce a wrapper that wraps storing and changing to a new model.
Do we need to consider two or more sittings for a single model? This might be relevant if we are able to show the same model side by side in two pane view.
|
thanks for these notes. I'll look again for a better place to send the signal from, but I couldn't find it. I left a long comment in the bug explaining why some obvious simpler options didn't work. |
The method I used was to find all subclasses of LibraryFeature and instrument the activate* methods. The activate functions are called directly (for example, in SidebarModel::doubleClicked) so there's no existing signal I can hook in to. One possibility might be to have libraryfeature define a base version of Activate from which children inherit, but we'd still have to make sure that the children properly call the base class version before doing their own work. Another possibility would be to have the callers of the Activate functions also call saveHeaderState, but they don't own the header and this would also be easy to mess up if someone adds a new way to activate a crate and they forget to emit the signal. If there's somewhere else these could go I am all ears ;) |
|
I finally realized we only need to explicitly save the view state when we are activating a crate, not any of the other ones. So I can remove the signal emits from all the other places in the code. I added comments to explain. |
|
Btw, the Serato feature has been merged. You can test it with the sample files I provided here: #2480 (comment) |
| void WTrackTableView::saveViewState() { | ||
| // We save the header state when a track model requests it. | ||
| WTrackTableViewHeader* pHeader = | ||
| dynamic_cast<WTrackTableViewHeader*>(horizontalHeader()); |
| @@ -266,12 +275,26 @@ void WTrackTableView::loadTrackModel(QAbstractItemModel *model) { | |||
| * there's no need to exchange the headers | |||
| * this will cause a small GUI freeze | |||
| */ | |||
| WTrackTableViewHeader* oldHeader = | |||
src/library/crate/cratefeature.cpp
Outdated
| @@ -298,6 +298,8 @@ void CrateFeature::activateChild(const QModelIndex& index) { | |||
| VERIFY_OR_DEBUG_ASSERT(crateId.isValid()) { | |||
| return; | |||
| } | |||
| // We need to save the header state now, before we activate a new crate and change track IDs. | |||
| emit(saveViewState()); | |||
There was a problem hiding this comment.
emit() is deprecated and will trigger a clazy warning (see also below)
|
This patch is very similar to #3063 in case of implemented code. I can add the sorting column there as well, that way all subsystems will remember sorting per view |
|
that would be really great -- my patch is sort of abandonned because I could never do tests with serato / rekordbox |
Use the model settings to store and restore table header state for each crate/playlist/etc. Reimplementation of mixxxdj#2046
|
@ywwg done. Wanna give it a try ? |
|
This PR is marked as stale because it has been open 90 days with no activity. |
github-actions
bot
added
the
stale
|
superseded by #3063 |
https://bugs.launchpad.net/mixxx/+bug/1818342
Implements a method to save columns and sort order on a per-crate basis. This change introduces a new signal, unshowTrackModel, that is called by library features before they do work in preparation to do a showtrackmodel. This gives the table view a chance to save state before everything changes.
It's been a while since I've done QT signal/slot stuff, comments welcome!