Update InfoBar narrator behavior #4578

marcelwgn · 2021-03-18T23:44:19Z

Description

Now, the InfoBar will behave as follows:

Close button will be read as "Close infobar severity"
Should be announced as "Title Message Severity"
Default icons can be read and will be announced as "Icon severity ..."

Motivation and Context

Closes #4533 , closes #4534

How Has This Been Tested?

Tested manually.

Screenshots (if appropriate):

…ty level

dev/InfoBar/InfoBar.cpp

marcelwgn · 2021-03-18T23:47:31Z

dev/InfoBar/Strings/en-us/Resources.resw

-  <data name="InfoBarCloseButtonName" xml:space="preserve">
-    <value>Close</value>
-    <comment>Automation name of the close button.</comment>
+  <data name="InfoBarCloseButtonNameSeverityError" xml:space="preserve">


Here and below: @gabbybilka feel free to suggest better text that should be used to announce the respective information, those were just a short text I used to plan and test things out.

ranjeshj · 2021-03-20T19:09:36Z

@gabbybilka @YuliKl can you help define what should be announced ? "Close infobar error" sounds incorrect. Perhaps it could be
"Informational infobar close button"
"Success infobar close button"
"Warning infobar close button"
"Error infobar close button"

where the first word matches the severity enum.

marcelwgn · 2021-03-20T20:38:55Z

Updated the wording for the close button now.

dev/InfoBar/Strings/en-us/Resources.resw

gabbybilka · 2021-03-22T17:27:36Z

dev/InfoBar/Strings/en-us/Resources.resw

+    <comment>The formatted string read by narrator when the InfoBar opens: "{Title} {Message}, {severity level}"</comment>
+  </data>
+  <data name="InfoBarSeverityErrorName" xml:space="preserve">
+    <value>severity error</value>


@YuliKl, I think this should be "Info message, error severity" to give context that the InfoBar is a message before the content is read out. Notification could also work here?

Or just "Error InfoBar" to match the close button text.

Updated wording now.

gabbybilka · 2021-03-22T17:29:41Z

dev/InfoBar/Strings/en-us/Resources.resw

@@ -133,8 +145,40 @@
    <value>InfoBar</value>
    <comment>This is the custom landmark used to denote an InfoBar to narrator.</comment>
  </data>
+  <data name="InfoBarIconSeverityErrorName" xml:space="preserve">
+    <value>Icon severity error</value>


@chingucoding Is it possible for the value to be the name of the icon if the icon is either the default or changed via resource from the default font set in generic.xaml?
It would be great if a user-provided icon set in IconSource had a name that we could set here as well.
@StephenLPeters has anything similar been done in other controls? Does Teaching Tip announce its icon?
@YuliKl I'm not sure of the value this announcement would provide with the current text considering the severity of the InfoBar will be announced when it opens. Maybe for when a user returns to the InfoBar after the initial announcement?

No, today, icon's are entirely ignored by narrator.

@gabbybilka Current behavior, if you use the build in icons, we use our texts for UIA. If the user specify their own icon, they should be in charge of the UIA behavior of that (at least we are not interfering there to any degree).

ranjeshj · 2021-03-25T12:13:29Z

/azp run

azure-pipelines · 2021-03-25T12:13:40Z

Azure Pipelines successfully started running 1 pipeline(s).

johnpf74 · 2021-03-26T00:09:22Z

@gabbybilka @YuliKl can you help define what should be announced ? "Close infobar error" sounds incorrect. Perhaps it could be
"Informational infobar close button"
"Success infobar close button"
"Warning infobar close button"
"Error infobar close button"

where the first word matches the severity enum.

Should there be similar text for the action button?

ranjeshj · 2021-04-06T19:12:08Z

@chingucoding Sorry for the delayed response on this. We have been trying to reach out and get more information from accessibility experts on the correct approach for these issues. This is what we have finally landed on (which I think is simpler and gives a better accessibility experience).

• Set AutomationProperty.IsDialog=”true” in the default style so that the InfoBar elements (Icon + Title + Message) are read when an action or close button is first focused. When focus leaves an InfoBar and returns, the InfoBar elements will be re-read. This is special behavior that Narrator has for elements with this property set to true which works pretty well for InfoBar.
• Add Icon to the UIA tree to communicate that element as ‘Information image’, ‘Success image’, ‘Error image’, ‘Warning image’
• When the Severity, Title, or Message has changed the InfoBar should re-raise its notification event. Note that if multiple of these change one after the other, we raise the event just once after all of them are set.

@gabbybilka and @johnpf74 as FYI.

gabbybilka · 2021-04-06T19:20:54Z

@chingucoding Sorry for the delayed response on this. We have been trying to reach out and get more information from accessibility experts on the correct approach for these issues. This is what we have finally landed on (which I think is simpler and gives a better accessibility experience).

