Clean up track right click context menu #1427

Be-ing · 2017-12-20T02:33:40Z

Organize right click track context menu https://bugs.launchpad.net/bugs/1733199
Lazy load playlist and crate submenus to avoid hanging GUI when these are not used https://bugs.launchpad.net/mixxx/+bug/1733200
Add menu items to clear hotcues. I tried adding items to clear the main cue and loop too, but I don't know why those are not working. Alleviating https://bugs.launchpad.net/mixxx/+bug/1666275

Before:

After:

This builds on @poelzi's work in #1341.

Switch from normal QAction menu to a composition of QCheckBox and QActionWidget. The checkbox shows in which crates the selection is in. Changing the crates selection does not collapse the menu, which allows much easier categorization of tracks without going through the menu from scratch. Use tristate when multiple tracks are selected. When mulitple tracks are selected which do not share a crate, use the tristate partially selected to indicate this. Fix styling of pointers. Use QVariant to transport CrateId in checkbox Use optimized SQL query to select crates and count tracks. Unfortunatelly QSqlQuery has no way of binding QLists in WHERE x IN statements. Use make_partented as suggested by daschuer rename "Add to Crates" -> "Crates" to reflect function better

Optimize formatting of string lists for SQL queries

If the song changes while the current history view is open, the selection of songs is lost. Therefor all crate changes are lost. The history view saves and restores the selection through a new usefull api for selecting tracks in the wtracklistview.

…e/crate_selectbox

daschuer · 2017-12-20T07:14:35Z

Thank you for jumping in here.
I think we should distinguish between organizing functions and dj-ing functions. I like to have all DJing functions instantly visible.
Especially the top an bottom Auto DJ entries should be on the first level.
I have never used the other load to features, but I can imagine that if one has used it he will miss it on the first level as well. Maybe it is used in cases where drag and drop dies not work.
We can also consider to move all non-mass actions to the properties dialog.

Be-ing · 2017-12-20T07:23:56Z

Especially the top an bottom Auto DJ entries should be on the first level.

Yes, now that I see the reorganized menu, I think we have space for two more menu items.

I have never used the other load to features, but I can imagine that if one has used it he will miss it on the first level as well. Maybe it is used in cases where drag and drop dies not work.

Neither have I, and I don't want to take up a lot of space in the top level of the menu for them. The only use case I can think of for them is when the mouse is unusable for whatever reason (hardware issue, physical disability).

We can also consider to move all non-mass actions to the properties dialog.

I'm hesitant to hide useful features in a dialog.

Be-ing · 2017-12-20T07:41:15Z

src/track/track.cpp

+    for (CuePointer pCue : m_cuePoints) {
+        if (pCue->getType() == Cue::LOAD) {
+            disconnect(pCue.get(), 0, this, 0);
+            m_cuePoints.removeOne(pCue);


@uklotzde any ideas why this is not working?

Be-ing · 2017-12-20T07:41:43Z

src/track/track.cpp

+    for (CuePointer pCue : m_cuePoints) {
+        if (pCue->getType() == Cue::LOOP) {
+            disconnect(pCue.get(), 0, this, 0);
+            m_cuePoints.removeOne(pCue);


@uklotzde any ideas why this is not working too?

I don't remember right now how this works with c++, but in Java you cannot remove an element of a list that you're iterating with a for loop. You need to get an iterator, and make a loop with the iterator.
Since the iterator only has references to next and previous, it doesn't get affected by removing elements from the list.

Then why does clearing hotcues work?

This is definitely wrong: "... any non-const function call performed on the QList will render all existing iterators undefined."

Use this one:
http://doc.qt.io/archives/qt-4.8/qmutablelistiterator.html

I remember it, because I just removed it from AnalyzerQueue ;)

I tried changing this to for (const CuePointer& pCue : m_cuePoints) { but it did not solve the issue.

I'm guessing there's something else being triggered elsewhere in the code that actually clears the hotcues...

uklotzde · 2017-12-21T07:38:19Z

src/track/track.cpp

@@ -746,6 +746,42 @@ void Track::removeCue(const CuePointer& pCue) {
    emit(cuesUpdated());
 }

+void Track::removeMainCue() {
+    QMutexLocker lock(&m_qMutex);
+    for (const CuePointer& pCue : m_cuePoints) {


Use QMutableListIterator and remove() through the iterator

I tried that and it is not the issue. I think Track::removeHotCues is working because of some non-obvious side effect it has.

uklotzde · 2017-12-21T07:38:50Z

src/track/track.cpp

+            m_cuePoints.removeOne(pCue);
+        }
+    }
+    markDirtyAndUnlock(&lock);


Only mark as dirty and emit signals if at least one item has been removd

uklotzde · 2017-12-21T07:40:51Z

src/track/track.h

@@ -246,6 +246,9 @@ class Track : public QObject {
    // Calls for managing the track's cue points
    CuePointer createAndAddCue();
    void removeCue(const CuePointer& pCue);
+    void removeMainCue();


Combine into a single function with the cue type as a parameter

uklotzde · 2017-12-21T07:46:21Z

Clearing the musical key is missing. We should include all analyzed results in the "Clear" menu.

uklotzde · 2017-12-21T07:49:11Z

I need to access the "Change BPM" options very often when tagging new tracks. The deep nesting is very inconvenient if you just want to half/double and subsequently lock the analyzed bpm.

I suggest moving the BPM options into a top-level item named "Adjust BPM >". This might also include an entry to clear the the current value and beatgrid in addition to the dedicated "Clear >" section.

uklotzde · 2017-12-21T07:54:02Z

"Clear" should not be nested in "Metadata", because it also includes track markers and analysis results that are stored separately. I suggest to move it to the top level.

ronso0 · 2017-12-21T21:46:31Z

I need to access the "Change BPM" options very often when tagging new tracks. The deep nesting is very inconvenient if you just want to half/double and subsequently lock the analyzed bpm.

Yes, BPM is also my n°1 action, followed by Properties & Show file in folder

Be-ing · 2017-12-22T05:49:37Z

Be-ing · 2017-12-22T06:09:17Z

Unless anyone has a clear idea why clearing cues only works for hotcues and fixing that can be done quickly, I think we should merge this without implementing clearing the main cue and loop.

daschuer · 2017-12-22T07:19:14Z

I need to access the "Change BPM" options very often when tagging new tracks. The deep nesting is very inconvenient if you just want to half/double and subsequently lock the analyzed bpm.

Yes, BPM is also my n°1 action, followed by Properties & Show file in folder

Yes, right. Reading this I can also remember to some emergency tweaks, after loading a track, and recognise an bpm issue. On the other hand this should somehow to a decks menu we do not have.
In any case this is a DJing use case wich should be instantly accessable.
How about moving bpm back to root for 2.1?

Be-ing · 2017-12-22T07:22:24Z

I'm reluctant to add any more to the top level of the menu now that the 3 AutoDJ items are back at the top level and Clear is at the top level. Is clearing the beatgrid really so urgent that it can't be behind one submenu?

daschuer · 2017-12-22T07:55:35Z

I can live with the current state, because it is actually wrong in the library at all, but we have two additional voice for it.
Why is clear on the top level?

poelzi · 2017-12-22T12:42:27Z

👍
Tho I have to say, I barely need hide from library and most of the time i press it by accident. Resulting in me searching the tracks in the hidden list just to unhide them again. It would be nice if it at least asks every time not only when it's in playlists. (There is no undo)

daschuer · 2017-12-23T08:08:41Z

@Be-ing, what is the state here? We are ad our beta deadline, and this one cannot be merge after beta.
Postpone this?

Be-ing · 2017-12-23T08:10:53Z

I'm okay with merging this as-is.

daschuer · 2017-12-23T08:17:11Z

OK! Thank you.

ronso0 · 2017-12-29T14:40:34Z

Thanks for cleaning up the menu. But now that I'm reviewing, sorting and clearing out my tracks of 2o17, I really miss

BPM
please put tis back to top level, it's important: Mixxx doesn't recognise the beats of most of my footwork and jungle/dnb tracks, in most of the cases I have multiply it by 2 or 3/2
Hide from Library
is this control still available at all? hiding recordings, long mixes and temporary files from track library is important

nopeppermint · 2017-12-29T15:21:38Z

in my opinion it would be nice to just show the actual displayed sampler deck and not all 64 in the context menu..

Be-ing · 2017-12-29T15:26:07Z

in my opinion it would be nice to just show the actual displayed sampler deck and not all 64 in the context menu..

That is a good idea. We'd have to unify the ControlObject names used by skins first.

naught101 · 2018-01-04T06:36:26Z

OMFG, the new context menus are SOOO GOOD. So much clearer, and it's so great how you can add to multiple crates/playlists without the context menu closing! Thank you! 🎉

When I'm in DnB mode, I agree with ronso0 re: the BPM stuff, but I'm in trip-hop mode now, and it's fine. Maybe it would be better to instead work on the analysis engine's response to DnB?

"analyse" would be a super useful addition to the menu - would save always having to scroll to the bottom of the library side bar. But that might be fixed with the library redesign anyway?

Be-ing · 2018-01-04T07:19:40Z

"analyse" would be a super useful addition to the menu - would save always having to scroll to the bottom of the library side bar. But that might be fixed with the library redesign anyway?

I agree. The tricky part is where to put the analysis progress information. In 2.1 only the Analyze library feature has a place for that information. I think we should keep this in mind when we come back to the library redesign. We may find other uses for a general purpose area for library context information, for example showing the cumulative duration of selected tracks. I opened a Launchpad bug for this a while ago that I have targeted for 2.2, but no one has yet volunteered to implement it.

Be-ing · 2018-01-04T07:26:48Z

If there is a consensus for moving the BPM editing submenu back to the first level, I'd be okay with that.

…ixxxdj/mixxx#1427. Update content to reflect changes.

poelzi and others added 9 commits November 11, 2017 02:01

Optimize formatting of string lists for SQL queries

de8f406

Merge pull request #2 from uklotzde/feature/crate_selectbox

7f2e1e7

Optimize formatting of string lists for SQL queries

Merge branch 'master' of https://github.com/mixxxdj/mixxx into featur…

39d609e

…e/crate_selectbox

Merge branch 'master' of https://github.com/mixxxdj/mixxx into featur…

6c8b9a1

…e/crate_selectbox

style fixes according to be's comments

341acf1

use const reference to prevent copy

187ec3d

organize track right click context menu and lazy load playlists + crates

7bbb3c6

Be-ing commented Dec 20, 2017

View reviewed changes

Be-ing added 2 commits December 20, 2017 21:51

move Add to Auto DJ menu items back to main track context menu

64396e0

use const reference in for loops

97dfb0c

uklotzde reviewed Dec 21, 2017

View reviewed changes

Be-ing added 3 commits December 21, 2017 21:36

Merge remote-tracking branch 'upstream/master' into track_context_menu

b8a6fe8

move Clear submenu out from Metadata submenu of track context menu

aa93746

code cleanup

bce0071

Be-ing mentioned this pull request Dec 22, 2017

merge master to 2.1 branch #1438

Merged

8 tasks

daschuer merged commit ea4b987 into mixxxdj:master Dec 23, 2017

Be-ing mentioned this pull request Dec 23, 2017

Use checkbox menu in crate selection. #1341

Merged

Be-ing deleted the track_context_menu branch January 4, 2018 07:15

esbrandt mentioned this pull request Apr 15, 2018

WIP Manual 2.1.x updates mixxxdj/manual#59

Merged

37 tasks

esbrandt added a commit to esbrandt/manual that referenced this pull request Apr 15, 2018

The track tables right click context menu has majorly changed after m…

8c8947b

…ixxxdj/mixxx#1427. Update content to reflect changes.

daschuer mentioned this pull request Jun 28, 2022

WTrackMenu: move actions into Auto DJ and Track Files menus #4830

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up track right click context menu #1427

Clean up track right click context menu #1427

Be-ing commented Dec 20, 2017 •

edited

Loading

daschuer commented Dec 20, 2017

Be-ing commented Dec 20, 2017

Be-ing Dec 20, 2017

Be-ing Dec 20, 2017

JosepMaJAZ Dec 20, 2017

Be-ing Dec 20, 2017

uklotzde Dec 20, 2017

uklotzde Dec 20, 2017

Be-ing Dec 21, 2017

Be-ing Dec 21, 2017

uklotzde Dec 21, 2017

Be-ing Dec 22, 2017

uklotzde Dec 21, 2017

uklotzde Dec 21, 2017

uklotzde commented Dec 21, 2017

uklotzde commented Dec 21, 2017 •

edited

Loading

uklotzde commented Dec 21, 2017 •

edited

Loading

ronso0 commented Dec 21, 2017

Be-ing commented Dec 22, 2017

Be-ing commented Dec 22, 2017 •

edited

Loading

daschuer commented Dec 22, 2017

Be-ing commented Dec 22, 2017

daschuer commented Dec 22, 2017

poelzi commented Dec 22, 2017

daschuer commented Dec 23, 2017

Be-ing commented Dec 23, 2017

daschuer commented Dec 23, 2017

ronso0 commented Dec 29, 2017

nopeppermint commented Dec 29, 2017

Be-ing commented Dec 29, 2017

naught101 commented Jan 4, 2018 •

edited

Loading

Be-ing commented Jan 4, 2018

Be-ing commented Jan 4, 2018

Clean up track right click context menu #1427

Clean up track right click context menu #1427

Conversation

Be-ing commented Dec 20, 2017 • edited Loading

daschuer commented Dec 20, 2017

Be-ing commented Dec 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uklotzde commented Dec 21, 2017

uklotzde commented Dec 21, 2017 • edited Loading

uklotzde commented Dec 21, 2017 • edited Loading

ronso0 commented Dec 21, 2017

Be-ing commented Dec 22, 2017

Be-ing commented Dec 22, 2017 • edited Loading

daschuer commented Dec 22, 2017

Be-ing commented Dec 22, 2017

daschuer commented Dec 22, 2017

poelzi commented Dec 22, 2017

daschuer commented Dec 23, 2017

Be-ing commented Dec 23, 2017

daschuer commented Dec 23, 2017

ronso0 commented Dec 29, 2017

nopeppermint commented Dec 29, 2017

Be-ing commented Dec 29, 2017

naught101 commented Jan 4, 2018 • edited Loading

Be-ing commented Jan 4, 2018

Be-ing commented Jan 4, 2018

Be-ing commented Dec 20, 2017 •

edited

Loading

uklotzde commented Dec 21, 2017 •

edited

Loading

uklotzde commented Dec 21, 2017 •

edited

Loading

Be-ing commented Dec 22, 2017 •

edited

Loading

naught101 commented Jan 4, 2018 •

edited

Loading