26583: Percussion Pads context menu refinements

[krasko78]

Resolves: #26583

Key Points:

1. Replaced StyledMenuLoader with ContextMenuLoader.
2. The coordinates of the click are now passed to the loader in the show() method.
3. Moved the loader outside of the footer and into the entire pad. Technically this was not needed for things to work but I figured it made sense.

• I signed the CLA
• The title of the PR describes the problem it addresses
• Each commit's message describes its purpose and effects, and references the issue it resolves
• If changes are extensive, there is a sequence of easily reviewable commits
• The code in the PR follows the coding rules
• There are no unnecessary changes
• The code compiles and runs on my machine, preferably after each commit individually
• I created a unit test or vtest to verify the changes I made (if applicable)

[avvvvve]

@krasko78 lovely, thanks!
@mathesoncalum please code review

[mathesoncalum]

Thanks very much @krasko78 - looks good!

My only question is about opening the context menu via the keyboard. If you look at padContent in PercussionPanelPad.qml, a connection is made between onTriggered and openFooterContextMenu.

I'm guessing this doesn't work with these new changes as we don't pass it a pos argument? (or maybe it does, but we'll probably get some warnings).

[avvvvve]

@krasko78 Actually, one more request while we're in here. In the context menu, could we change 'Delete' to 'Delete pad' as Marc suggested here?

[avvvvve]

Ah, you're right @mathesoncalum, I can't "press" the footer button now when it's focused using space/enter like I can in the Nightly build.

(In the video where it says "4.4" I meant to write "Nightly", whoops)

Screen.Recording.2025-02-19.at.5.46.15.PM.mov

[krasko78]

@mathesoncalum I've missed that, thanks! I'll fix it.
@avvvvve Speaking of further tweaks, I was wondering if "Define keyboard shortcut" should have an ellipsis at the end since it opens a new dialog?

[krasko78]

@mathesoncalum @avvvvve I've pushed a fix for the broken context menu activation with the keyboard and changed "Delete" to "Delete pad". Also removed 'footer' from names related to the context menu as the context menu is not footer-specific any more.

I also noticed that the pad could not be activated with the keyboard. This was not related to my changes but I decided to fix it too. In PercussionPanelPad.qml on line 100 when triggerPad() on the model was called, it was missing the keyboard modifiers as a parameter. I added ui.keyboardModifiers() and it worked. However, I noticed that pressing Space/Enter with a modifier key does not trigger the active control. This means that it is not possible to Shift+Ctrl/Meta + Space/Enter on a pad to accomplish the same as Shift+Ctrl/Meta+click works. This is probably a separate topic.

If there is nothing else, I believe the only thing left here is for Ben to tell me about the ellipsis.

[mathesoncalum]

Thanks @krasko78.

On your point about triggerPad - I opened PR #26616 earlier to address this. I prefer the method used over there because the onTriggered signal in padNavCtrl simply isn't called if a modifier is held, so the ui.keyboardModifiers() bit ends up being redundant.

Indeed this is something we might look to address if we want to support "add to chord" and "insert" functionality using a modifier + space, but let's keep it simple for now.

[krasko78]

@mathesoncalum Sounds good. I've removed my change around the modifiers. :)

[mathesoncalum]

Good stuff - thanks for the quick turnaround! ⚡️

[avvvvve]

@krasko78 looks great! you are right that it should be "Define keyboard shortcut..." with the ellipse. mind swapping that in?

otherwise @zacjansheski mind giving this a test too?

[krasko78]

There we go! Ellipsis added.

[krasko78]

@zacjansheski Hold on please, the ellipsis is rendered weirdly in Windows. I am looking...

[krasko78]

Something with the encoding of one particular source file was not quite right. Should be OK now.

[zacjansheski]

Tested on MacOS 15, Windows 11, Ubuntu 22.04.3. Approved
#26583 FIXED