-
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
1/2 TextInput accessibilityErrorMessage (Talkback, Android) #33468
Changes from 125 commits
ad9dc3e
431a9d5
a4ef7a7
60b6c9b
fb30702
f22e1f3
caab17c
06c4908
d1b6182
3092ac8
e521859
f92f843
d94f343
6bd7240
202292a
2820a95
b8846c9
bf75148
5e9b4c3
97c2dff
31bfa40
394f794
3a8347a
f39e5f1
60bc4e3
3c48da3
aca0d99
37f0004
0708d07
9df9484
17852df
add6198
3d5100c
53fe1c0
6c0f432
0ee77ed
84edc00
299eba6
27dd208
2e660d2
e1c8f54
d4a390b
f29a21c
79c5e4f
cc602e0
4ba33c3
6f69758
4df9d95
99b4a24
1365f50
3a1ef4a
994089c
eb33c93
bb2df1f
de50984
7252c15
586af1f
d1b6f67
56087d3
3d831a9
0513007
425056d
ea9bb4c
7f7b281
ef89ba2
af8a0bd
29b99ba
f70870e
bb4597e
d96a284
9a58e2e
9958dcd
ad56244
bb7b0c3
195fb0b
343eea1
521c907
efba00c
dac4b96
1d00ea8
6bac85e
54aca18
8e06f08
ee964b6
0e5ad7c
8670b13
d801b87
53d8464
cabb212
318a964
f1a8b00
aefa74c
086dce7
ddd7f53
9f1e465
1574fc4
3b5acf3
f978045
037089f
04f5e14
f594d51
582b1e8
e4a95c6
b53ed9d
8cd7f42
dce9b91
90aa959
3c4296f
3f3312d
9858e54
16d7d4d
99b39d5
34c5526
933aa3c
30f3f96
de3c2f3
ce5e52a
b2a6964
9a3117b
596df4d
f5a3bb6
84786a2
c8790a1
f4acead
63fd956
5e5e2c0
cb0c506
d85c90a
7d0c4db
a92e7f6
114b4e0
011178c
9c48846
9829e68
869dd49
b43c547
043b24a
6c9e233
2818fa4
c3ee8f9
5d43830
46e3d06
af2c906
b1b8bf0
f0d0186
b006276
9d1adec
e4094c9
ca6d811
bd826f4
1f9c10e
61b049e
29a13b6
e19d492
085730f
f51645e
f138da4
e3f2d68
954a58f
27b963b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -26,6 +26,8 @@ NS_ASSUME_NONNULL_BEGIN | |||
@property (nonatomic, assign, readonly) BOOL textWasPasted; | ||||
@property (nonatomic, copy, nullable) NSString *placeholder; | ||||
@property (nonatomic, strong, nullable) UIColor *placeholderColor; | ||||
@property (nonatomic, readwrite, nullable) NSString *currentAccessibilityError; | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fabriziobertoglio1987 I'm getting a build error because this differs from Libraries/Text/TextInput/Singleline/RCTUITextField.h Re
Which property declaration should we use? cc @blavalla if you have thoughts There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lunaleaps The RCTTextInputComponentView.mm is the iOS Fabric Component for TextInput. Creates an instance _backedTextInput of type BackedTextInputViewProtocol using RCTUITextView or RCTUITextField constructors. react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm Line 78 in f0d0186
BackedTextInputViewProtocol, RCTUITextView and RCTUITextField headers should match (currently they don't, accessibilityErrorMessage type differs in RCTBackedTextInputViewProtocol). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fix_wrong_type_error.mp4 |
||||
@property (nonatomic, readwrite, nullable) NSString *previousAccessibilityError; | ||||
|
||||
@property (nonatomic, assign) CGFloat preferredMaxLayoutWidth; | ||||
|
||||
|
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.
What is the rationale for adding two props when they could instead be one? (Also, probably no need to reiterate in the type of the prop in the docblock, since the type is already defined and will be presumably extracted for any reference documentation purposes.)
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.
@yungsters It was introduced with the code review #33468 (comment)
Adapts this API to the Web API aria-errormessage
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-errormessage
This API (
accessibilityErrorMessage
andaccessibilityInvalid
) is built with the Android AccessbilityNodeInfo API. The API allows adding accessibility information to each View node and announcing them with talkback.Each android view and widget (EditText, TextView, or View) stores its accessibility information in its AccessibilityNodeInfo node.
TalkBack reads this information when the screenreader's focus changes.
The error message of an EditText is set on the AccessibillityNodeInfo using the following APIs:
#33468 (comment)
This API relies on the AccessbilityNodeInfo#setError. It does not add a hidden text element to the View hierarchy using react-native. Instead, we use the Native Android API (AccessibilityNodeInfo) to add the error message to the node of the EditText. TalkBack announces the error message when focusing on the EditText (TextInput).
I'll be happy to remove the accessibililtyInvalid prop and further improve the PR. 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.
I removed the documentation in the docblock of prop accessibilityInvalid with commit cb0c506
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.
Hmm this is interesting because it doesn't completely follow the web standards since
accessibilityErrorMessage
doesn't provide the elementId but it also introduces this extra logic aroundaccessibilityInvalid
which is part of the web standards.I think I'd lean towards keeping the two separate props but including in the documentation that we're modelling this after
aria
standards. See my comment here: facebook/react-native-website#3010 (comment)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.
Thanks a lot, @lunaleaps. I updated the documentation as requested with commit facebook/react-native-website@8510e8c 🙏 .
preview of the documentation