Library: add control to open/close the track menu

[ronso0]

Note: I don't intend to merge this, unless others consider it a useful addition, of course.
This is just to get a review if someone is interested.

This adds [Library],show_track_menu control. It's read/write, i.e. set to 1 to open the track menu for the track currently selected in the tracks table at the current mouse pointer position -- set to 0 to close it.
It's set to 1 if there's an open track context menu, no matter if opened with right-click, menu key (Shift+F10?) or the CO.
This is only supposed to work with the track menu of the tracks table -- other instances opened from the deck labels are not registered. The tracks table does not need to be focused for this to work.

In conjunction with #4618 this allows me to pick a query from the "Search related tracks" submenu easily with encoder pushs (Enter) and turns (Up / Down), so I need to use my laptop's touchpad even rarer.

[JoergAtGithub]

This CO makes sense to me. Especially with the Search Related Track function.

[ronso0]

Yeah, and it works well. It's also useful for deleting a track from disk.
I've mapped it to Shift + Trax knob press and the navigation works well: turn scroll, press confirm.

There's still an issue with submenus: in ~10% of the attmpts I can open for example the "Search related" menu but for some reason subsequent move/scroll commands are still sent to the main tracks menu. Seems to be a focus issue related to the cursor position because that determines in which direction the menu unfolds.
The current implementation may work better, though I didn't test it yet with a controller. Diff: before I used only the cursor pos, now I use the existing contextMenuEvent slot with an emulated QContextMenuEvent incl. QContextMenuEvent::Mouse reason.

Some more findings from real life testing:

• if a dialog has focus [Library], Move... controls should sent Tab / Shift+Tab keystrokes (not up/down/left/right)
• if a dialog has focus [Library], GoToItem control should sent Space instead of Enter

[daschuer]

I can confirm that this is useful. Especially when we think of creating a touch skin.

[ronso0]

Okay, there's interest so this could be merged?
I'll test the updated commit with my controller to see if it works 100% for submenus.

[ronso0]

Works nicely! Tested with #4618

[daschuer]

Can we close this PR now since it is part of #4618

[ronso0]

No, I prefer to have those feature in separate PRs.
It's in #4618 only to simplify testing the new menu navigation. I'll rebase that once this PR is merged.

[ronso0]

I changed the control to [Library],toggle_track_menu

[JoergAtGithub]

Is toggle really what suits best? If we want to toggle from a script, we've allready script.toggleControl
But can I map an ESC button, which should only close, but never open a menu?

[ronso0]

The difference is that this does not toggle the control but the context menu, only 1 is processed: you'd always use engine.setvalue("[Library],toggle_track_menu", 1); and the c++ code invokes the appropriate action.
This works fine with controllers, as well as with a future GUI / touch screen implementation where we only need a 1-state trigger button (IMO there's no point in having that button indicate the menu visibility, as well as no point in using it to close the menu as you would simply click anywhere outside the menu, as usually).

[daschuer]

For me the new name is clear, but I don't mind to change back to the old name, if it is considered to be confusing. Just give me a hint when this is ready to be merged.

[Swiftb0y]

IMO I prefer the richer CO where you can actually read the state from the CO. I do not see the benefit of hiding that information via a "smarter" toggle CO.

[ronso0]

This toggle is only for opening/closing WTrackMenu parented to WTrackTableView. The connection is made in LibraryControl > WTrackTableView (> m_pTrackMenu). It's not "smart", it's simple and efficient.
For a read/write CO an aditional connections are required, for example overriding QMenu::closeEvent and sending that all the way to LibraryControl...
Is it really worth the effort for a rather obvious widget?
What is the benefit of reading the visible state?

FWIW if the visible state is intended for elaborate scripting, this is already possible with #4618 which extends the [Library],focusedWidget enum by 4 ContextMenu(all context menus and those of the main menu bar) and 5 Dialogs.

[Swiftb0y]

FWIW if the visible state is intended for elaborate scripting, this is already possible with #4618 which extends the [Library],focusedWidget enum by 4 ContextMenu(all context menus and those of the main menu bar) and 5 Dialogs.

So is it possible to open these menus by setting focusedWidget to the appropriate value?
I'm worried that its a bad design for a CO that has overloaded capability (close menu when visible, open context menu when not). This seems to me like a similar mistake in design where beatjump CO would also move the loop with it when the loop was active. Lots of people didn't want that so they worked around that in their mappings. Thats why I'm saying that this new CO is "smart". Also, being a "write-only" CO makes it harder to use because its essentially opaque. Having to query another CO (focusedWidget) to be able to reason about the behavior of toggle_track_menu is an awkward API.

[ronso0]

So is it possible to open these menus by setting focusedWidget to the appropriate value?

no, only existing library widget can be focused with [Library],focused_widget (enum 1-3).
The other values (4, 5, 6, 0) are read-only: any QMenu, any QDialog, unvategorited widgets, none

Reverted the name to [Library],show_track_menu
And I made the track menu toggle read/writeable, went quicker than I thought. Still: this works only for the WTrakMenu of the WTrackTableViews! It will not be aware of other WTrackMenu instances, so multiple menus can be open (though only the most recent one can receive keypresses, GoToItem and MoveUp/Down/Vertical, with #4618)

[Swiftb0y]

this works only for the WTrakMenu of the WTrackTableViews

Thank you. I think this is fine as long as we document it as part of the MixxxControls.

[ronso0]

Sure, I'll update the manual after merge.

[ronso0]

Documentation comments updated, rebased, squashed, ready for review!

[ronso0]

(forgot to git add some comments)

[daschuer]

This is mergeable now. However some conflicts have been developed.

[ronso0]

I'll take care of it in two weeks.

[ronso0]

This is mergeable now. However some conflicts have been developed.

Fixed, ready for merge

[daschuer]

Thank you!