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

fix(fabric, a11y): implement accessibility role #2293

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Saadnajmi
Copy link
Collaborator

This is part of a series of PRs where we are cherry-picking fixes from #2117 to update our Fabric implementation on macOS.

TODO

Summary:

Pick a few changes related to implementing accessibility role

Test Plan:

CI should pass.

@Saadnajmi Saadnajmi requested a review from a team as a code owner November 21, 2024 22:29
@Saadnajmi Saadnajmi marked this pull request as draft November 21, 2024 22:29
Nick Lefever added 3 commits November 21, 2024 14:40
Summary:
Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text.

This change will allow to select UI elements based on the text contents.

Test Plan:
Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText:
 {F1162808272}

Reviewers: shawndempsey, chpurrer, #rn-desktop

Reviewed By: chpurrer

Differential Revision: https://phabricator.intern.facebook.com/D51736932

Tasks: T170938725

Tags: uikit-diff
Summary:
Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text.

This change will allow to select UI elements based on the text contents.

Test Plan:
Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText:
 {F1162808272}

Reviewers: shawndempsey, chpurrer, #rn-desktop

Reviewed By: chpurrer

Differential Revision: https://phabricator.intern.facebook.com/D51736931

Tasks: T170938725
Summary:
Paper was rendering text as AXStaticText. This diff updates the `RCTParagraphComponentView` to propagate the same role in Fabric for text.

This change will allow to select UI elements based on the text contents.

Test Plan:
Use the Accessibility Inspector in Zeratul with Fabric enabled. With the changes the text elements are presented in the a11y hierarchy with AXStaticText:
 {F1162808272}

Reviewers: shawndempsey, chpurrer, #rn-desktop

Reviewed By: chpurrer

Differential Revision: https://phabricator.intern.facebook.com/D51736933

Tasks: T170938725
@property (nonatomic, strong, nullable, readonly) NSObject *accessibilityElement;
@property (nonatomic, strong, nullable, readonly) RCTPlatformView *accessibilityElement; // [macOS]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should probably ifdef this for iOS vs macOS

@@ -17,6 +17,7 @@
#import <React/RCTConversions.h>
#import <React/RCTCursor.h> // [macOS]
#import <React/RCTLocalizedString.h>
#import <React/UIView+React.h> // [macOS]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we need this import?

Comment on lines +128 to +130
inline NSAccessibilityRole RCTUIAccessibilityRoleFromAccessibilityTraits(
facebook::react::AccessibilityTraits accessibilityTraits)
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Double check this list with paper

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.

1 participant