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

Make SUI previews readable by screen readers #18418

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Jan 9, 2025

Summary of the Pull Request

Fixes a few accessibility bugs in the SettingContainer previews. Main changes include:

  • SettingContainer was considered a separate UIA element from the inner expander. It's been marked as AccessibilityView=Raw to "remove" it from the UIA tree.
  • Added a CurrentValueAccessibleName property to the SettingContainer to expose the current value to the screen reader for SettingContainers that have expanders. Non-expander SetttingContainers already worked fine.
  • Applied CurrentValueAccessibleName to various settings throughout the settings UI for full coverage. Added a CurrentValue for the ones that were missing it.
  • Removed a redundant/hidden tab stop in Icon

References and Relevant Issues

Padding was not updated since #18300 is handling that. This'll just automatically make it accessible.
Font axes and features weren't updated to show previews, but I'm happy to do it if given a suggestion.

Part of #18318

Detailed Description of the Pull Request / Additional comments

  • SettingContainer updates:
    • AccessibilityView = Raw for SettingContainers with expanders. This is because the expander itself is the one we care about. No need to have another layer of UIA objects saying it's a group.
    • Added a CurrentValueAccessibleName property
      • This specifically defines what should be read out by the screen reader, similar to AutomationProperties.Name
      • It updates automatically when CurrentValue changes.
      • It's applied on the inner Expander, if one exists.
      • The accessible name is constructed to be "<Header>: <CurrentValueAccessibleName>". If CurrentValueAccessibleName isn't provided, we try to use the CurrentValue if it's a string.
  • Profile (and appearance) settings:
    • Icon's value is now read out by a screen reader instead of staying silent. It'll read the icon path.
    • A redundant/hidden tab stop was removed from Icon.
    • TabTitle now displays/reads "None" if no tab title is set.
    • ColorScheme is now read out by a screen reader.
    • The color scheme overrides (i.e. Foreground, Background, SelectionBackground, and CursorColor) are now read out by a screen reader. Format is "#".
    • BackgroundImageAlignment is now displayed and read out by a screen reader.
  • LaunchSize is now displayed and read out by a screen reader. Format is "Width x Height".

Validation Steps Performed

Tabbed through the settings UI with a screen reader. Each of these settings now reads out a preview.

{
// the accessible name was not defined, so try to
// extract the value directly from the CurrentValue property
if (const auto currentValText{ currentVal.try_as<hstring>() })
Copy link
Member

Choose a reason for hiding this comment

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

i love this

@@ -103,6 +113,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
}

void SettingContainer::_OnCurrentValueChanged(const Windows::UI::Xaml::DependencyObject& d, const Windows::UI::Xaml::DependencyPropertyChangedEventArgs& /*e*/)
{
const auto& obj{ d.try_as<Editor::SettingContainer>() };
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const auto& obj{ d.try_as<Editor::SettingContainer>() };
const auto obj{ d.try_as<Editor::SettingContainer>() };

not necessary; decays to const auto anyway

@@ -205,6 +209,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
}

winrt::hstring LaunchViewModel::LaunchSizeCurrentValue() const
{
return winrt::hstring{ fmt::format(FMT_COMPILE(L"{} × {}"), InitialCols(), InitialRows()) };
Copy link
Member

Choose a reason for hiding this comment

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

love this!

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

God, this is very well done!

@DHowett DHowett enabled auto-merge (squash) January 21, 2025 23:09
Copy link
Contributor

@PankajBhojwani PankajBhojwani left a comment

Choose a reason for hiding this comment

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

This is awesome!

@DHowett DHowett merged commit 3e969d5 into main Jan 22, 2025
20 checks passed
@DHowett DHowett deleted the dev/cazamor/sui/preview-a11y branch January 22, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To Cherry Pick
Status: To Cherry Pick
Development

Successfully merging this pull request may close these issues.

3 participants