[MU4 Issue] Preferences | Shortcuts broken when a search filter is used

[wizofaus]

When you've typed something in the search filter in the Shortcuts page on the Preferences dialog, trying to clear/modify/reset the shortcut sequence for any of the displayed actions updates the wrong the action.

To Reproduce
Steps to reproduce the behavior:

1. Go to Preferences|Shortcut
2. Type, e.g. 'a' in the Search field
3. Select action "A"
4. Click clear
5. Nothing appears to happen
6. Remove the 'a' from the search field
7. Note that now in fact the action "16th note" (first in the list) now has no shortcut sequence
8. Same applies for using Define... & Reset to default
9. Also, select 16th note - note that "Define..." is greyed out! But double clicking works fine. Logically it's "clear" that should be disabled if an action currently has no shortcut sequences.

Expected behavior
Clearing/modifying/resetting a shortcut key sequence should work for the selected action even if there's a search filter in place. "Define..." should be available for an action with no shortcut sequence.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):
Windows 10

Additional context
Add any other context about the problem here.

[wizofaus]

Also currently the shortcuts dialog only shows actions that are found in shortcuts.xml, but there's a very large number of actions that don't have any shortcuts defined by default (e.g. "add-trill"). Many of them don't have associated UI elements (meuns etc.) meaning there's no way to access these commands currently at all.
I have a fix for this that ensures when UiActionsRegister::updateShortcuts() gets called ShortcutsRegister::shortcut now adds a shortcut entry there isn't already one. But arguably it would be better to change the ShortcutsModel used by the QML dialog to get the list of actions from the UiActionsRegister (it doens't currently expose an interface for this though).

But more problematically, currently I don't seem to be able to define new shortcut sequences via this dialog currently and actually have them work! It seems it might work if I exit and restart the app though?

[wizofaus]

Another issue with shortcuts - if you use "Add" when defining a sequence and there's not a sequence already defined, the sequence ends up being defined as, for instance , Ctrl+Alt+E (with the leading comma), which Qt doesn't like (causes asserts and even crashes in the debug version anyway).

[bkunda]

This is a critical issue as it interferes with defining shortcuts user flow

[bkunda]

Just to illustrate some of what this problem is affecting:

• Redefining a shortcut for a selected item with filter active redefines the shortcut for the item usually in the corresponding table row.
• When the filter is turned off, the selected shortcut has not been redefined. Instead, the command in the row that the shortcut was repositioned to (via the filter) has been redefined.

Illustrated below, shortcut for inserting note "A" gets moved to 3rd row with filter on. When filter is removed, the 16th note is instead redefined (as per @wizofaus's description above):

[igorkorsukov]

Also currently the shortcuts dialog only shows actions that are found in shortcuts.xml, but there's a very large number of actions that don't have any shortcuts defined by default (e.g. "add-trill"). Many of them don't have associated UI elements (meuns etc.) meaning there's no way to access these commands currently at all.
I have a fix for this that ensures when UiActionsRegister::updateShortcuts() gets called ShortcutsRegister::shortcut now adds a shortcut entry there isn't already one. But arguably it would be better to change the ShortcutsModel used by the QML dialog to get the list of actions from the UiActionsRegister (it doens't currently expose an interface for this though).

I believe that we should not add all actions from UiActionsRegister to the list of shortcuts.
I see two reasons why this should not be done:

1. Not all actions are public, there are internal actions. The more actions we "open" for the user, the more we will be constrained, the more difficult it will be to change (improve) something, will have to spend a lot of effort for backward compatibility or break backward compatibility, which is also bad.

2. We have an action context and a shortcut context - these are two different things.
   For normal operation, each shortcut should have a context when it can be sent. Otherwise, conflicts will appear or it will not be possible to assign the same keys for different actions.
   Simple example:
   When the notation is in focus, we can press the Up key and a pitch-up action will be sent.
   The pitch-up action has a context - the notation is open, that is, it can be sent at any time, if there is an open notation, even if it is not in focus, we can send this action, for example, through the menu, or by clicking a button (or we will add such button)
   The pitch-up shortcut has context - notation in focus, that is, it can only be sent if there is notation in focus, in other cases, when pressing the Up key, other actions are performed.

[igorkorsukov]

But more problematically, currently I don't seem to be able to define new shortcut sequences via this dialog currently and actually have them work! It seems it might work if I exit and restart the app though?

Fixed #10089

[igorkorsukov]

Another issue with shortcuts - if you use "Add" when defining a sequence and there's not a sequence already defined, the sequence ends up being defined as, for instance , Ctrl+Alt+E (with the leading comma), which Qt doesn't like (causes asserts and even crashes in the debug version anyway).

Fixed in some PR )

[igorkorsukov]

@DmitryArefiev Please check, PR #10141

[DmitryArefiev]

Tested #10141 on Win10, Mac12, LinuxMint 20.2 - FIXED

Please merge into master

[DmitryArefiev]

@wizofaus @bkunda Please check this in master when you have time (just to be sure I didn't missed anything)

Thanks!

[bkunda]

@DmitryArefiev I've had a look at this and it now seems to work as expected. Looking good from my end 👍🏻

[wizofaus]

Nope, still seems to be quite broken...which is not surprising given @igorkorsukov's PR that fixes it hasn't been merged yet!
I also noticed an additional issue that for me by default the "Close" command shows
Ctrl+F4; Ctrl+F4; Ctr...

and that's it - no reason why Ctrl+F4 should be shown twice, but also problematic that there's no way of finding out what the additional shortcuts are.
It would also seem to be the case that if a command already has more than 2 shortcuts there's no way of removing just those additional shortcuts, you can only replace ALL of them with a single one, but I'd agree that's not a high priority issue.

[bkunda]

Clarification: I checked out #10141, which seems to be working correctly (at least on Mac; I am currently unable to access my Windows machine so cannot verify anything there).

I also wonder whether the issue you've spotted @wizofaus is on Windows only. This is what I see on MacOS (which seems to be correct):