Skip to content

Added button with accessibility action example and support for Touchables#25582

Closed
KevinGVargas wants to merge 2 commits into
facebook:masterfrom
KevinGVargas:accessibilityActionTouchables
Closed

Added button with accessibility action example and support for Touchables#25582
KevinGVargas wants to merge 2 commits into
facebook:masterfrom
KevinGVargas:accessibilityActionTouchables

Conversation

@KevinGVargas
Copy link
Copy Markdown
Contributor

Summary

We added a test to make sure button and accessibility actions would not have unwanted behavior. Additionally we added support for accessibility actions for all touchables. However we discovered that RCTTextView and possibly anything else that does not derive from RCTView does not support accessibility actions and need to be children off a component that does support it for it to not crash in ios. This became noticeable when TouchableWithoutFeedback only worked if text is a child of view on ios.

In a local branch we where able to modify RCTTextView to support accessibility actions and text no longer needed to be a child of view for it to work.

Changelog

[General] [Added] - Button test with accessibility actions
[General] [Added] - Support for accessibility actions to all Touchables. With TouchableWithoutFeedback being a special case where text must be a child of view. (See AccessibilityExample.js for an example)

Test Plan

Test plan is testing in RNTester making sure the examples work

Open Question

What would you say is the best practice for adding accessibility action support for all the components that do not extended from RCTView?

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Amazon Partner: Amazon labels Jul 10, 2019
@react-native-bot react-native-bot added Component: Button Type: Enhancement A new feature or enhancement of an existing feature. labels Jul 10, 2019
Copy link
Copy Markdown

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

</View>
</RNTesterBlock>

<RNTesterBlock title="Button with custom accessibility actions">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

prettier/prettier: Delete ·

</View>
</RNTesterBlock>

<RNTesterBlock title="Button with custom accessibility actions">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

}
}}
onPress={() => Alert.alert('Button has been pressed!')}
accessibilityRole="button">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

prettier/prettier: Delete ·

<View >
<Text>Click me</Text>
</View>
</TouchableWithoutFeedback>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

prettier/prettier: Delete

@osdnk
Copy link
Copy Markdown
Contributor

osdnk commented Jul 10, 2019

Thanks!

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@osdnk has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @AmazonKevin in b8ccb26.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 11, 2019
AKB48 pushed a commit to UnPourTous/react-native that referenced this pull request Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: Button Merged This PR has been merged. p: Amazon Partner: Amazon Type: Enhancement A new feature or enhancement of an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants