-
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
feat: add accessibilityLabelledBy
props
#32470
feat: add accessibilityLabelledBy
props
#32470
Conversation
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:
eslint
found some issues. Runyarn lint --fix
to automatically fix problems.google-java-format
found some issues. See https://github.com/google/google-java-format
@@ -432,6 +432,13 @@ export type ViewProps = $ReadOnly<{| | |||
*/ | |||
accessibilityActions?: ?$ReadOnlyArray<AccessibilityActionInfo>, | |||
|
|||
/** | |||
* Specifies the nativeID of the associated label text. When the assistive technology focuses on the component with this props, the text is read aloud. | |||
* |
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.
no-trailing-spaces: Trailing spaces not allowed.
@@ -432,6 +432,13 @@ export type ViewProps = $ReadOnly<{| | |||
*/ | |||
accessibilityActions?: ?$ReadOnlyArray<AccessibilityActionInfo>, | |||
|
|||
/** | |||
* Specifies the nativeID of the associated label text. When the assistive technology focuses on the component with this props, the text is read aloud. | |||
* |
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.
prettier/prettier: Delete ·
@@ -36,6 +36,7 @@ | |||
import com.facebook.react.uimanager.events.Event; | |||
import com.facebook.react.uimanager.events.EventDispatcher; | |||
import java.util.HashMap; | |||
import com.facebook.react.uimanager.util.ReactFindViewUtil; |
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:
@@ -39,0 +39 @@
+import java.util.HashMap;
@@ -200,6 +201,24 @@ public void onInitializeAccessibilityNodeInfo(View host, AccessibilityNodeInfoCo | |||
setRole(info, accessibilityRole, host.getContext()); | |||
} | |||
|
|||
final Object accessibilityLabelledBy = host.getTag(R.id.labelled_by); | |||
if (accessibilityLabelledBy != null) { |
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:
@@ -206,12 +206,11 @@
- ReactFindViewUtil.findView(
- host.getRootView(),
- new ReactFindViewUtil.OnViewFoundListener() {
- @Override
- public String getNativeId() {
- return (String) accessibilityLabelledBy;
- }
-
- @Override
- public void onViewFound(View view) {
- info.setLabeledBy(view);
- }
+ ReactFindViewUtil.findView(
+ host.getRootView(),
+ new ReactFindViewUtil.OnViewFoundListener() {
+ @Override
+ public String getNativeId() {
+ return (String) accessibilityLabelledBy;
+ }
+
+ @Override
+ public void onViewFound(View view) {
+ info.setLabeledBy(view);
public void onViewFound(View view) { | ||
info.setLabeledBy(view); | ||
} | ||
} |
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:
@@ -219 +218 @@
- );
+ });
PR build artifact for 4b82094 is ready. |
PR build artifact for 8f710ad is ready. |
Base commit: f58c496 |
PR build artifact for ea3e774 is ready. |
Base commit: f58c496 |
@ShikaSD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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.
Also see the Accessibility APIs proposal: react-native-community/discussions-and-proposals#410
* | ||
* @platform android | ||
*/ | ||
accessibilityLabelledBy?: ?string, |
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.
Technically this should also support an array of ids so multiple elements can label this one
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 couldn't come up with a concrete plan (like when it received multiple IDs) to reproduce the consistent behavior of aria-labelledby
on Android, but I changed props to be able to specify an array. Thank you very much.
@grgr-dkrk Do you have any plans for implementing this on iOS? Additionally, could we add documentation for this in |
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:
eslint
found some issues. Runyarn lint --fix
to automatically fix problems.
<Text nativeID="formLabel2">First Name</Text> | ||
<TextInput | ||
accessibilityLabel="input test2" | ||
accessibilityLabelledBy={["formLabel2", "formLabel3"]} |
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.
prettier/prettier: Replace "formLabel2",·"formLabel3"
with 'formLabel2',·'formLabel3'
<Text nativeID="formLabel2">First Name</Text> | ||
<TextInput | ||
accessibilityLabel="input test2" | ||
accessibilityLabelledBy={["formLabel2", "formLabel3"]} |
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.
quotes: Strings must use singlequote.
<Text nativeID="formLabel2">First Name</Text> | ||
<TextInput | ||
accessibilityLabel="input test2" | ||
accessibilityLabelledBy={["formLabel2", "formLabel3"]} |
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.
quotes: Strings must use singlequote.
PR build artifact for 1282770 is ready. |
I didn't plan to implement it on iOS because there is no API equivalent to this props (like
If there is no problem, I will work on it. Thank you very much. |
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was successfully merged by @grgr-dkrk in 36037fa. When will my fix make it into a release? | Upcoming Releases |
…mic (#34371) Summary: `accessibilityLabelledBy` accepts String or Array type. - The JavaScript Array type corresponds to java [ReadableArray][3] ([HybridData][4]) - The JavaScript String type corresponds to the java String Use [DynamicFromObject][5] to parse String to Dynamic. https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java#L222-L228 All credits to [grgr-dkrk][1] (PR #32470). fixes [https://github.com/facebook/react-native/issues/30846][2] ## Changelog [Android] [Fixed] - accessibilityLabelledBy use DynamicFromObject to parse String to Dynamic Pull Request resolved: #34371 Test Plan: <details><summary>testing accessibilityLabelledBy with TextInput</summary> <p> https://user-images.githubusercontent.com/24992535/183593138-7ced1974-6a06-4f0f-822a-1ade1edc7212.mp4 </p> </details> <details><summary>testing accessibilityLabelledBy with Switch</summary> <p> ![Screen Shot 2022-08-09 at 15 53 37](https://user-images.githubusercontent.com/24992535/183596336-4b73186b-6d27-485e-a6ea-29a0f0b9ef50.png) </p> </details> <details><summary>testing paper renderer after the fix</summary> <p> https://user-images.githubusercontent.com/24992535/183605619-01f1be64-788a-43bd-88b0-a7b2cad75148.mp4 </p> </details> [1]: https://github.com/grgr-dkrk [2]: #30846 [3]: https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java#L1 [4]: https://github.com/facebookincubator/fbjni/blob/main/java/com/facebook/jni/HybridData.java [5]: https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicFromObject.java#L74 Reviewed By: lunaleaps Differential Revision: D38706360 Pulled By: huntie fbshipit-source-id: e4771552d3fddfad50f4d4cbbf971fe4a718e134
…mic (facebook#34371) Summary: `accessibilityLabelledBy` accepts String or Array type. - The JavaScript Array type corresponds to java [ReadableArray][3] ([HybridData][4]) - The JavaScript String type corresponds to the java String Use [DynamicFromObject][5] to parse String to Dynamic. https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java#L222-L228 All credits to [grgr-dkrk][1] (PR facebook#32470). fixes [https://github.com/facebook/react-native/issues/30846][2] ## Changelog [Android] [Fixed] - accessibilityLabelledBy use DynamicFromObject to parse String to Dynamic Pull Request resolved: facebook#34371 Test Plan: <details><summary>testing accessibilityLabelledBy with TextInput</summary> <p> https://user-images.githubusercontent.com/24992535/183593138-7ced1974-6a06-4f0f-822a-1ade1edc7212.mp4 </p> </details> <details><summary>testing accessibilityLabelledBy with Switch</summary> <p> ![Screen Shot 2022-08-09 at 15 53 37](https://user-images.githubusercontent.com/24992535/183596336-4b73186b-6d27-485e-a6ea-29a0f0b9ef50.png) </p> </details> <details><summary>testing paper renderer after the fix</summary> <p> https://user-images.githubusercontent.com/24992535/183605619-01f1be64-788a-43bd-88b0-a7b2cad75148.mp4 </p> </details> [1]: https://github.com/grgr-dkrk [2]: facebook#30846 [3]: https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java#L1 [4]: https://github.com/facebookincubator/fbjni/blob/main/java/com/facebook/jni/HybridData.java [5]: https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicFromObject.java#L74 Reviewed By: lunaleaps Differential Revision: D38706360 Pulled By: huntie fbshipit-source-id: e4771552d3fddfad50f4d4cbbf971fe4a718e134
…mic (facebook#34371) Summary: `accessibilityLabelledBy` accepts String or Array type. - The JavaScript Array type corresponds to java [ReadableArray][3] ([HybridData][4]) - The JavaScript String type corresponds to the java String Use [DynamicFromObject][5] to parse String to Dynamic. https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java#L222-L228 All credits to [grgr-dkrk][1] (PR facebook#32470). fixes [https://github.com/facebook/react-native/issues/30846][2] ## Changelog [Android] [Fixed] - accessibilityLabelledBy use DynamicFromObject to parse String to Dynamic Pull Request resolved: facebook#34371 Test Plan: <details><summary>testing accessibilityLabelledBy with TextInput</summary> <p> https://user-images.githubusercontent.com/24992535/183593138-7ced1974-6a06-4f0f-822a-1ade1edc7212.mp4 </p> </details> <details><summary>testing accessibilityLabelledBy with Switch</summary> <p> ![Screen Shot 2022-08-09 at 15 53 37](https://user-images.githubusercontent.com/24992535/183596336-4b73186b-6d27-485e-a6ea-29a0f0b9ef50.png) </p> </details> <details><summary>testing paper renderer after the fix</summary> <p> https://user-images.githubusercontent.com/24992535/183605619-01f1be64-788a-43bd-88b0-a7b2cad75148.mp4 </p> </details> [1]: https://github.com/grgr-dkrk [2]: facebook#30846 [3]: https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java#L1 [4]: https://github.com/facebookincubator/fbjni/blob/main/java/com/facebook/jni/HybridData.java [5]: https://github.com/facebook/react-native/blob/e509f96baf5e523301a5c9567c416508ff20d175/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicFromObject.java#L74 Reviewed By: lunaleaps Differential Revision: D38706360 Pulled By: huntie fbshipit-source-id: e4771552d3fddfad50f4d4cbbf971fe4a718e134
Summary
related: #30846, #26739
Added
accessibilityLabelledBy
props to find the nativeID of the associated label, it mainly for<TextInput>
.The reason for implementing it as
labelledBy
instead oflabelFor
is as follows.labelFor
because the<Text>
component does not add thelabelFor
received from her Props to the View's tag.aria-labelledby
, which is intuitive for web developers. It also seems easy to convert to a web platform.Changelog
[Android] [Added] - add
accessibilityLabelledBy
propsTest Plan
I checked it with RNTester using an Android11.
inputtest.mp4