Allow adding emulated system keyboard keys in the Input Gestures GUI

[LeonarddeR]

Link to issue number:

Closes #6060.

Summary of the issue:

In the current situation, emulated system keyboard key scripts can only be added in gestures.ini or by updating a gesture map programmatically.

Description of how this pull request fixes the issue:

Allows adding custom emulated system keyboard keys from the input gestures gui, by selecting the category root tree view item and pressing add. An user will be able to enter a gesture to emulate, the entered gesture will be added to the tree view as a regular script and can be assigned as normal.

Testing performed:

Added and removed several gestures from the gui.

Known issues with pull request:

Some minor UX problems regarding adding and removing custom emulated keys:

1. When adding an emulated key without assigning it, an user can also press the remove button, in which case the emulated script disappears.
2. Same is the case when adding a new emulated key, assigning it and then unassigning it.
3. When you add a custom emulated key and then close the dialog, the custom emulated key is lost. This is documented in the user guide.
4. When you add an emulated key and assign it, then save, the remove the assignment, you can't remove the emulated script. After saving again, the emulated script will be removed anyway.

These issues have to do with the fact that we can't reliably check from the input gestures dialog whether a custom emulation script is part of the user gesture map or another map. IN the latter case, removal should be unsupported. Thus, explicit removal of an emulated script is only possible when it has been added during that instance of the input gestures dialog.

Honestly, I think these are trivial issues, so I'm filing this pr to let initial review take place and discuss the limitations in the current implementation.

Change log entry:

• New features
  • New emulated system keyboard keys can be added from NVDA's Input gestures dialog. (Custom emulated system keyboard keys #6060)
    • To do this, press the add button after you selected the Emulated system keyboard keys category.

[dan1982code]

I notice that moving to "emulated system keyboard keys" and then tabbing does not show the "add" button. I have to hit down arrow once to get to my first emulated key, and then tabbing gets me to the "add" button.

Thank you thank you for adding this. It's super-important for configuring braille displays without having to go into gestures.ini.

[LeonarddeR]

I assume you are running this branch from source?

[dan1982code]

My fault. I thought the PR was incubating. Please ignore. I will run from source now and test further.

[feerrenrut]

@dan1982code did you end up testing this?

[feerrenrut]

I'm not really sure how to test this myself, so hopefully it gets a good workout while incubating.

[LeonarddeR]

@feerrenrut, thanks for incubating this. What do you think about the UX issues I outlined above?

@dan1982code: Have you been able to do any tests while running from source? The pr is now incubating, so you should be able to test this from the next branch.

[dan1982code]

I was running into a bug before (running from source) where Add wasn't active, but now I can add and Remove successfully. I'm testing with a Brailliant BI here -- a braille display is the key scenario for this PR, I think. So far so good, but I'll test more systematically today and report again.

[LeonarddeR]

@dan1982code commented on 24 jan. 2018 15:03 CET:

I was running into a bug before (running from source) where Add wasn't active...

Are you sure you were running from the correct branch? It should actually be quite impossible for the add button to be inactive on the emulated categories root.

[dan1982code]

I was confident that I was using the right source branch, but now I don't know, because I've tried many times today, after computer/NVDA restart etc., and can't reproduce the add button not working. Sorry for the trouble -- all seems fine here now.

[derekriemer]

This fails if you do the following:

1. Add on emulated keys.
2. press nvda+1
3. Go into that, and add a gesture, then press d, select all layouts.
4. Now, hit okay, and type d somewhere.

[LeonarddeR]

Interesting catch there. This is because keyboardHandler.KeyboardInputGesture.fromName does not know about the NVDA key, which has been fixed as part of #7843. However, when applying that fix, I noticed some other bugs with KeyboardInputGesture.fromName.

>>> keyboardHandler.KeyboardInputGesture.fromName("control+l").displayName
u'ctrl+l'
>>> keyboardHandler.KeyboardInputGesture.fromName("l+control").displayName
u'+ctrl'

I will commit a fix for that issue tomorrow. I will also write unit tests for the NVDA key as well as several edge cases KeyboardInputGesture.fromName should cover. Furthermore, we really want to have emulate scripts show up as NVDA+1, and definitely not 1+NVDA.

Issues reported by @derekriemer need to be fixed before this is ready to go in master.

[derekriemer]

Interesting, so we'll need to prioritize modifiers.

[LeonarddeR]

@derekriemer commented on 26 Jan 2018, 05:54 CET:

Interesting, so we'll need to prioritize modifiers.

Somehow, yes. The current code just assumes that the last key in the sequence is the main key and the others are modifiers.

[LeonarddeR]

I believe I made this fully python 3 ready now. Won't have the time to look into this any time soon, though.

[feerrenrut]

@dingpengyu Please don't make comments like this, we are aware of this PR.

The last comment from the author indicated remaining issues, which have not been resolved.

I just fixed the conflicts, but it makes sense to do some additional discussion about the UX issues first.

[LeonarddeR]

This pr is now pretty old and with the UX issues still there, I'm afraid it's getting nowhere without major refactoring, for which I don't have the time now. I'm tempted to close this, donating the code to whoever is interested to take this further.

[feerrenrut]

If you are happy for someone else to take this on. You can leave it open for now, I'll apply the abandoned label.

[LeonarddeR]

Sure I am. This abandoned label is a nice addition.

[Adriani90]

cc: maybe @JulienCochuyt or @Andre9642 could be interested in working on this further?

[michaelDCurran]

This pr now contains some pretty big conflicts in gui/settingsDialogs.py probably due to the merging of #10307.
@feerrenrut or @JulienCochuyt either of you may have a better chance at merging master and fixing these conflicts than me.

[JulienCochuyt]

@michaelDCurran wrote:

This pr now contains some pretty big conflicts in gui/settingsDialogs.py probably due to the merging of #10307.

Indeed, we kinda stepped on each other feet here.
@feerrenrut, I won't be able to give it a proper look before the end of the week due to customer obligations.
Let me know if you prefer I try to tackle this one nevertheless.

By the way, this led me to re-read my old code for PR #10307 and notice two errors I made that you'll hopefully help me fix by considering merge of the fix-up PR #11338.

[dpy013]

hello all
pr 11338 It has been merged.
Whether or not to merge the master discerning fixes source/gui/settingsDialogs .py conflict issues?
thanks

[feerrenrut]

@JulienCochuyt Yes, I'll take a look at this.

[feerrenrut]

Closing as superseded by #11402