-
Notifications
You must be signed in to change notification settings - Fork 312
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
Add heading levels to Extensions page #3505
Add heading levels to Extensions page #3505
Conversation
@@ -48,107 +48,111 @@ | |||
Margin="0,0,0,28" /> | |||
|
|||
<!-- Installed items --> | |||
<Grid Margin="{StaticResource SettingsPageSubTitleMargin}"> | |||
<TextBlock x:Uid="InstalledTextBlock" | |||
<StackPanel x:Uid="InstalledItemsStackPanel" IsTabStop="True"> |
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 am not sure about the IsTabStop
variable. It is one more button the user needs to press to navigate the UI. And it does not affect the narration of the internal controls.
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.
Thanks for letting me know!
As of the most recent commit, I removed the TabStops from both headers. The "Installed" group is now read by the narrator, but the "Available in the Microsoft Store" header is not. I think this is because there is an interactable element (GetUpdatesButton) in the InstalledItemsStackPanel, so the group is automatically read with when the focus is on the button. The AvailableFromStoreStackPanel, on the other hand, has an ItemsRepeater with interactable subelements. Each of these is read as its own group. When I add an AutomationsProperties.Name to the ItemsRepeater, the heading isn't read. Do we need a CustomRepeaterAutomationPeer on the ItemsRepeater to read the heading or is there a better approach?
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.
Asking every user of DevHome to "Please change verbosity" is not the correct fix. We should either
- Find a work around to this issue, or
- Close the task as external and make a bug on the narrator.
I want to make sure I understand the second issue correctly. The issue is the narrator is not announcing the headers in the extension list. This is happening because the first interactable element isn't a top element. This results in narrator correctly announcing the tabbed-to element, but not the header. Does that sound right? |
Good to know that switching settings is not a fix. Let's find a resolution. I think we're on the same page:
Here's the current narrator behavior (default verbosity, scan off): current_narrator.mp4One workaround we considered was changing the extensions from a SettingsCard (which is its own group) to a button so that both the "Available in the Microsoft Store" group and the extension name were announced. We decided to not take this approach because we wanted to mimic the UI design of the Microsoft Store. Although the Microsoft Store has a similar two-group layout, the groups on its LibraryPage are announced because each group has buttons and/or navigation elements as children: Another team ran into a similar issue where a grandparent group was not read: https://task.ms/32064580. They had concluded that the narrator behavior was by design and didn't make any further changes. Therefore, I think best approach would be a workaround. I'm not sure where to start:
|
I've updated the current PR to address just the heading level fix. If I can find a workaround for the "Available in the Microsoft Store" group narration, I will make a separate PR for it. Otherwise, I will close that bug as external. |
<data name="InstalledItemsStackPanel.[using:Microsoft.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve"> | ||
<value>Installed</value> | ||
</data> | ||
<data name="AvailableFromStoreStackPanel.[using:Microsoft.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve"> | ||
<value>Available in the Microsoft Store</value> | ||
</data> |
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.
Do these actually get read or is it the TextBlocks inside?
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.
The InstalledItemsStackPanel.[using:Microsoft.UI.Xaml.Automation]AutomationProperties.Name
gets read as the group name (e.g. "Installed group"). If scan mode is on, it would read "Installed group; [and if you navigate to the heading] Heading level 2: Installed" (the TextBlock). If the narrator read the "AvailableFromStore" group, it would read its StackPanel's AutomationProperties.Name. If you navigate to the "Available in the Microsoft Store" heading in scan mode, it reads "Heading level 2: "Available in the Microsoft Store" (the TextBlock).
I think the narrator reads the StackPanel Automation Properties.Name as the group name because the item groups underneath them are ItemsRepeaters, not Lists or another control with a designated header.
Summary of the pull request
I enabled the narrator in scan mode to read the heading levels of the "Installed" and "Available in the Microsoft Store" headings on the Dev Home Extensions page by adding Automation.HeadingLevel properties to each text block.
References and relevant issues
https://task.ms/52443515
Detailed description of the pull request / Additional comments
Before:
03e67077-a961-4d1f-b8ca-aafefea96bd9.mp4
After changes:
heading_level_scan.mp4
Validation steps performed
PR checklist