Skip to content

Conversation

@lyahdav
Copy link
Contributor

@lyahdav lyahdav commented Jul 14, 2021

Per #7290 these changes are required. I confirmed that the Playground-win32 announceForAccessibility example now works with this change.

Microsoft Reviewers: Open in CodeFlow

@lyahdav lyahdav requested a review from a team as a code owner July 14, 2021 18:52
Comment on lines +116 to +118
rootElement.SetValue(
winrt::Windows::UI::Xaml::Automation::AutomationProperties::LandmarkTypeProperty(),
winrt::box_value(80002));
Copy link
Member

Choose a reason for hiding this comment

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

this should not be needed if you do the first API call. This is a fallback in cases where you don't have an accessible root. Narrator may not behave "as well" when setting landmark type this way, so prefer using just the xaml root.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I verified this is needed (or maybe I'm missing something about your comment?). If I delete this then Narrator doesn't announce anything when trying the AccessibilityInfo example in playground-win32 RNTester. This also aligns with what's mentioned in #7290:

AccessibleXamlRoots must be able to create an automation peer or need to set AutomationProperties::LandmarkTypeProperty() to 80002.

Copy link
Member

Choose a reason for hiding this comment

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

@lyahdav what is the concrete type of the thing that you pass to SetAccessibleXamlRoot ? it needs to be one of the FrameworkElement derived types that is associated with the list of derived types in this page:

https://docs.microsoft.com/en-us/uwp/api/Windows.UI.Xaml.Automation.Peers.FrameworkElementAutomationPeer?view=winrt-20348

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per https://github.com/microsoft/react-native-windows/blob/main/packages/playground/windows/playground-win32/Playground-Win32.cpp#L366 this is a Grid. From what you linked there is no GridAutomationPeer. At the point we SetAccessibleRoot we only have that control available. So what makes sense, to set LandmarkTypeProperty or to create some other dummy control? LandmarkTypeProperty seems cleaner.

@lyahdav lyahdav force-pushed the fix-accessibility-playground-win32 branch from 4c20e72 to ab92aaa Compare July 15, 2021 19:03
@chrisglein chrisglein requested a review from chiaramooney July 21, 2021 17:24
Copy link
Contributor

@chiaramooney chiaramooney left a comment

Choose a reason for hiding this comment

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

👍

@chiaramooney chiaramooney merged commit e0e673c into microsoft:main Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants