Input gestures dialog improvements

[amirmahdifard]

Link to issue number:

fixes #16816
fixes #10983

Summary of the issue:

In the 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 (edit button) and (edit) action to the context menu that appears when a (Gesture) is in focus, Allowing user to replace and edit 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 onEdit 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.

[amirmahdifard]

I have more for this pr, So I convurted to draft until my tests have been completed

[amirmahdifard]

Done. It was about the actions and shortcuts of the input gestures dialog, So Now the actions and suggestion regarding the delete key for gesture delete shortcut is implemented. It was very annoying that we didn't have a (edit) possibility to edit a gesture raather than removing and adding again. It saves several steps for the user. Suggestions regarding shortcut keys for other actions (Add) and (edit) Are Welcome!

[CyrilleB79]

I have just tested this PR.

@amirmahdifard thanks for this! The possibility to be able to modify an existing gesture rather than having to delete and add a new one is very appreciated!

What about using "Change" or "Modify" on the button rather than "Edit"? This would sound more natural to me. Please other people, give your opinion on this, especially if you are native English speaker.

At last, playing with NVDA to test this PR, I have had an error once when trying to reopen the dialog. I have not been able to reproduce again so I do not know if it is related to this PR or not. Just reporting in case someone else has seen this issue.

[amirmahdifard]

@CyrilleB79 hi,
thanks for your feedback! today morning I was thinking about I really need to open an issue regarding editing existing gestures because it's really needed, And I saw the issue And I was like, ok great! I will start to implement this right now!
About button's name, I have considered only 2 correct cases for this, replace, and edit. Which I picked edit at the end. Because it's just the more natural. We use edit for messages, and everything that we can Edit the content from the old to the new. We also can't use rename in this case because we are not renaming anything. So I've picked the moste matched groupe with the others. Add gesture, edit gesture, remove gesture.
About your error, I think it was something else And I know that it's not related to this pr because i'm a sensitive person and since i'm working on a screen reader NVDA, I test my changes for a fue hours with every covering cases before I push to github. So I didn't see an Error, And I also don't think it's possible because I mostely took care about following the same tested proofe patten of NVDA's code it self.
Thanks! If you have a shortcut suggestion for edit/add a gesture, Feel free to tell me! Currently we only have shortcut for remove gesture which is delete key.

[tareh7z]

Hi!

In my humble opinion, I don't think adding a button to edit an existing gesture is really necessary. It would introduce an additional button without providing a significant benefit. Replacing an existing gesture with a new one can already be done by pressing the Delete key and then using the Alt+A shortcut to add a new gesture.

Thanks.

[amirmahdifard]

@tareh7z This is your personal opinion. We have open issue and a lot of user feedback regarding the possibility to edit gestures.
It's faster and needed, Currently, You mentioned remove the gesture, Add a new one. This can be done by edit function. Remove is only correct if user wants to completely remove the gesture And don't add a new one. If your opinion would be the case everywhere, We wouldn't have an edit option in any app.
Edit action is nesisery for any item. If an item can be added and removed, it should be able to be modifyed as wel. That's known.

[tareh7z]

Hi!

I understand your point. My concern isn't the existence of an edit action itself, but whether it actually adds enough value in this specific case to justify another button in the UI. If users overwhelmingly prefer it and the feedback supports it, then that is a reasonable argument for adding it.

Thanks.

[josephsl]

Hi,

Some overall comments on this PR and other recent ones that I hope we can keep in mind when reviewing this work:

1. As Tarik mentioned, do not justify the code paths unless you have additional support. If you are proposing a PR based on your own personal opinions, then prepare to answer questions like the one Tarik is asking you to address. So far, I don't think you did so completely; I think what Tarik is asking you to do is to state why you are proposing this feature not just based on issues and personal inconveniences. What justifies adding an "edit" button to edit input gestures? What substantial evidence such as user feedback statistics and compelling community anecdotes can you provide to justify this code path? GitHub issues and their content can qualify as evidence, but be sure to understand the narrative bheind those issues fully before proposing changes.
2. PR attitude in general: I would like to kindly suggest learning to think carefully and negotiate. I remember we had a similar conversation a while back about telling people to accept your proposals or else. I think that attitude is showing up again in some recent PR's: dismissing contradictions and counterpoints as unnecessary and pushing with your proposals all the way, reading your "accepted" change as evidence of contributions. Do not dismiss questions that question your "material" (really digital) contributions; take time to consider their thoughts and ask why they are telling you to consider their "personal opinions" too. Consider, reflect, negotiate. Dismissing someone's critical questions as personal opinions is one of the quickest ways to be sidelined in software development industry.
3. Embrace nonacceptance. Building from item 2, do not just count accepted issue/PR pairs as evidence of contributions. If you do, you may fall into a trap where substantial, material, and visible contributions are what you are after, further pushing to get your proposals accepted or else. Learn to embrace nonacceptance, including PR's being rejected. Nonacceptances are learning moments, and embracing nonacceptances boosts your credibility in the end (I myself had some issues/PR's closed (sometimes rejected), and I learned to embrace rejections as moments of reflection and improvement).

As you work on future NVDA pull requests, I kindly suggest thinking about the following question: how should I be remembered five years from now? Would you like to be remembered as someone pushing for personal opinions without considering others' thoughts thoroughly, or a contributor who understands you won't always get your proposals accepted? Choose wisely.

Finally, one advice from a veteran contributor: you are at a stage in NVDA contribution where you must now consider what the organization (NV Access and the NVDA community overall) wants you to remember. when being hired or volunteering for the first time, people may think that they have some part in influencing the overall organization. This is not always the case: you will go through a phase where you will be torn between your "ideas" about work and the organizational reality you are facing. In NVDA contribution terms, this means your idea of "contributing" something to the NVDA project will be contrasted with what NV Access and the community actually demands from contributors: thorough thinking evidenced by issue/PR documentation, responding to comments and addressing review notes, to name a few. Only after going through this phase (having both general (macro-level) and personal (micro-level) understanding of what the community/organizations wants from you and finding a path toward your unique contribution) will you be seen as acceptable to the community (it took me a better part of 2012 and 2013 to go through this phase when it came to my NVDA work). As you can tell, my discussion of this "socialization" phase closely aligns with some of the big criticisms NV Access and some members of the community have for you: learn that nonacceptance is an option and gladly take it when it happens. So here's my advice: do not propose massive changes and pull requests until becoming comfortable with what the community expects from you.

Good luck.

Thanks.

[amirmahdifard]

@tareh7z yes. You will even will be end up using it at some point if you for example want to change a gesture for one of your scripts, and an edit action in that case would look easya for you raather than remove>add. Thanks.

[josephsl]

Hi,

I think there is another (implied) part to Tarik's question that I think we should address before continuing: what value will that bring to NVDA screen reader overall?

Regarding wording, I propose "change". We have processes such as changing speech syntehsizers with the interface labeled "change" in NVDA settings/Speech panel. Terms like "edit" and "replace" are also good, but I think we should keep it a bit general and make it easy to translate.

Also, one thing to keep in mind: it might be possible that one or more ad-dons might be relying on the "look and feel" of input gestures dialog. Adding new elements means these add-ons may need to be modified to offer the same "look and feel" if add-on authors choose to do so.

Thanks.

[amirmahdifard]

@josephsl I saw the open issue in github that was requesting editing existing gestures, And I didn't think that it's bad. I had the possibility, so I Implemented it, linked the related issue here, and submitted. I don't have anything else to say.

[amirmahdifard]

There are no impact for addons, because I didn't touch an existing functions at all.
About value, we already have one value here, and also people who need a possibility, and they open a github issue for it. And I saw that issue and implemented it.
I don't know what are you trying to say, you want to say that edit gesture should not be added? onnistly when I started to implement this, I only considered the people who need this. I didn't think that it will have disagree opinions as well. As I said, i don't have anything more to say.
Just I didn't think that if I Fix a github open issue for a long time by linking a pr to it, some people will come and say why you did it.
If this opinion would be the case, then why something called "edit" exist, then lets just make everyone to delete and add again on their everything. I saw users feedback that said if we want to edit a gesture, no edit is available in any way, we must manually remove a gesture and add a new one. This makes it clear that user wants to have an action to press it, enter their new gesture, and replace it with the old one.
That's all. The value for this feature is this user expectation.

[wmhn1872265132]

I also support using 'change', which is clearer than editing and is only used to redefine new gestures

[wmhn1872265132]

There is a scenario where directly changing a gesture is very practical:

