Skip to content

Default gesture for the screen curtain#15590

Merged
seanbudd merged 3 commits into
nvaccess:masterfrom
CyrilleB79:addGesture
Oct 9, 2023
Merged

Default gesture for the screen curtain#15590
seanbudd merged 3 commits into
nvaccess:masterfrom
CyrilleB79:addGesture

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #10560

Summary of the issue:

The screen curtain is missing a default gesture (working out of the box). Users want to have a default gesture in order not to have to define one themselves.

Description of user facing changes

NVDA+control+escape is defined as default gesture.

The gesture has been discussed in #10560:

  • The main key is quite easy to find on a keyboard; e.g. PrintScreen is much harder because it's not always at the same place or sometimes behind Fn key.
  • Usage of punctuation key for main key should be avoided because it differs from a local keyboard layout to another. Even if it may be translated by translators, the issue remains for people switching their keyboard layout. Punctuation keys are better suited for personal gestures.
  • shortcut using 3 keys and not two due to the probable unfrequency of use with respect to other gestures/commands
  • with this gesture, there is no risk to activate the screen curtain by accident.

Description of development approach

Added in script's decorator and updated the documentation.

Testing strategy:

Tested this gesture in NVDA started with a blank config folder.

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 CyrilleB79 marked this pull request as ready for review October 6, 2023 14:53
@CyrilleB79 CyrilleB79 requested review from a team as code owners October 6, 2023 14:53
@XLTechie

XLTechie commented Oct 6, 2023 via email

Copy link
Copy Markdown
Collaborator

@codeofdusk

codeofdusk commented Oct 7, 2023

Copy link
Copy Markdown
Contributor

I'm not convinced that we need this. The screen curtain is an advanced feature and given various keyboard layouts users can easily assign a gesture that works for them.

@XLTechie

XLTechie commented Oct 7, 2023 via email

Copy link
Copy Markdown
Collaborator

@CyrilleB79

CyrilleB79 commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

@codeofdusk, wrote:

I'm not convinced that we need this. The screen curtain is an advanced feature and given various keyboard layouts users can easily assign a gesture that works for them.

I am a bit surprised by this comment.

From a user point of view, the need for such has been expressed and discussed on the mailing list as well as in #10560 by many people (you included).

IMO, screen curtain is not an advanced feature. And this seems the opinion of the majority as shown by the evidences provided by @XLTechie. It is present with a default shortcut key in all other screen readers, even in phone screen readers where there are less default gestures available (touch only gestures and very few keys).

Also #10560 has been triaged by NV Access, meaning that they seem to agree with this idea.

So could you be more specific and give more context:

  • What lets you think that screen curtain would be an advanced feature that does not deserve a gesture?
  • What is the problem with the various keyboard layout with regard to the gesture that was chosen?
    Thanks.

@LeonarddeR

LeonarddeR commented Oct 7, 2023

Copy link
Copy Markdown
Collaborator

I was a bit surprised by the quite complex gesture at first sight, then indeed realized that we need to use a gesture for this that isn't easy to press by accident. I'm happy with this gesture.

@CyrilleB79

CyrilleB79 commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

Note: Although the gesture has already discussed in #10560, it is still time to discuss it here for 2-3 days if new arguments are provided.

I realize that Jaws and VO both use the F11 key. Standardization is of course not mandatory, but is there an advantage for us to also standardize with an F11 combination, e.g. NVDA+control+F11.
On the other hand, NVDA+control+F11 is a bit less easy to execute than NVDA+control+Escape, the Escape key being very easily to find on the keyboard.

Let's wait for NV Access opinion before changing anything.

@amirsol81

amirsol81 commented Oct 7, 2023

Copy link
Copy Markdown

Thanks for this highly needed key stroke! It was strange that NVDA didn't provide one out of the box - unlike almost all screen readers.

@brunoprietog

brunoprietog commented Oct 8, 2023

Copy link
Copy Markdown

I love this, however, why don't we just copy the Voiceover gesture on Mac (and all other Apple operating systems where you can use a keyboard)? NVDA + Shift + F11? NVDA + Shift + Escape, by the way, looks a lot like the gesture you use to open the task manager

seanbudd
seanbudd reviewed Oct 9, 2023
View reviewed changes
Comment thread user_docs/en/userGuide.t2t Outdated
Comment thread user_docs/en/userGuide.t2t Outdated
@CyrilleB79 @seanbudd
Apply suggestions from code review
ae72476 
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
seanbudd
seanbudd reviewed Oct 9, 2023
View reviewed changes
Comment thread user_docs/en/changes.t2t Outdated
seanbudd
seanbudd approved these changes Oct 9, 2023
View reviewed changes

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @CyrilleB79

Qchristensen
Qchristensen approved these changes Oct 9, 2023
View reviewed changes

@Qchristensen Qchristensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will be a welcome addition for many people, great work!

@seanbudd seanbudd merged commit cb70a4a into nvaccess:master Oct 9, 2023
@nvaccessAuto nvaccessAuto added this to the 2024.1 milestone Oct 9, 2023
@CyrilleB79 CyrilleB79 mentioned this pull request Oct 10, 2023
Closed
@CyrilleB79 CyrilleB79 deleted the addGesture branch October 10, 2023 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

@Qchristensen Qchristensen Qchristensen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2024.1

Development

Successfully merging this pull request may close these issues.

Assign a gesture for the screen curtain by default

9 participants

@CyrilleB79 @XLTechie @codeofdusk @LeonarddeR @amirsol81 @brunoprietog @seanbudd @Qchristensen @nvaccessAuto