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

Windows 10/Action Center: announce item status messages for brightness, focus assist and others, as well as reclassify brightness control as a button #8954

Merged

Conversation

Projects
None yet
4 participants
@josephsl
Copy link
Collaborator

josephsl commented Nov 17, 2018

Link to issue number:

Fixes #8845

Summary of the issue:

Announce status messages for brightness and focus assist changes in Action Center, as well as make brightness button a plain button.

Description of how this pull request fixes the issue:

Implements the following:

  1. Adds UIA item status property ID to property change events map so other modules can pick it up.
  2. Reclassifies brightness button in Windows 10's Action Center to a button.
  3. Allows NVDA to announce status messages for toggle controls such as brightness and focus assist.

Note: for brightness control, this pull request will work on Windows 10 Version 1809 (October 2018 Update). In 19H1 (build 18277), a slider replaces brightness button.

Testing performed:

Tested on Windows 10 Version 1809 (build 17763), April 2018 Update (build 17134), 19H1 (builds 182xx) and others. Tested via both source code changes and through Windows 10 App Essentials add-on.

Known issues with pull request:

None

Change log entry:

Bug fixes:

  • In Windows 10's Action Center, NVDA will announce status messages when toggling quick actions such as brightness and focus assist.
  • In action Center in Windows 10 October 2018 Update and earlier, NVDA will recognize brightness quick action control as a button instead of a toggle button.

Thanks.

josephsl added some commits Nov 17, 2018

UIA handler: add item status property change event. Re #8845.
Some controls raise UIA item status property change event, allowing clients to announce status changes. This is most noticeable in Windows 10's Action Center where one can change brightness and Focus Assist (quiet hours) where toggling these controls will cause them to raise this event.
Shell Experience Host: announce status changes for various Action Cen…
…ter controls. Re #8845.

Thanks to UIA's item status property change event, status for the following will be announced when toggling them:
* Brightness
* Focus Assist/quiet hours

Because item status property change is raised multiple times (or rather, NVDA announces status messages multiple times), have a string to hold last announced status.
Shell Experience Host: Brightness button is now a plain button, not a…
… toggle button. Re #8845.

Brightness button is now a plain button, not a toggle button. Note that this control is now a slider in Windows 10 19H1 build 18277 and later.

@josephsl josephsl requested a review from leonardder Nov 17, 2018

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Nov 17, 2018

Hi,

Actually, I'd welcome review from Mick and Jamie, too. Thanks.

@@ -16,3 +31,14 @@ def chooseNVDAObjectOverlayClasses(self,obj,clsList):
clsList.remove(ContentGenericClient)
except ValueError:
pass

# Argh, somehow, item status property repeats when Action Center is opened more than once.

This comment has been minimized.

Copy link
@leonardder

leonardder Nov 28, 2018

Collaborator

May be remove the word argh here to make this comment look somewhat more professional 😜

@@ -16,3 +31,14 @@ def chooseNVDAObjectOverlayClasses(self,obj,clsList):
clsList.remove(ContentGenericClient)
except ValueError:
pass

# Argh, somehow, item status property repeats when Action Center is opened more than once.
_itemStatusMessage = None

This comment has been minimized.

Copy link
@leonardder

leonardder Nov 28, 2018

Collaborator

May be add the word cached to this variable to make it clear that this is used for comparison purposes.

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Nov 28, 2018

josephsl added some commits Nov 28, 2018

UIA item status: address review comments. Re #8845.
Comment: make it more professional.
_itemStatusMessage -> _itemStatusMessageCache for readability.
@michaelDCurran
Copy link
Contributor

michaelDCurran left a comment

I see the following problems with this PR:

  • The name (e.g. focus assist) is announced when fully toggled off, even though there is no itemState. For example, toggling the focus assist button I hear: "focus assist: priority" "focus assist: alarms" "focus assist:".
  • I don't think it is necessary to hear the name of the control at all when it is toggled.
  • I think it is strange that the item status is announced when toggled, but not when the control is focused.
    I would suggest that you Create a specific toggleButton overlay class in the Action Center appModule which
  • implements the value property exposing itemStatus as its value.
  • Implements the UIA_itemStatus event which simply calls event_valueChange.
  • you could move the itemStatus cache onto this overlay class.

This all assumes that it is in deed the focused control that fires these itemStatus events?

@josephsl

This comment has been minimized.

Copy link
Collaborator Author

josephsl commented Jan 8, 2019

Hi,

You're right - overlay class simplifies things a lot. Commit on its way (along with a new snapshot of WinTenApps with changes included).

Thanks.

josephsl added some commits Jan 8, 2019

Action Center/toggle button: address review comments. Re #8845.
Reviewed by Mick Curran (NV Access): tweaks and changes:
* An overlay class will be used to handle toggle button value change state.
* Current item status cache will now be part of the new toggle button overlay class.
currentItemStatus = self.value
if currentItemStatus and currentItemStatus != self._itemStatusMessageCache:
ui.message(currentItemStatus)
self._itemStatusMessageCache = currentItemStatus

This comment has been minimized.

Copy link
@michaelDCurran

michaelDCurran Jan 9, 2019

Contributor

I don't think this should be within the if block, otherwise if you toggle something to not pressed and then pressed, and the itemStatus only has 1 pressed value, then when going back to pressed the itemStatusCache will match and it will say nothing.

josephsl and others added some commits Jan 9, 2019

Action Center toggle button object: address review comment - assign c…
…ache no matter what the new value is. Re #8845.

@michaelDCurran michaelDCurran merged commit aee76d1 into nvaccess:master Jan 10, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nvaccessAuto nvaccessAuto added this to the 2019.1 milestone Jan 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.