assign Ctrl S keystroke to Save As command in NVDA Log Viewer #4532

Closed
nvaccessAuto opened this Issue Oct 6, 2014 · 14 comments

Comments

Projects
None yet
4 participants

Reported by blindbhavya on 2014-10-06 14:44
The summary line explains it all.
So, could Ctrl + S be made to work as the access key for the Save As option in the Log menu in the Log Viewer found in Tools submenu under NVDA Menu?

Comment 2 by nvdakor on 2014-10-07 06:08
Hi,
It might be argued that it's just three key presses away: Alt, ENTER, S. But I do understand the potential uses for this command.
Technical: would it be possible to modify onOutputChar event in gui/logViewer to detect Control+S (ASCII 19) so it'll activate onSave command? The thing is, I'm not sure if it'll accept key presses from non-English keyboards.
Thanks.

Comment 3 by jteh on 2014-10-07 06:20
It may actually be simpler than that. I think wx has a way of automatically handling defined accelerators for menu commands. I just don't remember the exact details. Poking through the wxMenu documentation should reveal it if I'm right.

Comment 4 by Joseph Lee <joseph.lee22590@... on 2014-10-07 06:26
In [5bcd9a6]:

Log Viewer: added Control+S to save the log file. re #4532
Because control characters are assigned in the lowest range of ASCII code, Control+S is ASCII code 19. Since log viewer intercepts key events, add a check for Control+S to open save as dialog accordingly.

Comment 5 by nvdakor on 2014-10-07 06:29
Hi,
I'll look into wx.menu to see if the above code change can be modified (if possible). I hope it doesn't break under #3763 later...
Thanks.

Comment 6 by nvdakor (in reply to comment 3) on 2014-10-07 06:41
Replying to jteh:

It may actually be simpler than that. I think wx has a way of automatically handling defined accelerators for menu commands. I just don't remember the exact details. Poking through the wxMenu documentation should reveal it if I'm right.

Found it: in the menu entry name portion, it would be:

"Save &As\tCtrl-S"

The tab character may show up as backslash t. The issue is that save as command is bound to WX_SAVEAS constant, which gets translated automatically to all supported languages. Adding a translatable string for the save as menu entry would be trivial, as it allows defining the accelerator along with the command (translators should be told not to change Control+S shortcut though).
Thanks.

Comment 7 by Joseph Lee <joseph.lee22590@... on 2014-10-07 06:53
In [0247143]:

Log Viewer: add Control+S accelerator via wx.Menu item for Save as so it can be translated by translators. re #4532

Comment 8 by jteh on 2014-10-07 07:03
While we're at it, that should probably be "Save as..."; note the ellipsis. Also, will Ctrl+S work (instead of Ctrl-S)? The "+" sign is more standard here.

Comment 9 by nvdakor (in reply to comment 8) on 2014-10-07 07:14
Replying to jteh:

While we're at it, that should probably be "Save as..."; note the ellipsis. Also, will Ctrl+S work (instead of Ctrl-S)? The "+" sign is more standard here.

Done - WX accepts Ctrl+S (follows MS conventions).
Thanks.

Collaborator

derekriemer commented Nov 15, 2015

Hi guys, Pretty sure &As will bind to a not s. Shouldn't that be "save A&s"? That's how I did it in wx last time I did this sort of work.

Contributor

dkager commented Nov 20, 2015

Pretty sure &As will bind to a not s.
Yup.

Shouldn't that be "save A&s"?
I would reserve the S for Save, which the log viewer doesn't have, and A for Save As. For that matter, Ctrl+S intuitively means to save without popping up a dialog to me. It's probably not a problem in this instance, but I wouldn't change the mnemonic.

Contributor

jcsteh commented Nov 20, 2015

  1. The shortcut in the menu is currently "s", even without this change.
  2. Even if we wanted to change it to "a", that doesn't affect the binding of the control keystroke.
  3. Control+s often does Save as if no file name has been specified. Try opening Notepad and just pressing control+s. This is always the case in the log viewer, so I think this is acceptable.
Contributor

jcsteh commented Nov 20, 2015

Oh, and the reason I bound it to "s" instead of "a" in the first place is that there is no plain "save" for the log viewer.

Contributor

dkager commented Nov 20, 2015

Regarding (1), I wasn’t aware of that as I tend to open the log externally. That kinda voids my point. :)

From: James Teh [mailto:notifications@github.com]
Sent: Friday, November 20, 2015 14:00
To: nvaccess/nvda nvda@noreply.github.com
Cc: Davy Kager mail@davykager.nl
Subject: Re: [nvda] assign Ctrl S keystroke to Save As command in NVDA Log Viewer (#4532)

  1. The shortcut in the menu is currently "s", even without this change.
  2. Even if we wanted to change it to "a", that doesn't affect the binding of the control keystroke.
  3. Control+s often does Save as if no file name has been specified. Try opening Notepad and just pressing control+s. This is always the case in the log viewer, so I think this is acceptable.


Reply to this email directly or view it on GitHub #4532 (comment) . https://github.com/notifications/beacon/AC9y8f5VFpc1u9dGJRUcTxzJAFcxVl0hks5pHxC9gaJpZM4GigOc.gif

@jcsteh jcsteh added a commit that referenced this issue Mar 16, 2016

@josephsl @jcsteh josephsl + jcsteh In the Log Viewer, you can now save the log using the shortcut key co…
…ntrol+s.

Also, the accelerator in the menu is now a instead of s, as is more standard.
Fixes #4532.
1de3c73

Incubated in 79a5a2d.

jcsteh was assigned by nvaccessAuto Mar 16, 2016

jcsteh added this to the 2016.2 milestone Mar 31, 2016

jcsteh closed this in 290813a Mar 31, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment