-
-
Notifications
You must be signed in to change notification settings - Fork 626
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
Conversation
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.
…ter controls. Re nvaccess#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.
… toggle button. Re nvaccess#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.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be add the word cached to this variable to make it clear that this is used for comparison purposes.
Hi, regarding word choice, yes, I’ll change that (borrowed from @jcsteh’s usage in the past). Thanks.
From: Leonard de Ruijter <notifications@github.com>
Sent: Wednesday, November 28, 2018 6:15 AM
To: nvaccess/nvda <nvda@noreply.github.com>
Cc: Joseph Lee <joseph.lee22590@gmail.com>; Author <author@noreply.github.com>
Subject: Re: [nvaccess/nvda] Windows 10/Action Center: announce item status messages for brightness, focus assist and others, as well as reclassify brightness control as a button (#8954)
@LeonarddeR approved this pull request.
_____
In source/appModules/shellexperiencehost.py <#8954 (comment)> :
@@ -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.
May be remove the word argh here to make this comment look somewhat more professional 😜
_____
In source/appModules/shellexperiencehost.py <#8954 (comment)> :
@@ -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
May be add the word cached to this variable to make it clear that this is used for comparison purposes.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#8954 (review)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgLkCESBQB-xb56q3IW3BoxxVt2Uv-Wks5uzppGgaJpZM4Ynsd-> .
|
Comment: make it more professional. _itemStatusMessage -> _itemStatusMessageCache for readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
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. |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
…ache no matter what the new value is. Re nvaccess#8845.
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:
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:
Thanks.