• A feature has two gestures, a and b.
• You want to change gesture b to gesture c.
• Delete gesture b.
• At this point, the focus is on gesture a, and the Add button is disabled. You must press the Up Arrow key to move focus to the feature name before the Add button becomes enabled.

Additionally, if a new gesture is assigned incorrectly, or the wrong keyboard layout is selected for the gesture, using the Change command is also more efficient than deleting and then adding again.

However, since this feature is not essential and there is now a context menu, I believe that:

• The dialog box does not need to have a Change button.
• The Change command should remain in the context menu.
• The Ctrl+H shortcut should be assigned to the Change command.

[tareh7z]

Hi!

I don't think anyone here is attacking you personally. People are questioning the proposal, not you, and that's a completely normal part of the review process in the open source world.

Also, I think Joseph gave you a lot of valuable advice. He has many years of experience with both open source projects and NVDA itself, so I think it's worth taking his comments seriously, taking a step back, and reflecting on them, even if you ultimately don't agree with everything he said.

Finally, I'd like to add that I agree with what @wmhn1872265132 said. Keeping the Change action in the context menu seems like the best solution for this kind of functionality.

Thanks.

[amirsol81]

For whatever it is worth, I also support this PR. Being able to change or modify an existing gesture would be hugely welcome.

[amirmahdifard]

hi guys! thanks for all of your feedback!
regarding the change instead of edit, i'm ok with it. I implemented the feature, and I will call it whatever makes more sence and the comunity wants.
Regarding the buttons, I Agree with buttons removal, but not this way. We can take 2 paths

1. Remove all item actions button which are change button, add button and remove button and only keep reset to defaults button, and have all item related actions in the context menu, which i agree with, but only if NVAccess also agrees.
2. All buttons should be kept as well as the context menu actions.
3. Regarding the feature, I saw that you are trying to say its pozitiv points. Onistly it's not a bad thing at all. We have edit actions for everything. I think people can know this that missing a feature will annoy those people who needs it, but its existence won't annoy anyone. The reson this is implemented here was because users needed it, opened an issue and requested it. We have add, edit and remove functions for everything. So I think the conversation here should only be about what the button should be called, and what shortcut key we should assighn to it.

[CyrilleB79]

I agree with renaming the "Edit" button to "Change": that would match what is already done in speech/braille settings panel. And to have a complete match, I suggest the same accelerator key, i.e. "h": "C&hange".

On the presence of buttons or not, I agree with last message of @amirmahdifard. We should try to do something consistent: either we duplicate all actions on single item between buttons and context menu (Add, Change and Remove), or we duplicate none.
In the work regarding the dictionary, NV Access has agreed to add a context menu but not to remove corresponding button. I do not see any good reason why this should be different here.

At last, I can see that "Reset to factory defaults" has been added in the context menu. I disagree with this approach. Resetting to factory default is quite rare so we do not need it in context menu. It's also clearer to add in context menu only the actions that apply to the selected item only. At last, in the dictionary work, the global reset action has not been put in the context menu so doing something consistent here would be appreciable.

[amirmahdifard]

@CyrilleB79 thanks!
in short, we now have 2 choices for the name of the button.

1. keep it as edit button to match the edit button in the speach dicts dialog,
2. call it change button to match brail and sinths selection that you mentioned.
   About the actions in context menu, it depends on what nvAccess decides
3. If they decide to desighn the dialog as standard, we will remove all globel actions from context menu, and remove the item related buttons. Result. Have item related actions in context menu only, and have globel actions as buttons only.
4. If they disagree with removing the buttons, so we have globel and item related actions both as buttons, Then we'll match the context menu as well. So we can pick one between.
5. having buttons only for globel actions and context menu only for item related actions,
6. have both buttons and context menu actions for both globel and item related actions.
   each one has its own goodys. The first one is standard dialog desighn, the second one maximises eas of access in the same agreement of NVAccess's agreement.

[CyrilleB79]

One additional clarification why I suggest to name the button "Change" rather than "Edit":

• "Change" is used to modify a single value as a whole, e.g. speech synth, braille display, update mirror.
  • If the single value to modify is a string corresponding to a name, "Rename" may be used instead, e.g. Profile
• "Edit" is used when something more complex is partially modified, e.g. dictionary entry. For another external example, we use "Edit" for a document or a text.

Hence my choice for "Change" rather than "Edit".

---

Also, regarding the 2 GUI architecture alternatives you list in #20437 (comment) (point 5. and 6.), I'd like to clarify that my proposal in #20437 (comment) is a third one:

• have both context menu entries and buttons for item actions (add, change and remove)
• have only a button for global action (remove all)

[amirmahdifard]

@CyrilleB79 thanks. Your reason for the change is correct and I also agree. I will call it change with c&hange.
Regarding the buttons, with the method you suggested, it would not be 100% consistent because then we'll have item related actions in both context menu and buttons, but the globel action only as button.
I think to prevent user facing change, and also we forced to do this for configuration profiles dialog because normal configuration is always available, we'll auder the context menu actions exactly the same as buttons. When context menu opened when catigory is in focus, only reset to default action will be available, same as the button. When focused on a script, the button's auder is like this. Add button, and reset to default button, so we'll have same auder in context menu. When a gesture is in focus, the button's auder is like this. Change button, remove button, and reset button. So we'll have same auder in the comtext menu. Also, in a dialog, when we have an action, we don't care if this action is used mostely or it is used very rarely, the action should be available to the user, and it's up to the user when they want to use it or not use it. Also, thankfully, buttons auder considers item actions, the moste used actions at the top, and the reset button is the last button at the bottom. So, to folow NVAccess's agreement, and prevent changing the dialog's facing for hundrids and hundrids of nvda users, we'll not risk, so we'll just use this method. I was thinking once more that our target here is to maximise eas of access for all nvda users. We don't want to change the user facing changes forcefully for users, so there would be no choise left for them but to learn and use the new method.
Aditionally, if NVAccess ever wanted to remove the buttons later, first we should add these context menus without removing the buttons so users will sloly get femilia with the context menus, and then after some time that users got used to the context menus, NVAccess can remove the buttons if they wanted to.
I'm being very careful about my work for nvda, because I Know that i'm touching a screen reader of all blind users and all screen reader users effect of using the screen reader. My point for nvda is only these things. Keep nvda easy and accessible, keep nvda to cover all user needs.

[CyrilleB79]

OK, I have heard your opinion. But I don't share it: it is not always 100% respected, but usually, context menu items are actions that apply to the selected item in a list.
For example, in Add-on Store, in the context menu of an add-on, you have the actions "Disable", "Remove" or "Help" because they apply to the selected add-on; but you have not "Install from external source" which does not apply to the selected item.

Anyway, let's hear NV Access opinion on this point.

[amirmahdifard]

@CyrilleB79 Yes, you're correct,
but buttons should be only for globel actions. So now that buttons are handling both globel and local actions, context menus should do the same.
Also, when the catigory level is selected, we only have reset to default button enabled and available, So context menu also have the same action in that case, so it's not empty. I've made context menu as dynamic as buttons. To perfectly match them with each other.

[amirmahdifard]

@CyrilleB79 hi again. I've applyed (Change) instead of (edit). If you want, please test it again and see if this is better as requested? thanks, I'm happy that you liked this feature, Along with the others and who opened the issue #10983 I get so happy when I see an issue, And I know how to implement it. it's unbeleavable, but it's the moste ever thing that makes me happy.

[josephsl]

Hi,

Two more things I would like us to consider based on recent comments:

1. Can NVDA cover the needs of all users: no because there are people the screen reader will not "talk to". For example, it wasn't until recently that NVDA added magnification, a visible way to communicate with low vision users. There are apps that NVDA does not support natively, there are braille displays that NVDA doesn't support yet, and there are screen reader usage scenarios requiring add-ons. A screen reader, just like any access technology, is a contextual technology. Therefore, we should be careful when saying things like "my pull request is designed to make things accessible to everyone, to cover needs of all users"; saying things like that will invite skeptical responses like the ones we read yesterday.
2. Context menu versus buttons: let's think outside the box for a second: give us really compelling statistics and/or community anecdotes from mainstream technology landscape such as operating systems, office suites, and others where the developer (original developer or a pull request submitter) justified a specific design over others. And no, using examples from other access technologies including NVDA add-ons do NOT count. Please do this exercise BEFORE committing more work; the purpose of "forcing" you to do this exercise (sorry for stating it that way) is to get you to think outside the box and justify the choices you are making in a series of pull requests (including this one) where you are advocating for adoption of context menus based mostly on personal anecdotes.

