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

Announce accessibility state changes happening in the background #26624

Closed
wants to merge 2 commits into from

Conversation

xuelgong
Copy link
Contributor

@xuelgong xuelgong commented Sep 27, 2019

Summary

Currently the react native framework doesn't handle the accessibility state changes of the focused item that happen not upon double tapping. Screen reader doesn't get notified when the state of the focused item changes in the background.
To fix this problem, post a layout change notification for every state changes on iOS.
On Android, send a click event whenever state "checked", "selected" or "disabled" changes. In the case that such states changes upon user's clicking, the duplicated click event will be skipped by Talkback.

Changelog

[General][Fixed] - Announce accessibility state changes happening in the background

Test Plan

Add a nested checkbox example which state changes after a delay in the AccessibilityExample.

[Problem]
Currently the react native framework doesn't handle the state
changes of the focused item that happen not upon double tapping. Screen
reader doesn't get notified when the state of the focused item changes
in the background.

[Solution]
On iOS, post a layout change notification for every state changes.
On Android, send a click event whenever state "checked", "selected" or
"disabled" changes. In the case that such states changes upon user's
clicking, the duplicated click event will be skipped by Talkback.

[Testing]
Add a nested checkbox example which state changes after a delay in the
AccessibilityExample.
@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 Sep 27, 2019
@blavalla
Copy link
Contributor

Overall the implementation looks good, but I added a few comments inline that I think will help clarify exactly what is being added here.

@@ -314,6 +298,114 @@ class ExpandableElementExample extends React.Component {
}
}

class NestedCheckBox extends React.Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

While this example is functioning how you intended it to, I found it pretty confusing at first why it was checking/unchecking multiple boxes when I selected one. I don't see how adding these additional checkboxes helps to showcase this feature either, as it could easily be shown with a single checkbox that gets checked on a delay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This example mimics ordering a pizza. When "meat" is chosen, all meats are checked. If only "beef" or "bacon" is selected, the state of the checkbox "meat" becomes "mixed". The reason I created nested checkboxes is to make sure that the state change of the unfocused item won't be announced. That is, if all meats are checked previously and then deselect beef, only the state change of "beef" checkbox will be announced. Even though the "meat" checkbox state also changes, screen reader shouldn't announce it.

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Thank you so much for the PR and thanks @blavalla for reviewing this PR.

Copy link
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.

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @xuelgong in baa66f6.

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 Oct 14, 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
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Amazon Partner: Amazon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants