Add Functionality to announceForAccessibility to Support Xaml Islands #7290
Conversation
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
ghost
added
Needs: Author Feedback
| static void SetAccessibleRoot(IReactPropertyBag properties, XAML_NAMESPACE.XamlRoot xamlRoot); | ||
| DOC_STRING("Retrieve XamlRoot for app.") | ||
| static XAML_NAMESPACE.XamlRoot GetXamlRoot(IReactPropertyBag properties); | ||
| DOC_STRING("Retrieve accessible XamlRoot for app.") | ||
| static XAML_NAMESPACE.XamlRoot GetAccessibleRoot(IReactPropertyBag properties); |
There was a problem hiding this comment.
can you make the type of the root FrameworkElement? it is not necessary for the app to have an accessible xaml root, just a framework element that we can announce on
| if (!textBlock) { | ||
| return; | ||
| if (react::uwp::IsXamlIsland()) { | ||
| if (auto xamlroot = |
There was a problem hiding this comment.
once you change the type of Get/SetAccessibleRoot to FrameworkElement, name this accordingly (accessibleRoot)
| if (!element) { | ||
| if (auto window = xaml::Window::Current()) { | ||
| if (element = window.Content()) { | ||
| element.SetValue(xaml::Automation::AutomationProperties::LandmarkTypeProperty(), winrt::box_value(80002)); |
There was a problem hiding this comment.
this is not needed. An app can do this on the frameworkElement they give us if they so choose, but we shouldn't do it for them
There was a problem hiding this comment.
Also this code would never execute as it is today, if you're in an island then Window::Current would end up being null in winui 3. It works today against system xaml but that's a bug/behavior that is being fixed in winui3
| } | ||
|
|
||
| ReactPropertyId<xaml::XamlRoot> AccessibleRootProperty() noexcept { | ||
| static ReactPropertyId<xaml::XamlRoot> propId{L"ReactNative.UIManager", L"AccessibleXamlRoot"}; |
There was a problem hiding this comment.
name it AccessibleRoot, or AccessibleRootElement or something liket that
asklar
left a comment
asklar
left a comment
There was a problem hiding this comment.
please change the type of accessible root to be frameworkelement as xamlroots are not required here
ghost
added
the
Needs: Author Feedback
ghost
removed
the
Needs: Author Feedback
| DOC_STRING("Set XamlRoot for app. This must be manually provided to the ReactInstanceSettings when using XamlIslands.") | ||
| static void SetXamlRoot(IReactPropertyBag properties, XAML_NAMESPACE.XamlRoot xamlRoot); | ||
| // This needs to be manually provided to the ReactInstanceSettings when using XamlIslands | ||
| DOC_STRING("Set accessible XamlRoot for app. This XamlRoot should be able to create an automation peer. This must be manually provided to the ReactInstanceSettings when using XamlIslands to have access to accessibility methods.") |
There was a problem hiding this comment.
please update the doc string too (here and below)
Summary: Cherry picks: microsoft#5849 microsoft#7220 microsoft#7290 Test Plan: See D29689294 Test Plan Reviewers: ericroz Reviewed By: ericroz Subscribers: eliwhite Differential Revision: https://phabricator.intern.facebook.com/D30940769 Tasks: T85606613 Signature: 30940769:1631724353:41db1d978132f06502803cfb4df0323f6da84732
3 participants
#7220 added functionality to announceForAccessibility which worked for UWP apps. New changes allow for method to work for apps using XAML Islands. Note: apps using XAML Islands must now set/specify an AccessibleXamlRoot in order for announceForAccessibility to work. AccessibleXamlRoots must be able to create an automation peer or need to set AutomationProperties::LandmarkTypeProperty() to 80002.
Resolves #7245
Microsoft Reviewers: Open in CodeFlow