input gestures dialog improvements

[amirmahdifard]

Link to issue number:

fixes #16816
fixes #10983

Summary of the issue:

In the first linked issue, requests have been made to be able to delete the gestures with the delete key, like the windows standard. The final result on that issue was adding the delete key to remove a gesture, read more on the linked issue
Also, while improving the dialog and added the context menu, It was a grate place for this pr to solv another issue, and edit an existing gesture

Description of user facing changes:

Added context menu And delete key shortcut to remove gestures on the input gestures dialog. Added (change button) and (change) action to the context menu that appears when a (Gesture) is in focus, Allowing user to replace and change a gesture without needing to remove the gesture, and add a new gesture from the script level

Description of developer facing changes:

none

Description of development approach:

Added On context menu, and on charhook methods to the input gestures, Used the exact same statements used on refresh buttons method, And bound them to the tree
Used the same patten as the onAdd method and added an onChange method, that safely removes the currently focused gesture, and Allow the user to enter a new one.

Testing strategy:

Manual testing. Tested all cases. Made sure that Context menu add, remove, and globel reset option works the same as buttons. Made sure that the delete button only works for the gestures. used same statements and patten as the refresh buttons method made by NVAccess it self. Made sure that the (pendingAdd) gard is properly protected in all cases. Edited an existing gesture, made sure that it properly edits. No unexpected bug, no crash, no duplication.

Known issues with pull request:

none

Code Review Checklist:

• Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• API is compatible with existing add-ons.
• Security precautions taken.

[CyrilleB79]

I think that "Remove all" should not be part of context menu. Indeed, context menu should only contain actions which apply to the currently selected item, not to the whole list of shortcuts of the window in general.

It's worth noting that there is already a visual distinction between buttons that perform an action on the selected item and the button which Applies to the whole list of gestures: "Add", "Remove" and now "Edit" are grouped together while "Remove all" is located apart.

We have already discussed this topic (and disagreed with each other) in #20437. I'm commenting here so that NV Access can make a decision on this point.

[cary-rowen]

First of all, regardless of how many improvements can be made to the input gestures dialog, my point is that we should avoid bundling a series of tightly coupled changes into a single PR. While I am not suggesting you create a separate PR for absolutely every small issue, keeping changes scoped and modular makes the review process much smoother.

Furthermore, regarding the "context menu" discussion, the concept of "context" should be very explicit here: it refers strictly to the currently selected item, not the global scope. Let's return to the true meaning of a "context menu"—it is meant to perform actions targeting the specific item in focus, rather than generic or wider-scoped window options.

Additionally, since you are able to create a brand new PR, I don't see why you couldn't simply point the branch of #20437 to this current development branch. Doing so would better preserve our ongoing discussions in #20437. In my view, that is exactly where our unresolved discussions belong, and those areas where we haven't reached a consensus are precisely the ones that hold the most value.

[amirmahdifard]

@cary-rowen hi. The reason I did that is as I explaned before, buttons are doing both globel and local actions, so I wanted to make context menus consistence with them. If buttons only took the globel action part, then context menu could grately take local actions of items only.