• Set AutomationProperty.IsDialog=”true” in the default style so that the InfoBar elements (Icon + Title + Message) are read when an action or close button is first focused. When focus leaves an InfoBar and returns, the InfoBar elements will be re-read. This is special behavior that Narrator has for elements with this property set to true which works pretty well for InfoBar.
• Add Icon to the UIA tree to communicate that element as ‘Information image’, ‘Success image’, ‘Error image’, ‘Warning image’
• When the Severity, Title, or Message has changed the InfoBar should re-raise its notification event. Note that if multiple of these change one after the other, we raise the event just once after all of them are set.

Also, to clarify a few points:

Can you remove the resource names for the close button? Since we're setting AutomationProperty.IsDialog to true the close button (and action button) will have the appropriate context. The extra info associated with the close button would be too much.
For the InfoBarOpened notification instead of the 'Info message severity error' text for the first value, please use the text for the icon that is now added to the UIA tree.

Thanks so much!

marcelwgn · 2021-04-07T18:16:48Z

I think the changes are implemented now @ranjeshj and @gabbybilka . The icon already was in the UIA tree. Regarding reanouncing the InfoBar, I am not sure how to implement or how to prevent consecutive changes to not overlap.

gabbybilka

@chingucoding

Regarding reanouncing the InfoBar, I am not sure how to implement or how to prevent consecutive changes to not overlap.

@StephenLPeters do you have any recommendations for how to implement this so that they don't overlap? Thanks both!

dev/InfoBar/InfoBar.cpp

gabbybilka · 2021-04-08T00:15:06Z

dev/InfoBar/Strings/en-us/Resources.resw

+  <data name="InfoBarIconSeverityWarningName" xml:space="preserve">
+    <value>Icon severity warning</value>
+    <comment>Automation name used to announce the severity warning icon.</comment>
+  </data>
  <data name="InfoBarOpenedNotification" xml:space="preserve">


After testing, this string is no longer read when the InfoBar is opened. @ranjeshj as FYI but I think that is fine and preferred as the string generated by setting isDialog to true is announced instead.
In case my test was faulty, can you update the InfoBarSeverity{Severity}Name values to be "{Severity} icon" for consistency with the icon names? 😊

gabbybilka · 2021-04-08T00:15:54Z

dev/InfoBar/Strings/en-us/Resources.resw

+    <comment>Automation name used to announce the severity error icon.</comment>
+  </data>
+  <data name="InfoBarIconSeverityInformationalName" xml:space="preserve">
+    <value>Icon severity informational</value>


Can each of these icon names be updated to "{Severity} icon"?

"Informational icon"

"Success icon"

"Warning icon"

"Error icon"

@chingucoding before merging in, can you update these names?

Oh yes, of course. Not sure how I missed this. should be updated now.

ranjeshj · 2021-04-08T13:46:34Z

I think the changes are implemented now @ranjeshj and @gabbybilka . The icon already was in the UIA tree. Regarding reanouncing the InfoBar, I am not sure how to implement or how to prevent consecutive changes to not overlap.

One way might be to batch the changes and raise the automation event on an event that happens later, like say in a LayoutUpdated event and immediately unsubscribe from that LayoutUpdated event.

beervoley · 2021-04-12T18:20:34Z

/azp run

azure-pipelines · 2021-04-12T18:20:45Z

Azure Pipelines successfully started running 1 pipeline(s).

marcelwgn · 2021-04-16T17:37:19Z

I don't really understand why the tests now fail with the changes made. All changes are in the UIA layer, none of them touch the Visual Tree from a rendering standpoint (only thing I can think of is a reference to elements on the visual tree). But I also don't really understand how the actual asserts in the failing test

microsoft-ui-xaml/dev/InfoBar/InteractionTests/InfoBarTests.cs

Line 136 in 3920257

public void LayoutTest()

or why they were designed the way they are. @teaP can you maybe help me out a bit here?

beervoley · 2021-04-16T18:46:25Z

/azp run

azure-pipelines · 2021-04-16T18:46:36Z

Azure Pipelines successfully started running 1 pipeline(s).

ranjeshj · 2021-04-19T18:23:59Z

Weird. The changes are purely accessibility related so I'm not sure why layout test is failing. @teaP @StephenLPeters any thoughts ? @chingucoding can you merge in master so we can be sure something is not stale ?

Also, lets make the UpdateLayout part a separate PR. Thanks.

marcelwgn · 2021-04-19T19:46:51Z

Merged with master now @ranjeshj .

Not sure what UpdateLayout part you are referring to here since all changes seem to be purely a11y wise, nothing related to the layout.

teaP · 2021-04-19T19:54:22Z

/azp run

azure-pipelines · 2021-04-19T19:54:33Z

Azure Pipelines successfully started running 1 pipeline(s).

teaP · 2021-04-19T19:55:24Z

I looked at this but I don't know what's happening either. The test seems to be failing because the strings are no longer top-aligned, but I'm not sure how that could have changed... maybe it's because the UIA bounding box was affected somehow?

marcelwgn · 2021-04-19T21:08:33Z

I looked at this but I don't know what's happening either. The test seems to be failing because the strings are no longer top-aligned, but I'm not sure how that could have changed... maybe it's because the UIA bounding box was affected somehow?

Is the bounding box affected by that? I thought it was just a purely "visual" aspect of a control, not something related to the UIA tree.

ranjeshj · 2021-04-20T15:54:58Z

/azp run

azure-pipelines · 2021-04-20T15:55:10Z

Azure Pipelines successfully started running 1 pipeline(s).

ranjeshj · 2021-04-20T18:13:48Z

@teaP @chingucoding can you take a look visually and see if there is a difference ? If not perhaps the test needs to be updated.

marcelwgn · 2021-04-20T20:55:10Z

I wasn't able to find a visual difference, master branch and this PR seemed pretty much the same with the changes done in the test.

teaP · 2021-04-20T21:03:09Z

I wasn't able to find a visual difference, master branch and this PR seemed pretty much the same with the changes done in the test.

Yeah, same here. I'm trying to replace it with an API test that does the same thing now.

teaP · 2021-04-21T20:38:03Z

In this end, it was pretty simply -- I realized the test was indexing into the UIA children of InfoBar, and this change added the icon as the first child, so all the indices were 1 off. :)

teaP · 2021-04-21T20:38:21Z

/azp run

azure-pipelines · 2021-04-21T20:38:32Z

Azure Pipelines successfully started running 1 pipeline(s).

marcelwgn · 2021-04-21T20:39:59Z

In this end, it was pretty simply -- I realized the test was indexing into the UIA children of InfoBar, and this change added the icon as the first child, so all the indices were 1 off. :)

Totally didn't think about this. Thank you so much for fixing the test @teaP !

marcelwgn added 2 commits March 18, 2021 20:49

Update infobar strings to be more descriptive and dependent on severi…

7e5d976

…ty level

Add icon and announcer severity information

6458c6e

ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 18, 2021

marcelwgn commented Mar 18, 2021

View reviewed changes

dev/InfoBar/InfoBar.cpp Outdated Show resolved Hide resolved

marcelwgn commented Mar 18, 2021

View reviewed changes

StephenLPeters added accessibility Narrator, keyboarding, UIA, etc area-InfoBar team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Mar 19, 2021

ranjeshj requested review from gabbybilka, teaP, YuliKl and StephenLPeters March 19, 2021 04:16

Fix bug, update wording

6c7054d

gabbybilka reviewed Mar 22, 2021

View reviewed changes

Update wording

3be610b

CR feedback

1100c62

gabbybilka reviewed Apr 8, 2021

View reviewed changes

Merge branch 'master' into user/chingucoding/inforbar-narrator-updates

3920257

ranjeshj approved these changes Apr 19, 2021

View reviewed changes

gabbybilka self-requested a review April 19, 2021 19:01

Merge branch 'master' into user/chingucoding/inforbar-narrator-updates

5e10ec0

Update UIA name for icon

cb00894

Fix up indices

ba81e15

ranjeshj merged commit 1f7779d into microsoft:master Apr 22, 2021

marcelwgn deleted the user/chingucoding/inforbar-narrator-updates branch April 22, 2021 21:13

Update InfoBar narrator behavior #4578

Update InfoBar narrator behavior #4578

Conversation

marcelwgn commented Mar 18, 2021

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Choose a reason for hiding this comment

ranjeshj commented Mar 20, 2021

marcelwgn commented Mar 20, 2021

gabbybilka Mar 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranjeshj commented Mar 25, 2021

azure-pipelines bot commented Mar 25, 2021

johnpf74 commented Mar 26, 2021

ranjeshj commented Apr 6, 2021

gabbybilka commented Apr 6, 2021 • edited Loading

marcelwgn commented Apr 7, 2021

gabbybilka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranjeshj commented Apr 8, 2021

beervoley commented Apr 12, 2021

azure-pipelines bot commented Apr 12, 2021

marcelwgn commented Apr 16, 2021

beervoley commented Apr 16, 2021

azure-pipelines bot commented Apr 16, 2021

ranjeshj commented Apr 19, 2021

marcelwgn commented Apr 19, 2021

teaP commented Apr 19, 2021

azure-pipelines bot commented Apr 19, 2021

teaP commented Apr 19, 2021

marcelwgn commented Apr 19, 2021

ranjeshj commented Apr 20, 2021

azure-pipelines bot commented Apr 20, 2021

ranjeshj commented Apr 20, 2021

marcelwgn commented Apr 20, 2021

teaP commented Apr 20, 2021

teaP commented Apr 21, 2021

teaP commented Apr 21, 2021

azure-pipelines bot commented Apr 21, 2021

marcelwgn commented Apr 21, 2021

gabbybilka Mar 22, 2021 •

edited

Loading

gabbybilka commented Apr 6, 2021 •

edited

Loading