Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new script to report accelerator key of currently focused object #13960

Merged
merged 9 commits into from
Aug 2, 2022

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented Jul 28, 2022

Link to issue number:

None (reported privately by @DHowett via @carlos-zamora).

Summary of the issue:

There is no way to simply obtain the accelerator key of the currently focused object without reading its entire description.

Description of how this pull request fixes the issue:

Adds a new script to report the accelerator key. By default, it is bound to Shift+numpad 2 (desktop) and Ctrl+Shift+NVDA+. (laptop). As a mnemonic, accelerator keys are (or at least used to be) visually displayed by an underlined character.

Testing strategy:

Tested Windows 11 Notepad (UIA), NVDA menus (MSAA), Google Chrome (MSAA), and Foobar 2000 (MSAA).

Known issues with pull request:

None known.

Change log entries:

== New Features ==

  • Added a new command to check the keyboard shortcut of the current focus (by default, Shift+Numpad 2 on desktop and Ctrl+Shift+NVDA+. on laptop).

Code Review Checklist:

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

@codeofdusk codeofdusk requested a review from a team as a code owner July 28, 2022 01:16
@codeofdusk codeofdusk requested a review from seanbudd July 28, 2022 01:16
source/globalCommands.py Outdated Show resolved Hide resolved
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
source/globalCommands.py Outdated Show resolved Hide resolved
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@seanbudd
Copy link
Member

@codeofdusk I'm not sure - it would be good to get an answer on this even if it's a separate issue / discussion / PR

@codeofdusk codeofdusk changed the title Add new unbound script to report accelerator key of currently focused object Add new script to report accelerator key of currently focused object Jul 28, 2022
@codeofdusk
Copy link
Contributor Author

@seanbudd It appears the issues I was having with MSAA apps were caused by me binding the script to a command that caused the menus to close. Setting a default gesture (based on the "current character" commands) seems to resolve the issue.

@codeofdusk codeofdusk requested a review from a team as a code owner July 28, 2022 04:15
@codeofdusk codeofdusk requested a review from Qchristensen July 28, 2022 04:15
@codeofdusk
Copy link
Contributor Author

PR now ready for review/merge.

Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@CyrilleB79
Copy link
Collaborator

I am a bit concerned by the use of Numpad for the default gesture.
Numpad key are usually used for interactions with the review cursor. Shouldn't we reserve the use of these keys for the review cursor?
@Qchristensen , @lukaszgo1 what do you think?

Could we find another key combination or keep it unbound by default?

@codeofdusk
Copy link
Contributor Author

I am a bit concerned by the use of Numpad for the default gesture.
Numpad key are usually used for interactions with the review cursor. Shouldn't we reserve the use of these keys for the review cursor?

@CyrilleB79 Similarity to review cursor keys is intended:

As a mnemonic, accelerator keys are (or at least used to be) visually displayed by an underlined character.

So this is the "current character" command with modifiers.

@CyrilleB79
Copy link
Collaborator

I am a bit concerned by the use of Numpad for the default gesture.
Numpad key are usually used for interactions with the review cursor. Shouldn't we reserve the use of these keys for the review cursor?

@CyrilleB79 Similarity to review cursor keys is intended:

As a mnemonic, accelerator keys are (or at least used to be) visually displayed by an underlined character.

So this is the "current character" command with modifiers.

I have understood that you want to find a similarity with a character command since accelerator key is an underlined character.
But IMO, it's more interesting to separate review command from the other ones than than finding an easy-to-remember gesture for this specific comand. Indeed, the difference between review cursor, browse mode cursor and system cursor is sometimes hard to understand for some users (not me); and all what can help to make this difference more noticeable is a plus.

Anyway, I have expressed my opinion now. But if I am the only one to think this way, just go forward with this PR as is; that's not so important.

@AppVeyorBot
Copy link

See test results for failed build of commit 87b7c54570

@AppVeyorBot
Copy link

See test results for failed build of commit 0e65abd4a1

@codeofdusk
Copy link
Contributor Author

@seanbudd Failure seems unrelated.

Copy link
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, will be well appreciated.

@seanbudd seanbudd merged commit 6a877eb into nvaccess:master Aug 2, 2022
@nvaccessAuto nvaccessAuto added this to the 2022.4 milestone Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants