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

Fixed narrator not announcing RangeSelector values / changes #4442

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Arlodotexe
Copy link
Member

@Arlodotexe Arlodotexe commented Dec 29, 2021

Overview

Bugfix, Fixes #3538

Current behavior

Narrator does not announce range selector values or updates to the values.

New behavior

Narrator now announces range selector values and updates.

To do this, I've

  • Added a new RangeThumb control - derives from RangeBase.
    • Binds min/max/value to the range selector.
    • Wraps around a Windows.UI.Xaml.Controls.Primitives.Thumb control, using the existing style.
    • Relays the needed events from the wrapped Thumb for range selector to use.
  • Added a new RangeThumbAutomationPeer class - takes in a RangeThumb and is treated like a slider.
  • Swapped Thumb with RangeThumb - in RangeSelector, RangeThumb has been implemented and fully integrated.
  • Tested the new behavior -- to make sure Narrator now works as expected.

Short demo of the new behavior:

recordedVideo2021-12-29-140436.mp4

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • New component
    • Pull Request has been submitted to the documentation repository instructions. Link:
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
    • If control, added to Visual Studio Design project
  • Sample in sample app has been added / updated (for bug fixes / features)
  • New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

@ghost
Copy link

ghost commented Dec 29, 2021

Thanks Arlodotexe for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost requested a review from azchohfi December 29, 2021 20:17
@ghost ghost added bug 🐛 An unexpected issue that highlights incorrect behavior sample app 🖼 labels Dec 29, 2021
@michael-hawker
Copy link
Member

Rather than creating an entirely new sub-class of Thumb just for an automation peer, can we not create a RangeSelectorAutomationPeer and raise events when the thumb properties change to alert the automation clients that it's changed?

Love the result from the video, but just wondering if we can simplify this to make it more maintainable and not expose a new class that we wouldn't want folks to pick-up and use. @chingucoding any suggestions? Or am I missing something about what's possible with an automation peer? Like wondering if we can use sub-peers to get the name of the thumb changing if need be or something? https://docs.microsoft.com/en-us/windows/apps/design/accessibility/custom-automation-peers#:~:text=Forwarding%20patterns%20from%20sub%2Delements

@Arlodotexe
Copy link
Member Author

@michael-hawker That's really helpful, I wasn't aware of this option! If this works it'll be a better option, it avoids creating a new control and potentially breaking custom styles that devs have already made in their app.

I'll wait a bit for notes from @chingucoding before digging into this more.

@marcelwgn
Copy link
Contributor

Rather than creating an entirely new sub-class of Thumb just for an automation peer, can we not create a RangeSelectorAutomationPeer and raise events when the thumb properties change to alert the automation clients that it's changed?

I think this could be a fairly good approach to this. I think we wouldn't really support any patterns since the all don't seem to fit really well here and we would should use the control type custom.

Love the result from the video, but just wondering if we can simplify this to make it more maintainable and not expose a new class that we wouldn't want folks to pick-up and use. @chingucoding any suggestions? Or am I missing something about what's possible with an automation peer? Like wondering if we can use sub-peers to get the name of the thumb changing if need be or something? https://docs.microsoft.com/en-us/windows/apps/design/accessibility/custom-automation-peers#:~:text=Forwarding%20patterns%20from%20sub%2Delements

I think you are slightly misunderstanding the pattern forwarding. In our case, we would want to get the name of the thumbs. If i understand the pattern forwarding correctly, you would use for example if assistive technology asks your control if it supports the scroll pattern and ListView for example would then return the internal scroll viewer. Getting the names of the thumbs should be fairly easy by just getting their automation peer and getting their AutomationProperties.Name property. Though I'm inclined to say that the "Minimum" or "Maximum" value changed instead of "Minimum value thumb" or what ever the UIA name of the thumb could be.

@michael-hawker michael-hawker modified the milestones: 7.1.3, 8.0 Jul 14, 2022
@michael-hawker
Copy link
Member

@Arlodotexe @niels9001 did this carry forward to the new control or did it get fixed with the other improvements made?

If fixed, let's close this PR and the corresponding bug (pointing to the new repo/release), otherwise, we can transfer the bug forward to the new repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility ♿ bug 🐛 An unexpected issue that highlights incorrect behavior sample app 🖼
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

RangeSelector does not announce value changes when changing thumb position with arrow keys
4 participants