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

[Accessibility] Add support for landmarks #8452

Closed
AgneLukoseviciute opened this issue Aug 19, 2021 · 9 comments
Closed

[Accessibility] Add support for landmarks #8452

AgneLukoseviciute opened this issue Aug 19, 2021 · 9 comments

Comments

@AgneLukoseviciute
Copy link
Contributor

AgneLukoseviciute commented Aug 19, 2021

Landmarks is an accessibility property used to define sections of UI and allow for faster navigation/skimming by users using screen readers. For example, when in Narrator's scan mode (capslock + space) the user should be able to jump between landmarks to easily skim the content.

@AgneLukoseviciute AgneLukoseviciute self-assigned this Aug 19, 2021
@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Aug 19, 2021
@asklar asklar removed the bug label Aug 20, 2021
@chrisglein chrisglein added this to the 0.66 milestone Aug 23, 2021
@chrisglein chrisglein added Area: Accessibility and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Aug 23, 2021
@chrisglein
Copy link
Member

@AgneLukoseviciute What are the cross-platform equivalents for this?

Worth checking what the Win32 RNW implementation does here, as they may have hit this as well and we should come up with a compatible answer.

@ksiler
Copy link
Contributor

ksiler commented Aug 23, 2021

Worth checking what the Win32 RNW implementation does here, as they may have hit this as well and we should come up with a compatible answer.

These properties aren't implemented on RN Win32 yet but they do exist natively (as UIA Heading Level and UIA Landmark Type) so we could add accessibilityHeadingLevel and accessibilityLandmarkType props (or something to that effect) to handle this.

@AgneLukoseviciute
Copy link
Contributor Author

AgneLukoseviciute commented Aug 23, 2021

@AgneLukoseviciute Agne Lukoseviciute FTE What are the cross-platform equivalents for this?

Worth checking what the Win32 RNW implementation does here, as they may have hit this as well and we should come up with a compatible answer.

For headings, Android TalkBack appears to use setHeading/isHeading which is a boolean and is also the same value that is set when accessibilityRole="header" is specified. VoiceOver in iOS recognizes headings by header accessibilityTrait which is also a boolean and is set when accessibilityRole="header." With this in mind, it might make the most sense to pick a default value for UIA Heading Level and whenever accessibilityRole="header" is specified, we set the UIA Heading Level to that default on the native side. This way we can avoid adding a windows-specific prop and it should behave same as RN Core. Note: for text this would need to wait on #8191.

As far as landmarks go, from first glance it looks like VoiceOver and Talkback only support ARIA landmarks on web and not natively. There's no absolute accessibilityRole->UIA Landmark Type translation that makes sense, and as @ksiler mentioned, a new prop such as accessibilityLandmarkType would be needed to support landmarks in windows.

@ksiler
Copy link
Contributor

ksiler commented Aug 30, 2021

As far as landmarks go, from first glance it looks like VoiceOver and Talkback only support ARIA landmarks on web and not natively. There's no absolute accessibilityRole->UIA Landmark Type translation that makes sense, and as @ksiler mentioned, a new prop such as accessibilityLandmarkType would be needed to support landmarks in windows.

@AgneLukoseviciute There's an option to specify a custom landmark for UIA Landmark Type and in that case it states you should provide a string to describe your landmark (using the UIA_LocalizedLandmarkTypePropertyId). Would we need another property to handle this? Or should we use the same prop (accessibilityLandmarkType) and have it accept an enum (with the expected landmark values) or string (to set a custom landmark value)?

@AgneLukoseviciute
Copy link
Contributor Author

@ksiler I like your idea of using the same prop, as long as the diversion from windows would not be too confusing for dev's. If a string is provided rather than an AutomationLandmarkType enum, we could then set LandmarkTypeProperty to custom and wire the provided string to LocalizedLandmarkTypeProperty - otherwise just wire the provided enum to LandmarkTypeProperty.

@chrisglein chrisglein modified the milestones: 0.66, 0.67 Oct 4, 2021
@chrisglein chrisglein modified the milestones: 0.67, 0.68 Oct 25, 2021
@AgneLukoseviciute AgneLukoseviciute changed the title [Accessibility] Add support for landmarks and headings [Accessibility] Add support for landmarks ~and headings~ Jan 27, 2022
@AgneLukoseviciute AgneLukoseviciute changed the title [Accessibility] Add support for landmarks ~and headings~ [Accessibility] Add support for landmarks Jan 27, 2022
@AgneLukoseviciute
Copy link
Contributor Author

https://github.com/react-native-community/discussions-and-proposals/blob/fd64774e8161973b87b8f0a27587fa039eb7b43b/proposals/0000-accessibility-apis.md suggests this be done through the accessibilityRole prop, work here is port the solution in #8708 to use 'accessibilityRole' instead of adding a new windows-specific prop.

@AgneLukoseviciute AgneLukoseviciute modified the milestones: 0.68, Backlog Jan 27, 2022
@chrisglein
Copy link
Member

Notes from planning poker

Headings are done, but we don't support heading levels (level 1, 2, 3, etc.) - we use a default level (was enough to unblock internal partner). Will create another issue for that specifically.
Do need landmarks. Designating sections for screen readers.
AutomationProperties.LandmarkTypeProperty Property (Windows.UI.Xaml.Automation) - Windows UWP applications | Microsoft Docs
Landmark Type Identifiers (UIAutomationClient.h) - Win32 apps | Microsoft Docs.
Expect time to test with Narrator and such takes a fair amount of time, regardless of the issue. Mapping aria properties for landmarks may not be an exact match for Windows, which would lead to upstream conversations/changes.

@AgneLukoseviciute
Copy link
Contributor Author

@YajurG some comments here might be good for context regarding headings & microsoft/react-native-gallery#337

@will-ks
Copy link

will-ks commented Apr 24, 2024

Hi, was this ever solved? From the linked PRs looks like only support for headers was added but not landmarks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants