-
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 getRecommendedTimeoutMillis to AccessibilityInfo #31063
add getRecommendedTimeoutMillis to AccessibilityInfo #31063
Conversation
Base commit: ac7ba3e |
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.
Code analysis results:
google-java-format
found some issues. See https://github.com/google/google-java-format
...roid/src/main/java/com/facebook/react/modules/accessibilityinfo/AccessibilityInfoModule.java
Show resolved
Hide resolved
break; | ||
default: | ||
break; | ||
} |
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.
google-java-format
suggested changes:
@@ -215 +215,2 @@
- mRecommendedTimeout = mAccessibilityManager.getRecommendedTimeoutMillis((int)originalTimeout, flag);
+ mRecommendedTimeout =
+ mAccessibilityManager.getRecommendedTimeoutMillis((int) originalTimeout, flag);
Base commit: ac7ba3e |
Hey, @grgr-dkrk thank you for this PR to help make React Native more accessible! I've added you to our project board. Apologies for this not getting added earlier. |
*/ | ||
getRecommendedTimeoutMillis: function( | ||
originalTimeout: number, | ||
uiContentFlags: AccessibilityUiContentFlagsTypes, |
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'm not sure we should bother to surface this param in the javascript API.
Under the hood it's actually not doing anything, no matter which flag you pass in, the same value comes back. Looking through the Android source, it seems like they originally intended to have two user settings for timings, one for interactive elements, and one for static elements, but at some point this was dropped in favor of a single setting that just sets both values behind the scenes. These flags seem like a vestigial remnant of that original idea, and will likely be removed in a later API version.
@kacieb, does RN generally have a standard for when we should match a platform API strictly versus going with a simpler API if both options have the same end result?
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.
Commenting again to request changes.
@nadiia has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
*/ | ||
getRecommendedTimeoutMillis: function( | ||
originalTimeout: number, | ||
uiContentFlags: AccessibilityUiContentFlagsTypes, |
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.
Commenting again to request changes.
Sorry for the comment churn here, @nadiia is implementing the changes I requested, so no need to any more work @grgr-dkrk! Thanks again for the PR. |
@grgr-dkrk Thank you for the contribution! Would you be open to add a PR for documenting |
Thank you @grgr-dkrk for this contribution! I would like to give you a shout-out on Twitter and include you in our end-of-month issues update for your contribution. Do you have a Twitter we can tag? |
@blavalla @amarlette @nadiia Thank you for your support! I have submitted a PR of documentation on this feature. My Twitter account is [@]dkrk0901, Thanks. |
Summary
resolve #30866
This PR is for using
getRecommendedTimeoutMillis
with React Native, which is available on Android 10 and above.This allows the Android "Time to take action (Accessibility timeout)" setting to be reflected in the app.
Changelog
[Android] [Added] - Add
getRecommendedTimeoutMillis
to AccessibilityInfoTest Plan
I couldn't find any tests at the code level, so I tested them on my Android device.
Android 10 (Pixel4a)
Settings
App
Android 7, iOS(Simulator)
Return the original timeout.