WTrackMenu: Add ability to select loaded track in library -- fixup/tweak

[ronso0]

First of all thanks @ninomp for starting #4688
This PR is built on top of that. I opened this PR against the Mixxx repo instead of @ninomp's to get more feedback.

I looked into the issue we discovered: the requested track is not selected in AutoDJ, Recording and Analyze. Though, the select request goes to the previously selected tracks view (library cover art is updated) and thus clears track selection there. The same happens when the action is triggered while a feature root with no tracks table is selected.
Reason was that AutoDJ, Recording and Analyze do not emit showTrackModel when being activated. I fixed that.
(note that the requested track will still not be selected in Recording and Browse since BrowseTableModel does currently not support getTrackRows())

However, the issue remained with feature root views that don't have a tracks table (Playlist, Crates, Browse). This 'user error' situation may be rare, but I prefer to avoid it.

Also, UX would be improved if 'Select track in library' would be selectable only if the track actually can be selected.

Both aspects are covered by disabling the menu action checking if a) the current view is not a tracks table or b) it doesn't contain the track (not at all, or due to filtering).
Looked into it and it seemed doable -- so I just tried it,. and it was way easier than I expected : )

I think there's still a chance to reproduce the issue: open the track menu, then change the sidebar selection with [Playlist], Select.. from a controller, then click the 'Select track..' action.

I did not yet touch the external library features -- I don't use them and I don't know how they work internally. I'd appreciate if someone else takes care testing.

[daschuer]

Thank you for picking this up.
Normally we issue a PR to the original contributors branch, that they are still in force to control the direction of their PR.
@ninomp how do you think about it? Is it OK to hand your PR over to @ronso0 and close your own one or do you have plans to revive your PR, merging this to your branch?

[ronso0]

I don't want to hijack that PR, therefore I wrote

I opened this PR against the Mixxx repo instead of @ninomp's to get more feedback.

and if Nino is able & motivated to continue his work he can simply pick my commits. (marked this as draft)
It's just I noticed that there's rather low contributor activity lately and repeatedly some PRs stalled because of missing response after review. For that case this PR is visible in mixxxdj/mixxx and could be merged independently.

[ninomp]

Hi guys! First, I would like to thank you for helping me. Secondly, I am currently struggling to find time for Mixxx. Personally, I would like us to work together to finish this. Having said that, what is left to do here?
P.S. I am currently testing this.

[ronso0]

Having said that, what is left to do here?

as written in the PR description

I did not yet touch the external library features -- I don't use them and I don't know how they work internally. I'd appreciate if someone else takes care testing.

I only briefly checked the :.activate slots of some external features and they are emitting showTrackModel. Some other features start loading the ext. databases in that slot and I didn't follow the signal flow there. Someone needs to test this.

[daschuer]

I have rechecked my original findings:

Can you also scroll to the selected track if it is already selected.

Works

It would be also nice, if the current would be selected in case you have searched a similar track.

Not working

The feature dos not work in case AutoDJ is selected. In this case the cover art is shown regardless of the track in in the AutoDJ List or not.

Now the menu entry is grayed out. OK
But why not jump to the tracks table, like we do in when searching fro similar tracks?
This would be a workaround until we implement searching in crates for both features.

My expectation was that this feature works for the currently selected library table. Helpfull to see the current track in the context of the others in a crate or library.

I have played with this a bit and I guess the uses case is quite similar to the "Search for similar" instead of search for compatible tracks:

For instance, instead of searching for compatible key ~key:"8B I can also just find the track and sort for the key column. This gives me actually a much better overview because I can scroll to wherever I want.

Instead the compatibility search feature has the purpose of a search query composer (only). It would be a game changer if it would be a recursive menu.

[ronso0]

My expectation was that this feature works for the currently selected library table. Helpfull to see the current track in the context of the others in a crate or library.

I have played with this a bit and I guess the uses case is quite similar to the "Search for similar" instead of search for compatible tracks:

So you expect the searchbox to be cleared to enforce showing the track?

The feature dos not work in case AutoDJ is selected. In this case the cover art is shown regardless of the track in in the AutoDJ List or not.

Now the menu entry is grayed out. OK
But why not jump to the tracks table, like we do in when searching fro similar tracks?
This would be a workaround until we implement searching in crates for both features.

In case the track was loaded from the AutoDJ queue it is removed from the queue (unless re-queue is enabled) so this would obviously only find the duplicate.
You expect the library switches to Tracks? And (if the track is not visble) clear the searchbox to enforce showing the track?
For my feeling this is to much automation. The searchbox should only be cleared if users explicitely presses the Clear button or pick some action from the "Search similiar tracks" menu.

[daschuer]

So you expect the searchbox to be cleared to enforce showing the track?

No, I did not expect it. I original expected that the track is shown in the current track table and only if it is already part of it. However I don't mind to turn this feature in any direction if we have a good reason for it.

