-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
add talkback navigation support for links and header #22447
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@yangweigbh can you rebase this onto master? There was a lot of work performed on automated tests recently and the extra coverage for this might be useful. |
@bartolkaruza done, but the test_android workflow seems have bug with the following error
|
Hi there! excited to see a fix for this issue 💃. Happy to help out if necessary, but the changes don't seem to be related to the CI failure. Might this just need a re-run of the build? |
@FLGMwt hi, how to rerun the CI test, i seem to not have the permission to rerun the test |
Sorry @yangweigbh, I'm not sure, just another outside consumer of RN. At the very least, I bet a new rebase or empty commit would trigger it. |
@yangweigbh Looks like it still failed w/ the retry. Would you mind terribly rebasing this onto latest master again? |
955cce6
to
0b71d17
Compare
@FLGMwt @bartolkaruza thank god, i have finally pass all the CI test, can we merge this PR as this PR has been hang on too long ! |
@yangweigbh thanks for taking the time, I'll ask if someone can review this from the core team side over the weekend. Hang tight, you're almost there 👍 |
@yangweigbh could you add this
to https://github.com/yangweigbh/react-native/blob/0b71d176fd13a6cc917d38fbbe4cad28db390809/RNTester/js/AccessibilityAndroidExample.android.js#L112 so that the example in RNTester is still complete after this PR? I've tested your change on my end and it works like a charm. I'll ask someone to merge after you add that example. |
Missing password
|
1. add role description for heading 2. add talkback navigation support for link and header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the example. I'm not an expert here but I trust @bartolkaruza.
There was a problem hiding this 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.
This pull request was successfully merged by Wei Yang in b9d3743. When will my fix make it into a release? | Upcoming Releases |
Summary
Fixes #22440
Test Plan:
Changelog:
[Android] [Changed] - Add talkback navigation support for links and header