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

Speech Viewer: allow to close with alt+F4 & add a close button on the title bar for use with pointing devices (#10791) #12330

Merged

Conversation

JulienCochuyt
Copy link
Collaborator

Link to issue number:

Fixes #10791

Summary of the issue:

The Speech Viewer currently has no close button nor can be closed with alt+F4.

As described by @Qchristensen in #10791 (comment), most dialogs in NVDA can be closed with alt+F4.
As argued by @bhavyashah in #10791 (comment), the Speech Viewer is especially useful for sighted testers who might be more familiar in using pointing devices than keyboard shortcuts.

Description of how this pull request fixes the issue:

Handle closing with alt+F4 & add a standard close button in the title bar of the dialog.

Testing strategy:

Tested all combinations of "show on start-up" and not, opening/closing with the menu/gesture/button of both Speech and Braille viewers.
Ensured the checked state of the menu entry does reflect the state of the dialog.

Known issues with pull request:

It might be interesting to make the dialog freeze then forcibly close it to ensure the menu entry checked state remains consistent in that case.

Change log entries:

Changes
The Speech Viewer can now be closed with alt+F4 and has a standard close button for easier interaction with users of pointing devices. The other ways of interacting with this dialog remain otherwise unchanged.

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@JulienCochuyt JulienCochuyt requested a review from a team as a code owner April 23, 2021 18:11
@seanbudd seanbudd self-assigned this Apr 26, 2021
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I seem to be unable to add the changelog update to changes.t2t - I'm guessing that the box to allow edits from maintainers isn't checked. Could you insert the changelog update or check this?

source/speechViewer.py Outdated Show resolved Hide resolved
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@JulienCochuyt JulienCochuyt marked this pull request as draft April 28, 2021 07:59
@JulienCochuyt
Copy link
Collaborator Author

@seanbudd wrote:

Looks good to me, but I seem to be unable to add the changelog update to changes.t2t - I'm guessing that the box to allow edits from maintainers isn't checked. Could you insert the changelog update or check this?

Sadly, this box is only available for PRs made from forks owned by users but not for forks owned by organizations.
I've just invited you to gain write access on our forked repo.
I've had to do the same for Reef a while ago.

@JulienCochuyt JulienCochuyt marked this pull request as ready for review April 28, 2021 08:18
…tions/nvda into i10791-speechViewerCloseButton
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks for adding me to the repo

@seanbudd seanbudd merged commit f84a7d9 into nvaccess:master Apr 29, 2021
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone Apr 29, 2021
@JulienCochuyt JulienCochuyt deleted the i10791-speechViewerCloseButton branch April 29, 2021 06:21
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.

Cannot close speech viewer with alt+f4
3 participants