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 UI messages when an action cannot be performed #13500

Merged
merged 9 commits into from
Mar 24, 2022

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Mar 17, 2022

Link to issue number:

Closes #6549

Summary of the issue:

Some UI messages are missing when an action cannot be performed.
If a user should be notified if they cannot perform an action due to being in secure mode, using the windows store version or a modal dialog is blocking.

Description of how this pull request fixes the issue:

The following ui messages have been added and implemented

  • Not available in Windows Store version
  • Not available in secure context
  • Not available while dialog response required

These were found by searching for usages of config.isAppX globalVars.appArgs.secure and isModalMessageBoxActive, respectively.

Testing strategy:

  • secure context:
  • Modal message box:
    • Perform an action that is not available with a modal open
      • e.g. nvda+q while the about dialog is open
    • confirm message is spoken
  • Using Windows store version:
    • Perform an action that is not available when using the windows store version:
      • e.g. activating the python console
    • confirm message is spoken

Known issues with pull request:

Some messages may be confusing or unexpected

Change log entries:

Changes

- NVDA now announces when an action cannot be performed due to not being available:
  - in the Windows Store version
  - in a secure context
  - while waiting for a response to a modal dialog
  -

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

@seanbudd seanbudd marked this pull request as ready for review March 17, 2022 06:18
@seanbudd seanbudd requested a review from a team as a code owner March 17, 2022 06:18
@seanbudd seanbudd requested review from feerrenrut and removed request for a team March 17, 2022 06:18
Copy link
Contributor

@lukaszgo1 lukaszgo1 left a comment

Choose a reason for hiding this comment

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

While I agree with the general direction of letting user know why their action has no effect this should only be done in cases where:

  • The given code path is available in secure mode and
  • it is invoked directly by the user.

source/config/__init__.py Outdated Show resolved Hide resolved
source/config/__init__.py Outdated Show resolved Hide resolved
source/gui/__init__.py Outdated Show resolved Hide resolved
source/gui/contextHelp.py Outdated Show resolved Hide resolved
@seanbudd seanbudd marked this pull request as draft March 17, 2022 23:55
source/gui/blockAction.py Outdated Show resolved Hide resolved
@seanbudd
Copy link
Member Author

While I agree with the general direction of letting user know why their action has no effect this should only be done in cases where:

  1. The given code path is available in secure mode and
  2. it is invoked directly by the user.

I agree with (2) and hope that this PR meets that now.
I disagree with (1) as, new code paths can be created, such as creating the ability to bind a gesture to open something that is otherwise hidden in secure mode. The secure mode handling and message could be at a low level to capture many paths to the action.

@seanbudd seanbudd marked this pull request as ready for review March 18, 2022 02:02
@seanbudd seanbudd self-assigned this Mar 18, 2022
@feerrenrut feerrenrut added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 18, 2022
lukaszgo1
lukaszgo1 previously approved these changes Mar 18, 2022
Copy link
Contributor

@lukaszgo1 lukaszgo1 left a comment

Choose a reason for hiding this comment

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

Al my comments are addressed, and the introduction of the blockAction.when decorator makes this very readable. Thanks for your work on this @seanbudd !

Copy link
Collaborator

@CyrilleB79 CyrilleB79 left a comment

Choose a reason for hiding this comment

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

Please try to keep these messages as explicit as possible.

source/gui/blockAction.py Outdated Show resolved Hide resolved
source/gui/blockAction.py Show resolved Hide resolved
source/gui/blockAction.py Outdated Show resolved Hide resolved
source/gui/blockAction.py Outdated Show resolved Hide resolved
source/gui/blockAction.py Outdated Show resolved Hide resolved
source/gui/blockAction.py Outdated Show resolved Hide resolved
source/gui/blockAction.py Outdated Show resolved Hide resolved
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Generally looks good.
I'd suggest this targets 2022.2

source/gui/blockAction.py Outdated Show resolved Hide resolved
@seanbudd seanbudd changed the base branch from beta to master March 22, 2022 23:07
add appx ui message

Add ui messages when blocked by modal message boxes, secure mode, and appx

move function out of gui init

revert additions to config init

create and use block action decorators

fix lint

Add documentation for block action when

improve message strings

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

change messageSting to translatedMessage

remove unused import

remove missed redundant code

fix when documentation

Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>

fix docs
@seanbudd
Copy link
Member Author

seanbudd commented Mar 23, 2022

I've squashed and rebased the approved commit onto master in b5e171b . This is so the PR can target 2022.2

source/gui/blockAction.py Outdated Show resolved Hide resolved
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
@AppVeyorBot

This comment was marked as outdated.

user_docs/en/changes.t2t Outdated Show resolved Hide resolved
@seanbudd seanbudd merged commit 55cf2b1 into master Mar 24, 2022
@seanbudd seanbudd deleted the addUISecureModeMessages branch March 24, 2022 05:46
michaelDCurran pushed a commit that referenced this pull request Mar 27, 2022
…ode when trying to open the symbol dialog. (#13539)

Fix-up of #13535
Follow-up of #13500

Summary of the issue:
In #13500 a decorator has been introduced to speak a message when an action is unavailable in secure mode.
In parallel an NVDA 2021.3.5 patch release has been produced; this release contains a fix preventing to open the symbol dialog in secure mode. The 2021.3.5 (rc) branch has then been merged into master to get this fix in master.
This led to the fact that no message was reported when trying to use a script to open the symbol dialog in secure mode.

Description of how this pull request fixes the issue:
Added the blockAction.when decorator where it was missing.
CyrilleB79 added a commit to CyrilleB79/nvda that referenced this pull request Mar 28, 2022
1. `@blockAction.when` now checks and reports first secure mode, then modal.
2. Add secure mode decorators to future-proof `onCheckForUpdateCommand`.
3. Use secure mode decorator for `onSaveConfigurationCommand`.
CyrilleB79 added a commit to CyrilleB79/nvda that referenced this pull request Mar 28, 2022
1. `@blockAction.when` now checks and reports first secure mode, then modal.
2. Add secure mode decorators to future-proof `onCheckForUpdateCommand`.
3. Use secure mode decorator for `onSaveConfigurationCommand`.
seanbudd pushed a commit that referenced this pull request Mar 30, 2022
1. `@blockAction.when` now checks and reports first secure mode, then modal.
2. Add secure mode decorators to future-proof `onCheckForUpdateCommand`.
3. Use secure mode decorator for `onSaveConfigurationCommand`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@secure decorator.
6 participants