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

Add UIA element grouping to SettingsContainer #15756

Merged
merged 2 commits into from
Jul 25, 2023

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Jul 24, 2023

Adds an AutomationProperty.Name to the main grid in the SettingContainer. Doing so makes it so that the group of elements is considered a "group <header>".

Now, when navigating with a screen reader, when you enter the group of elements, the "group <header>" will be presented. Thus, if the user navigates to the "reset" button, it'll be prefaced with a "group <header>" announcement first. If the user navigates to it from the other direction (the setting control), this announcement isn't made, but the user already has an understanding of what group of settings they're in, which is standard practice.

Closes #15158

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Accessibility Issues related to accessibility Product-Terminal The new Windows Terminal. labels Jul 24, 2023
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runformat I guess?

@DHowett
Copy link
Member

DHowett commented Jul 25, 2023

Wait, though. Does this not have anything to do with our actual groupings?

image

Do we have an affordance for that? What does that look like to a screen reader?

@DHowett
Copy link
Member

DHowett commented Jul 25, 2023

I guess it's not related to the bug report so it's not a problem, but I am curious!

@carlos-zamora
Copy link
Member Author

Yeah! I tried doing a similar thing for those groupings and I couldn't get it to work. Figured it's not related to the bug report so yeah, not a problem haha.

To be clear, I tried adding an AutomationProperties.Name to the StackPanel with a Binding, x:Bind, setting it via the code, and setting it in the XAML directly to the value. None worked -.-

@DHowett
Copy link
Member

DHowett commented Jul 25, 2023

Fair enough!

@carlos-zamora carlos-zamora merged commit d70794a into main Jul 25, 2023
@carlos-zamora carlos-zamora deleted the dev/cazamor/a11y/groupings branch July 25, 2023 20:42
DHowett pushed a commit that referenced this pull request Jul 27, 2023
Adds an `AutomationProperty.Name` to the main grid in the `SettingContainer`. Doing so makes it so that the group of elements is considered a "group \<header\>".

Now, when navigating with a screen reader, when you enter the group of elements, the "group \<header\>" will be presented. Thus, if the user navigates to the "reset" button, it'll be prefaced with a "group \<header\>" announcement first. If the user navigates to it from the other direction (the setting control), this announcement isn't made, but the user already has an understanding of what group of settings they're in, which is standard practice.

Closes #15158

(cherry picked from commit d70794a)
Service-Card-Id: 89990838
Service-Version: 1.17
DHowett pushed a commit that referenced this pull request Jul 27, 2023
Adds an `AutomationProperty.Name` to the main grid in the `SettingContainer`. Doing so makes it so that the group of elements is considered a "group \<header\>".

Now, when navigating with a screen reader, when you enter the group of elements, the "group \<header\>" will be presented. Thus, if the user navigates to the "reset" button, it'll be prefaced with a "group \<header\>" announcement first. If the user navigates to it from the other direction (the setting control), this announcement isn't made, but the user already has an understanding of what group of settings they're in, which is standard practice.

Closes #15158

(cherry picked from commit d70794a)
Service-Card-Id: 89990839
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
Development

Successfully merging this pull request may close these issues.

[Settings - Line Height] : Label 'Line Height' is not associated with the 'Reset' button.
4 participants