Describe the bug
Steps to reproduce the bug
Steps to reproduce the behavior:
<UserControl
x:Class="Microsoft.PowerToys.Settings.UI.Views.ShellPage"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:behaviors="using:Microsoft.PowerToys.Settings.UI.Behaviors"
xmlns:winui="using:Microsoft.UI.Xaml.Controls"
xmlns:helpers="using:Microsoft.PowerToys.Settings.UI.Helpers"
xmlns:views="using:Microsoft.PowerToys.Settings.UI.Views"
xmlns:ic="using:Microsoft.Xaml.Interactions.Core"
xmlns:i="using:Microsoft.Xaml.Interactivity"
xmlns:muxc="using:Microsoft.UI.Xaml.Controls"
mc:Ignorable="d">
<Grid>
<muxc:RadioButtons Header="abcd">
<RadioButton>Item 1</RadioButton>
<RadioButton>Item 2</RadioButton>
<RadioButton>Item 3</RadioButton>
</muxc:RadioButtons>
</Grid>
</UserControl>
Expected behavior
When we tab into the radio button the narrator must read the heading as well as the radio button that is in focus.
Eg: something along the lines of "abcd grouping Item 1 button 1 of 3".
Actual Behavior
It just says "Item 1, 1 of 3". It does not read out the heading set by the radiobuttons header. Setting an AutomationProperty does not fix the issue either.
Screenshots
Version Info
NuGet package version:
Microsoft.UI.Xaml.2.5.0-prerelease.200708003
Windows app type:
| UWP | Win32 |
| :--------------- | :--------------- |
| Yes | |
| Windows 10 version | Saw the problem? |
| :--------------------------------- | :-------------------- |
| Insider Build (xxxxx) | |
| May 2020 Update (19041) | |
| November 2019 Update (18363) | |
| May 2019 Update (18362) | Yes |
| October 2018 Update (17763) | |
| April 2018 Update (17134) | |
| Fall Creators Update (16299) | |
| Creators Update (15063) | |
| Device form factor | Saw the problem? |
| :----------------- | :--------------- |
| Desktop | Yes |
| Xbox | |
| Surface Hub | |
| IoT | |
Additional context
Link to issue in the PowerToys repository - https://github.com/microsoft/PowerToys/issues/6032
@StephenLPeters @YuliKl as FYI.
I'm no expert with the implementation of accessibility info so I don't assume I have the complete picture here. That said, I took a closer look at this. According to the documentation, headered controls typically would use the AutomationProperties.LabeledBy API to read out the (text-based) header when a control gets focus. The issue here seems to be that LabeledBy
is supposed to work with text elements (and as such controls which expose text, like TextBox
or TextBlock
.) The header control of the RadioButtons
control, however, is a ContentPresenter
with a Content
API of type object
. Passing the ContentPresenter to LabeledBy
does not cause the header to be read, even if the actual ContentPresenter content is a string.
I did try to look into providing an own GetLabeledByCore implementation for the RadioButtons control here in the hope of working around this but I was not successful.
An attempt which did prove to be successful here is to create a TextBlock in "code behind", establish the LabeledBy relation with it and set its Text
property to the header content - if it's a string:
// In RadioButtons::OnApplyTemplate()
if (const auto stackPanel = GetTemplateChildT<winrt::StackPanel>(L"RootPanel", controlProtected))
{
m_headerUIATextBlock = winrt::TextBlock();
winrt::AutomationProperties::SetLabeledBy(stackPanel, m_headerUIATextBlock);
UpdateHeader();
}
void RadioButtons::UpdateHeader()
{
if (m_headerUIATextBlock != nullptr)
{
m_headerUIATextBlock.Text(SharedHelpers::TryGetStringRepresentationFromObject(Header()));
}
}
void RadioButtons::OnPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args)
{
winrt::IDependencyProperty property = args.Property();
if (property == s_HeaderProperty)
{
UpdateHeader();
}
// More properties left out here....
}
While this works, I am not sure if that is a reasonable solution here. Controls like TextBox
and ComboBox
also have a Header
property of type object
and Narrator reads out their header value correctly (if it's a string). So it might be reasonable to just re-use the existing code used for the ComboBox
,... in the RadioButtons
context as well.
@ranjeshj @StephenLPeters FYI.
(P.S.: Again, without me knowing the full picture, could AutomationProperties.LabeledBy
perhaps be updated to also work with UIElements which don't have a Text
property but a more generic Content
property? The Content
property could then be read out by Narrator if its value happens to be a textual value. That way, LabeledBy
would "just work" even if the bound UIElement is an element like a ContentPreseter
with content like "I'm a header".)
The TextBox hack is pretty funny but seems unreasonable. @YuliKl do you know the proper way to set up this relationship? @MikeHillberg we've been doing a lot of changing properties which would naively be strings to objects instead to allow for more customization. This could be a potential issue with that strategy?
I don't know the right way to implement this. The issue definitely feels like a bug we should try to fix. @kmahone any ideas?
The header is read out fine by Narrator for a ComboBox
, for example, which seems to have more or less the same setup as our RadioButtons
control here (it also uses a ContentPresenter
for its header). So my first thought here would be to just take a look at how the ComboBox implements this Narrator behavior and then just shamelessly copy/paste that logic into our RadioButtons
control.
Looking at ComboBoxAutomationPeer, we override GetNameCore, call the base to see if there is already a name set (through automation name property). If an automation name is not set, then we provide the header as the name.
@ranjeshj I will take a look if that will work here as well (I faintly recall I tried that a few days ago to no avail but perhaps I missed something).
@ranjeshj As I thought to recall, overriding GetNameCore() alone doesn't appear to work. Here is what I did:
I created a new RadioButtonsAutomationPeer
class and overrode GetNameCore()
like this:
hstring RadioButtonsAutomationPeer::GetNameCore()
{
winrt::hstring name = __super::GetNameCore();
if (name.empty())
{
if (const auto RadioButtons = Owner().try_as<winrt::RadioButtons>())
{
name = SharedHelpers::TryGetStringRepresentationFromObject(RadioButtons.Header());
}
}
return name;
}
When focus enters a RadioButtons control with a header "TestRadioButtons", the header isn't read out even though GetNameCore() returns the correct value:
Is the ComboBoxAutomationPeer doing anything else here?
I don't see anything else that might affect it in ComboBox. I do notice that in the case of ComboBox, ComboBox can take focus, so that might be when Narrator is reading it out. Following up with accessibility folks.
These are the 3 requirements for the screen reader to read it out.
Can you inspect the tree via Accessibility Insights and see we are meeting these. I suspect #3 is what might be missing.
@ranjeshj Sorry for the late reply.
I created the following test page:
As you can see, I have a ComboBox with the header "ComboBoxHeader" and a RadioButtons instance with the header "RadioButtonsHeader". Narrator reads out the ComboBox header correctly, but fails to mention the RadioButtons header when stepping into the RadioButtons control with focus,
Accessibility insights gives me the following for these two controls:
| Control | MUX UI | AI Windows |
|---|---|----|
| ComboBox | | |
| RadioButtons | | |
What I can notice is that for the ComboBox, the header is actually part of the control. For the RadioButtons control, however, it looks like the header is just acting as a stand-alone text component.
@ranjeshj Do these findings help you?
Note in PowerToys, we got flagged for accessibility on this
@ranjeshj Are the images above helpful and if yes, what would be the next steps here? (If not, please let me know what additional images/gifs I should upload.)
@StephenLPeters @MikeHillberg thoughts ?
@ranjeshj @StephenLPeters @MikeHillberg I want to point out that we still have the workaround mentioned above available to use if this should be fixed soon and providing proper support (this might be an ItemsRepeater issue where it might lack some support for a control header?) would take longer.
@Felix-Dev while yes but when @StephenLPeters responds with the item below, it doesn't exactly inspire confidence to adopt
The TextBox hack is pretty funny but seems unreasonable
also if we adopt the hack, how would the hack work with the fix as if adopted, we could take the fix without realizing it.
Maybe, to fix this "properly", from a UIA perspective, RadioButtons should be a panel or something similar so that Narrator will perceive it as a control with children. That way, it would announce the header and then the selected item while also allowing users to understand the control easier conceptually.
@crutkas I am not entirely sure I understand your comment. The workaround using the TextBlock would be WinUI internal so customers such as PowerToys would just see that the header is pronounced as expected when focusing the RadioButtons control. Once WinUI has a proper fix ready, the workaround would be removed in a new WinUI version and customers won't notice any difference.
I agree with the sentiment expressed here though that we should strive to get this fixed without using such a workaround as other headered controls like the ComboBox also get the job done without it.
@chingucoding Sounds good to me though as I confessed some time ago, I am a novice to all things UIA considered so I will likely take some time tomorrow to read up more on this.
Ah, i thought you wanted the PT code base to have the workaround