If you can do the second exercise, that's great - better later than never. But in the future, please do this kind of exercise BEFORE even committing your first commit. Trust me: having a conversation with yourslef about design choices and evidence saves reviewer time and work (up to an extent). As some have noted, I want to make sure that your proposal is solid and have broader community support to make life easier for NV Access staff who will be reviewing this code (consider community members as "first-level gatekeepers" and test audience to guide your thinking).

Thanks.

[amirmahdifard]

@josephsl hi. Sorry. By saying nvda covers all user needs, I didn't mean we absolutely support all user needs in the urth like this.
I ment like, We will try our best to fix as moste bugs as possible, when users open an issue regarding their need, if NVAccess agreed on it and it was doable for a contributer wether that is me, you, or someone else, we will do our best to fix and implement, so sloly, it becomes better and better as much as it is possible.

[amirmahdifard]

anyway, guys, I feal the conversation in this pr became too complicated and unrelated. This pr was simply about That I fixed 2 open issues regarding delete key to remove the gestures and #10983
regarding ability to change existing gestures, And now I feal the conversation got too far complicated.
lets allow the pr to take its normal forward steps. Thanks.

[cary-rowen]

Hi @amirmahdifard,

First off, thanks for putting this together!

As a fellow contributor who cares deeply about this project—and someone who has definitely had their fair share of heated debates over features—I wanted to share a few thoughts from a fresh perspective.

Looking closely at how the thread has evolved, a couple of things stood out to me that might be worth reflecting on:

1. When replying to @CyrilleB79 and @josephsl, it’s easy to fall back on arguments like "this feels more natural" or "it won't hurt anyone." But for long-standing projects, maintainers naturally default to historical consistency and structured proof. If we can provide concrete, scenario-based breakdowns—similar to what @wmhn1872265132 shared later by demonstrating exactly how many keystrokes this saves a user—the argument becomes incredibly hard to push back against.
2. When @tareh7z questioned whether the button was truly necessary, your response ("if everyone thought like that, no app would have an edit function") sounded a bit defensive. In open source, endless questioning and initial pushback (or "nonacceptance") are just part of the daily routine. The maintainers are scrutinizing the code boundaries, not your capabilities. It helps a lot to keep it strictly technical and try not to take their skepticism as a dismissal of your hard work.

Like Joseph mentioned, knowing when to compromise and how to negotiate is a massive skill here. Writing the code is often just 30% of the work; the other 70% is collaborating within the existing project rules and building consensus with the community. Dropping the defensive posture is usually the quickest way to turn that raw technical passion into merged code.

Thanks again for all the effort you've poured into this, and I’m looking forward to seeing this feature land in a way that feels seamless and native to NVDA!

Best,
Cary

[amirmahdifard]

@cary-rowen hi. Very thank you for your comment.
well, it's true that i'm a new contributor and may be I don't know how to do some things, but When the person commented and said this feature would not be needed, I became a bit disapointed for a fue reason.

1. They saw that open issue that was requesting edit gestures And I opened this pr to solv an existing agreed open issue.
2. When i started to say without this feature, user needed to remove the gesture, find the parent script again, press add again, enter the new gesture which is a lot of steps and sometimes causes confution, user can just find the gesture, and think like, ok, I don't want to remove this gesture. I don't want to add a new one. I just want to change this gesture. So i will press change, nvda will tell me to enter input gesture, and when I do and finish the steps, I will see that I correctly managed to change my existing gesture to something else that I want.
   But instead of telling this to him, I was like, this is your personal opinion. If this feature is implemented, it means there are users out there who needed and requested this feature.
   I want to give you some example. Personaly, I don't have a brail display device. Nvda's all brail features are unneeded for me And I have never used it. Same as mouse and touchpad features. Same for magnifire. But Can i go tell hay NVAccess, please remove brail features from nvda, remove magnifire, remove all mouse support just because I don't need them? no! absolutely I can't because I understand that there are people out there who are using these features. So same here. I'm also sorry anyway. It's a normal fealing everyone could feal when they implement something and someone randomly comes and says this feature is not needed, we can just remove the gesture and add a new one. Anyway, that is still available, so that user can still remove the gesture and add a new one if they didn't wanted to use this feature.

[amirmahdifard]

sorry. I messed up this branch, So I forced to create a new one just because I was trying to force push and rebace my branch