Skip to content

Add apropriate messages when interacting with empty status lines #8475

Merged
feerrenrut merged 7 commits into
nvaccess:masterfrom
lukaszgo1:i7789EmptyStatusLines
Jul 30, 2018
Merged

Add apropriate messages when interacting with empty status lines #8475
feerrenrut merged 7 commits into
nvaccess:masterfrom
lukaszgo1:i7789EmptyStatusLines

Conversation

@lukaszgo1

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #7789

Summary of the issue:

When user reads empty status line the message "blank" is spoken and when attempting to copy it nothing is announced.

Description of how this pull request fixes the issue:

When user attempts to read, spell or copy empty status line appropriate message is presented.

Testing performed:

Tested with Firefox status bar. I'm aware, that Firefox's status bar is simply a badly implemented and not exactly empty, but i cannot find a app with real empty status bar.

Known issues with pull request:

None

Change log entry:

Section: Bug fixes

When user interacts with empty status bar NVDA will now inform user about it.

@josephsl

josephsl commented Jul 2, 2018

Copy link
Copy Markdown
Contributor

Hi,

Apparently there is an indentation error in global commands file (line 1395), and Appveyor complains about this.

Thanks.

@josephsl josephsl 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,

Besides going over indentation error, you can do this by simply adding a check for empty text right before counting how many key presses were there. You can do it in two ways:

  1. Combine check for no status bar object along with empty text so NvDA will say, "no status line found".
  2. Adding a check for empty string, and if it is truly empty, you can say, "no status line text found".

Thanks.

@josephsl

josephsl commented Jul 2, 2018

Copy link
Copy Markdown
Contributor

Hi,

A tip: checking for empty text right before counting script count instead of going through each script count case makes this change way simpler to debug and saves bytecode generation.

Thanks.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

Hi @josephsl
Your approach has in my opinion one important disadvantage and it is the fact, that regardless of the user action the same message about empty status bar will be presented.

@josephsl

josephsl commented Jul 2, 2018 via email

Copy link
Copy Markdown
Contributor

@lukaszgo1

lukaszgo1 commented Jul 2, 2018 via email

Copy link
Copy Markdown
Contributor Author

@josephsl

josephsl commented Jul 2, 2018 via email

Copy link
Copy Markdown
Contributor

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

i've done it in this way, because in #7789 @feerrenrut says

Expected behavior:
I think something should be announced to acknowledge the command in both cases. In the first perhaps "no status bar information" in the the second "unable to copy status bar to clipboard"

@Brian1Gaff

Brian1Gaff commented Jul 3, 2018 via email

Copy link
Copy Markdown

@feerrenrut

Copy link
Copy Markdown
Contributor

Expected behavior:
I think something should be announced to acknowledge the command in both cases. In the first perhaps "no status bar information" in the the second "unable to copy status bar to clipboard"

I think the reason behind this was to highlight difference in the two actions. It reflects that, despite attempting both similar but distinct actions, neither is able to succeed.

@AAClause

Copy link
Copy Markdown
Contributor

Another thing if I may, instead of if text == '': I will do if not text.strip():.
Thus this way, you cover the case where the string consists only of spaces.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

@Andre9642 Thanks for the tip.

feerrenrut
feerrenrut previously approved these changes Jul 30, 2018
"Users are now informed when attempting to read or copy an empty status bar. (nvaccess#7789)"
@feerrenrut feerrenrut dismissed josephsl’s stale review July 30, 2018 08:00

The discussion around this review seems to be complete.

@feerrenrut feerrenrut merged commit e31b8f2 into nvaccess:master Jul 30, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.3 milestone Jul 30, 2018
@lukaszgo1 lukaszgo1 deleted the i7789EmptyStatusLines branch May 13, 2019 15:57
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.

Repeat read status bar command 3 times on empty status bar announces nothing

6 participants