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

Override reactAccessibilityElement for RCTTextView #1992

Merged
merged 5 commits into from
Dec 1, 2023

Conversation

FalseLobster
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary:

RCTTextView does not override reactAccessibilityElement to the RCTUnfocusableTextView child. This causes various properties, like accessible, to not get applied correctly.

In VoiceOver, there's a bug where if you have a Touchable like so:

<TouchableOpacity
        accessibilityRole="button"
        accessibilityLabel="Button with accessibility role as button and label"
        onPress={onPress}>
        <Text>Button with A11y Label Defined</Text>
</TouchableOpacity>

It reads "button, group". By making the accessible prop apply correctly, the user can set the <Text accessible={false} /> and VoiceOver will just read "button".

Before:
Screenshot 2023-11-29 at 2 40 14 PM
After:
Screenshot 2023-11-29 at 2 39 07 PM

Changelog:

[INTERNAL] [FIXED] - Make accessibility props apply correctly to components on macOS.

Test Plan:

See Summary

@FalseLobster FalseLobster requested a review from a team as a code owner November 29, 2023 22:59
@FalseLobster FalseLobster merged commit 40c325b into microsoft:0.72-stable Dec 1, 2023
15 of 16 checks passed
@FalseLobster FalseLobster mentioned this pull request Jan 2, 2024
4 tasks
FalseLobster added a commit that referenced this pull request Jan 31, 2024
#### Please select one of the following
- [ ] I am removing an existing difference between facebook/react-native
and microsoft/react-native-macos 👍
- [ ] I am cherry-picking a change from Facebook's react-native into
microsoft/react-native-macos 👍
- [x] I am making a fix / change for the macOS implementation of
react-native
- [ ] I am making a change required for Microsoft usage of react-native

## Summary:

0.73-stable version of PR #1992

## Test Plan:

Verified it works as expected in 0.73.  See other PR for details
FalseLobster added a commit that referenced this pull request Jan 31, 2024
#### Please select one of the following
- [ ] I am removing an existing difference between facebook/react-native
and microsoft/react-native-macos 👍
- [ ] I am cherry-picking a change from Facebook's react-native into
microsoft/react-native-macos 👍
- [x] I am making a fix / change for the macOS implementation of
react-native
- [ ] I am making a change required for Microsoft usage of react-native

## Summary:

main version of PR #1992

## Test Plan:

Verified it works as expected in main.  See other PR for details
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.

3 participants