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

NumberBox AutomationProperties.Name and Min/Max is Ignored by AutomationPeer #2951

Closed
robloo opened this issue Jul 19, 2020 · 12 comments · Fixed by #6145
Closed

NumberBox AutomationProperties.Name and Min/Max is Ignored by AutomationPeer #2951

robloo opened this issue Jul 19, 2020 · 12 comments · Fixed by #6145
Labels
A11yMediumImpact A11ySev2 Non-blocking for core user tasks or blocking for non-core user tasks. A11yWCAG accessibility Narrator, keyboarding, UIA, etc area-NumberBox NumberBox Control help wanted Issue ideal for external contributors team-Controls Issue for the Controls team

Comments

@robloo
Copy link
Contributor

robloo commented Jul 19, 2020

Describe the bug

Setting a NumberBox's AutomationProperties.Name attached property is ignored by its AutomationPeer. The Minimum/Maximum values are also not used but should be.

Steps to reproduce the bug

  1. Set the AutomationProperties.Name attached property of a NumberBox in XAML
  2. Using the Windows Narrator, focus the NumberBox (and therefore the InputTextBox inside it)
  3. The NumberBox AutomationPeer and Narrator will not use the AutomationProperties.Name when the InputTextBox is focused
  4. The NumberBox AutomationPeer and Narrator will not use any Minimum/Maximum values

Expected behavior

This should behave the same as the Slider. The AutomationProperties.Name should be used first then a description of the value. Additionally, the AutomationPeer should really give the min/max values of the range as well. The Slider just gives a percent but the NumberBox needs to be more specific.

Screenshots
N/A

Version Info

NuGet package version:
Microsoft.UI.Xaml 2.4.0

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Jul 19, 2020
@StephenLPeters StephenLPeters added accessibility Narrator, keyboarding, UIA, etc area-NumberBox NumberBox Control team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jul 20, 2020
@StephenLPeters
Copy link
Contributor

@robloo we had this issue #1854 which says it was fixed. Seems related but not exactly the same?

@StephenLPeters StephenLPeters added the help wanted Issue ideal for external contributors label Jul 20, 2020
@robloo
Copy link
Contributor Author

robloo commented Jul 21, 2020

@StephenLPeters Yea, thought it sounded familiar. I should have looked in more detail. That said, I'm expanding it to also add the Minimum/Maximum values as well as that's pretty important for accessibility.

I ran across this over the weekend with WinUI 2.4.0. I'll update to 2.4.2 and test again for the Name property.

@StephenLPeters
Copy link
Contributor

I looked at the change and I do think it should fix the name issue. @YuliKl is there someone from the accessibility team we can ask about what to do about the min and max values?

@ranjeshj
Copy link
Contributor

The API looks similar to RangeBase - should the peer support IRangeValueProvider ?

@robloo
Copy link
Contributor Author

robloo commented Jul 23, 2020

@StephenLPeters I tested this again with latest 2.4.2 of WinUI -- it still has an issue and doesn't read aloud the AutomationProperties.Name.

"Edit zero, zero selected" is all I get with the below code.

<mux:NumberBox
  x:Name="Channel1NumberBox"
  AutomationProperties.Name="Red Channel"
  Grid.Column="1"
  Grid.Row="1"
  CornerRadius="0,2,2,0"
  VerticalAlignment="Center" />

I also notice that IRangeValueProvider is implemented already

I think the issue might be related to focus. It's behaving as if only the InputBox is focused -- and it is but only as a part of the NumberBox. NumberBoxAutomationPeer is not used and instead narrator is just using the default AutomationPeer for TextBox (which doesn't have an AutomationProperties.Name set as that is part of NumberBox).

@YuliKl
Copy link

YuliKl commented Jul 23, 2020

@SavoySchuler FYI. Perhaps you can reach out to someone on the Accessibility team for help.
I'm also wondering whether #1882 is related.

@MikeHillberg
Copy link
Contributor

I think the issue might be related to focus. It's behaving as if only the InputBox is focused -- and it is but only as a part of the NumberBox. NumberBoxAutomationPeer is not used and instead narrator is just using the default AutomationPeer for TextBox (which doesn't have an AutomationProperties.Name set as that is part of NumberBox).

I think that's exactly right. You'd expect to have the same issue with AutoSuggestBox, but in its OnApplyTemplate is forwards the Automation name to the TextBox part. It looks like editable ComboBox needs the same treatment.

More generally maybe we should teach TextBoxAutomationPeer to work with its templated parent when it's acting as a template part. In the example of NumberBox, it needs to be a range provider for the sake of min/max.

@marcelwgn
Copy link
Collaborator

@MikeHillberg Is there anything we can do in WinUI 2 to prevent the TextBoxAutomationPeer to interfere with us here and allow us to pass Min/Max to UIA?

@ranjeshj Would it be fine if I create a PR to pass the AutomationProperties.Name to the TextBox so we can fill that gap? Also, when AutomationProperties.Name is not set, but the header of the NumberBox is set, should we pass the header value as AutomationProperties.Name value? After all, that is one of the ways the TextBox gets its UIA name.

@ranjeshj
Copy link
Contributor

@chingucoding lets give the same treatment for automation name in the mean time (and open a separate issue to track TextBoxAutomationPeer to understand the templated parent scenario).

@marcelwgn
Copy link
Collaborator

@ranjesh Just to be sure, so should we apply to the same header handling logic to numberbox or should we just pass the AutomationProperties.Name value to the textbox?

@ranjeshj
Copy link
Contributor

ranjeshj commented Oct 27, 2020

Do we have the header handling logic in WinUI already ? If so, lets match that pattern. I was thinking we can lookup the automationproperties.name and use if if it is valid, if not fallback to the header.

@SavoySchuler @YuliKl does that sound reasonable ?

@marcelwgn
Copy link
Collaborator

I think we have a helper for this that get's us the appropriate UIA name depending on the properties set. Though this would require us to listen to changes on the AutomationProperties.Name property so we can update the textboxes UIA name accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A11yMediumImpact A11ySev2 Non-blocking for core user tasks or blocking for non-core user tasks. A11yWCAG accessibility Narrator, keyboarding, UIA, etc area-NumberBox NumberBox Control help wanted Issue ideal for external contributors team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants