Skip to content

Report 'unsupported' when toggling screen layout in Word#10795

Merged
feerrenrut merged 14 commits into
nvaccess:masterfrom
jakubl7545:i7297
Feb 23, 2021
Merged

Report 'unsupported' when toggling screen layout in Word#10795
feerrenrut merged 14 commits into
nvaccess:masterfrom
jakubl7545:i7297

Conversation

@jakubl7545

Copy link
Copy Markdown
Contributor

Link to issue number:

closes #7297

Summary of the issue:

Currently, when user tries to toggle screen layout in MS Word by pressing NVDA+v the letter is passed to the document. This can be confusing for some users who may expect screen layout to be available.

Description of how this pull request fixes the issue:

It adds a script to both Word modules which just speaks the appropriate message.

Testing performed:

In word with browse mode on and UIA support enabled or disabled pressed NVDA+v and noticed the message that this action is not supported.

Known issues with pull request:

None

Change log entry:

None needed.

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 97b3901ead

@AppVeyorBot

Copy link
Copy Markdown

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please move one instance of this to browseMode.BrowseModeDocumentTreeInterceptor. This ensures that the not supported message will be pronounced when pressing NVDA+V.
While at this, I think we should consider removing the default assignment for this script anyway.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 8d3876190a

@lukaszgo1

Copy link
Copy Markdown
Contributor

@LeonarddeR wrote:

While at this, I think we should consider removing the default assignment for this script anyway.

Any particular reason for doing so? Removing assignments which are there for years is extremely confusing for users IMHO.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Any particular reason for doing so? Removing assignments which are there for years is extremely confusing for users IMHO.

Well, apart from scarcity of gestures, screen layout is a thing specific to virtual bufffers, not just browse mode. In Word, Excel, Edge HTML, there is no support for screen lay out. Note that I'm only saying that I'd consider it, I wouldn't insist on removing the gesture. But, if there would ever be a good candidate for NVDA+v that might be more important than this one, I'd sacrifice it.

@jakubl7545

Copy link
Copy Markdown
Contributor Author

@LeonarddeR Done. As for removing assignment, I'll do it if such decision is made.

@feerrenrut feerrenrut added the bug label Apr 8, 2020
@LeonarddeR LeonarddeR self-requested a review October 10, 2020 17:21
LeonarddeR
LeonarddeR previously approved these changes Oct 10, 2020

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @jakubl7545 overall this looks good.

Please also update the script_toggleScreenLayout in virtualBuffers/__init__.py to use a @script decorator.

It means both implementations are visible in the history (making the override more clear). It also means both functions are using the same mechanism, and helps to move us towards replacing the old approach.

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 2335315a8a

@jakubl7545

Copy link
Copy Markdown
Contributor Author

The error says that 'scriptHandler.isScriptWaiting' is imported but unused in virtualBuffers/init.py. However, I see it in refreshBuffer script in that file. @feerrenrut I'm not sure what to do with that.

@feerrenrut

Copy link
Copy Markdown
Contributor

The error says that 'scriptHandler.isScriptWaiting' is imported but unused in virtualBuffers/init.py. However, I see it in refreshBuffer script in that file. @feerrenrut I'm not sure what to do with that.

hmm, that is an interesting case. This happens because we only send the diff to to flake8, so that PR's don't have to fix the whole file. The diff includes the changed imports, but not the usage.

Ah... also, note that the usage is 'scriptHandler.isScriptWaiting' and there is an import for scriptHandler and then the name isScriptWaiting is also imported explicitly? That name is not used, it is only used via scriptHandler.

I wouldn't remove it, because other files may depend on it. But we can clean it up a little and mark it as deprecated, to be removed in the future.

I'll add a suggestion in the diff.

Comment thread source/virtualBuffers/__init__.py Outdated
Comment thread source/virtualBuffers/__init__.py
@lukaszgo1

Copy link
Copy Markdown
Contributor

@feerrenrut Since this PR removes unused imports it'd be great to add it to the 2021.1 milestone to avoid it being forgotten before the release.

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good thanks @jakubl7545

@feerrenrut feerrenrut merged commit b4087a2 into nvaccess:master Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Word: Report 'unsupported' when pressing NVDA+V in browse mode

5 participants