[daschuer]

The switch/gray out idea in the AutoDj use case question, is special. What would be the best responds that serves the user intend the best ...

[ronso0]

The feature dos not work in case AutoDJ is selected. In this case the cover art is shown regardless of the track in in the AutoDJ List or not.

Now the menu entry is grayed out. OK But why not jump to the tracks table, like we do in when searching fro similar tracks?

I don't understand. which tracks table do you refer to? The queue or "Tracks" feature?

[daschuer]

In this case I mean the Tracks feature.

[ronso0]

There are two aspects:

• highlight the track when searching similiar tracks
  That would be nice if "Search similiar..." is invoked from decks. Though it's out of scope here IMO.
  (file a bug so we don't forget about it)
• from AutoDJ, go to Tracks and try to select the track
  If users are looking for new tracks for the AutoDJ queue they'd need to be in Tracks anyway (or crates or any other tracks view), so I'm reluctant to make an exception for AutoDJ and add this automation. I think we should keep the implementation consistent.

[ronso0]

Argh, the controls row for AutoDJ, Recording and Analyze is gone after 9aac4c6 😬

I'll look into it, seems I didn't understand all the track view mechanics, yet.

[daschuer]

I have filed:
https://bugs.launchpad.net/mixxx/+bug/1972116
https://bugs.launchpad.net/mixxx/+bug/1972117

[daschuer]

What is the main use case for this new feature?

Since you can already see/and edit the track properties via "Properties", there is only the "Find similar tracks" use case left. It this correct, or do I overlook something?

So it is worth to consider to join the feature with "Search related Tracks".

What do you think?

[ronso0]

What is the main use case for this new feature?

I don't have a real need for this feature currently. Though, after I browsed the list and scrolled too far to have the track in sight I'd use it to scroll back to the track in the current view.

[ronso0]

@ninomp What is your use case?

[ronso0]

Argh, the controls row for AutoDJ, Recording and Analyze is gone after 9aac4c6 grimacing

emit showTrackModel() is completely wrong for the special features, that would make WLibrary switch to the main (common) WTrackTableView and load the special trackmodel there, thus switching away from the special feature Dlg 🤦‍♂️

So this is the wrong approach in the first place
db9cc69#diff-4232cb0ca12d2815afd223231c3ff938356957d8f5e3bd2ce9999d77bd6cbba9R403
and my commits building on top of that are kinda pointless.

Maybe I come up with an alternative solution.
@ninomp What do you think?

[daschuer]

Though, after I browsed the list and scrolled too far to have the track in sight I'd use it to scroll back to the track in the current view.

What is your reason for doing this?

[ronso0]

Though, after I browsed the list and scrolled too far to have the track in sight I'd use it to scroll back to the track in the current view.

What is your reason for doing this?

Usually I work in Tracks only. I filter for some genre or custom tag and sort by rating or play count. Mostly I work from the top down (rating--) but I scroll down to unrated (or unplayed) tracks where I might spot a known, yet unrated track to play.
Then I'd like to go up again to continue where I left off.

[daschuer]

Ah great, so we can summarize your use case as well as "finding similar tracks" with the same genre and a similar play-count/rating. That matches my one as well.

[ronso0]

Ah great, so we can summarize your use case as well as "finding similar tracks" with the same genre and a similar play-count/rating. That matches my one as well.

I doubt we are on the same page. Searching and similiar tracks has nothing to do with my use case. It could also be that I'm in the chronological view (sorted descending by 'date added'). Like checking out new tracks, and throw in an old favourite (filtered view), then get back to the previous position (clear search, 'Select track')

Again: IMO the 'Select track' functionality may be used by 'Find similiar tracks', but 'Select track' should be independent and work for each view (no switch to Tracks), exactly as @ninomp described the feature in #4688 (comment)

[daschuer]

I doubt we are on the same page.

Since everything you write matches my use case, I think I was not able to express my idea good enough.

Let my try again:

• In Tracks or any Crate, tracks are always listed next to similar neighbors. The degree of similarity is selected by the sorting column. When you are sorted by date added they are similar because they have been added on the same day.
• When you now add a similarity filter / search query, you add another dimension of similarity to the list.
• Finding a similar track achieves the same result either when your filter matches the sort column.

Conclusion:

• 'Select track' is actually 'find similar' without an additional filter.

My idea is to revise both features with this knowledge. I have no fixed idea, only some loose questions we may verify before writing more code:

1. Is 'Select track' a developer term? Because the user actually wants to see similar tracks in the current library feature regardless of the fact if the current track is a member or not?
2. Why are we we switching to the Tracks feature unconditionally when "finding similar"?
3. Is 'Select track' the first step in a recursive filter menu?

Maybe we find a better menu that serves all these ideas? If finally consider the implementation of this PR the best UI for the feature, it is also fine for me, but we are sure we have not missed something better.

[ronso0]

Okay, thank you for the clarification!

I'll try to explain how my use cases differ.

In Tracks or any Crate, tracks are always listed next to similar neighbors. The degree of similarity is selected by the sorting column. When you are sorted by date added they are similar because they have been added on the same day.

Yes, but I don't want to filter the library, I want all tracks in the list.
I assume that is also the reason why @uklotzde implemented 'Find similiar..' to switch to Tracks: access to the entire track pool. Doing that in a crate or playlist --which is already some sort of filtered view-- doesn't make much sense to me, unless you have huge crates, of course.

Actually, I find 'Select track' useful to pull the selected track back into the view --without touching the searchbox.
For me, a replacement for a 'Loaded to deck #' column lp:1789456 that would somehow highlight currently loaded tracks in the table. Though this is probably also cumbersome if you scrolled very far. A dedicated Ξ (Select track) button in each deck would also work.
See lp:1863932

[ronso0]

I removed the wrong commits and refined the one that checks if the track is in the current view 648a779

Works fine in all views.

[daschuer]

Thank you, It is working good now. However I think the usability can be improved.

Yes, but I don't want to filter the library, I want all tracks in the list.

Of cause. Do we agree that the underlying use case however is the same? "Watching the track among other similar tracks?"

This gives us the chance to streamline the GUI and find the correct solution for corner cases. Following questions will come up:

• How to get a round the grayed out entry.
• When switch to tracks feature (yes/no)
• What to do when track is in crate but hidden by a filter
• What to do when track is not in crate.
• Allow the "find similar" feature for crates.
• How to deal with existing search entries.

How about making the Menu like this (first draft for discussion)

• Select in Library
• Search related in all Tracks
  • All
  • Key harmonic with
• Append search
  • Key harmonic with

All will select the current track if available

[daschuer]

Instead of graying out we may show either
"Select in Library" / "Select in All Tracks" / "Select in All Tracks (reset search)"

[ronso0]

Thanks, those are good questions.

Depending on the current view we could show different action(s):

• views that contain the track: "Select track ..."
• in views with an active filter: make "Select track.." a menu with an "clear search to show the track" action (or show only this action?)
• in views that don't contain the track (or don't have a tracks view): "Show track in 'Tracks' "

Apart from that I don't want to touch the "Search related tracks" feature here. Maybe your proposed "Append query" actions could even be + button right next to each similarity filter (so there is the regular filter action that replaces the current query, as well as + that appends it).
This and the proposals above are a lot of work. I agree that we could refine the "Search related .." feature and use it for the current view, too.

I propose to merge this basic feature first¹ and iterate in follow-ups. IMO the current functions bool isTrackInCurrentView and emit selectTrack(trackId) are required pretty much as they are now.

¹depending on how @ninomp is available he could pick my commits, or (my preference) I rebase his and my commits on main since the actual feature discussion has moved here.

[daschuer]

I propose to merge this basic feature first¹ and iterate in follow-ups.

Yes sure. We just need to make sure we are heading in the right direction.

So please give a hint when this is merge-able.

I think we can also make use of a feature to switch to a track in the Tracks view from any crate/play-list.
Do you have an idea how to integrate that with the current menu?

[ronso0]

I think we can also make use of a feature to switch to a track in the Tracks view from any crate/play-list.
Do you have an idea how to integrate that with the current menu?

Well, that could be Show track in "Tracks", shown only in the tracks tables and in the same place as Select track in library, The implementation is probably something like this + emit selectTrack(trackId)

mixxx/src/library/library.cpp

Lines 654 to 661 in 30cc5f1

	void Library::searchTracksInCollection(const QString& query) {
	VERIFY_OR_DEBUG_ASSERT(m_pMixxxLibraryFeature) {
	return;
	}
	m_pMixxxLibraryFeature->searchAndActivate(query);
	emit switchToView(m_sTrackViewName);
	m_pSidebarModel->activateDefaultSelection();
	}

[ninomp]

@ronso0 I rebased my commits on top of latest main: https://github.com/ninomp/mixxx/commits/select-in-library_rebased
Feel free to rebase your work on top of this.

[ronso0]

@ronso0 I rebased my commits on top of latest main: https://github.com/ninomp/mixxx/commits/select-in-library_rebased Feel free to rebase your work on top of this.

Thank you, done.

@daschuer This is ready for review.

[ronso0]

Ci failure was due to the Validate AppStream metadata check not finding the screenshots from mixxx.org
Images are there and load fine, so I'll restart the CI

[daschuer]

Thank you. This works good enough for a merge.

However, do you mind to move the feature to the top, or on the second to position. (because it looks odd to have a grayed out feature on the top)
It fits more to "search in library" than to the track file related group where it is located now.

[daschuer]

LGTM, Thank you!

[ninomp]

@ronso0 Thank you for taking over and finishing this.
@daschuer Thank you for review.

[ronso0]

❤️ 🎉