-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 accessibilityHint matcher #2313
Comments
Accessibility hint is not a throwaway string; it is supposed to be a localized string on iOS and is use in the accessibility system: https://developer.apple.com/documentation/objectivec/nsobject/1615093-accessibilityhint?language=objc So I don't see how that is a good candidate for uniquely identifying elements. You might as well use by.text or by.label. Since testID is translated to |
The text on that link makes no sense. The accessibility identifier is intended to be a one to one way to match your UI elements. If you expect an element to be visible and then interact with it, I don't see how that gives you a "false sense of security", unless you have the same identifier for multiple UI elements. On the other hand, abusing the accessibility system for things it is not intended to has actual real-world harm for disabled users. |
Thank you for the feedback !
If I have an icon to press for instance, without text or label, or an uncontrolled input (but you know what the item does) I find it to make more sense to use the accessibility description of what the icon does rather than just adding a That being said, I understand the technical limitation. |
If you care about accessibility, any image view should have a label, as things like Voice Over use the label to read to the user. While labels and hints have different technical meaning, they are so close in their ability to distinct UI elements (e.g. not very good), I still don't see a point. You should be setting hints and labels not because of tests, but because you care about accessibility and your disable users. If you add hints that are unique just for tests, and your users use Voice Over, the real-world harm is that they will hear strings that are not intended for the users to hear. This is why on iOS and Android, you have accessibility identifiers just for this purpose. |
I'm confused, but where did you see that accessibility options are written only for tests ? They are written alongside text / label for disabled people. Sometimes they are even the only information about a component (for instance an icon button). |
If your hints are intended for users, that's fine, but why would you add a hint but not add a label? My point here is that label and hint are very analogous from a test's matching standpoint. Text and label matching are useful when it is impossible to set an identifier, such as in system views. They are discouraged, but provided just in case. Hint would be equally discouraged, so I just don't see the point. Even Apple doesn't see the point, as it is not exposed in its own testing API. Apple also doesn't expose text, but we felt this was important enough to have (as text is different from label in some cases). I don't feel this is the case for hint. |
By all means, you should be using by.id() as much as possible. |
Hey, a RNTL maintainer here 👋. I think we were too harsh about this "recommendation" of avoiding using testIDs. Our intention was to force users a little to think about accessibility, which in our experience is often forgotten when developing a UI, unfortunately. Making test queries accessibility-oriented increases the chance for a developer to do so. Using testID is fine in most cases, but we believe the textual matchers should be used first, because they resemble user behaviors more closely. Users can't see testIDs and while the ID may not change, a text it represents may – I can imagine a button that mistakenly changes "confirm" to "cancel" because some text/replace went too far. The semantics have changed, while keeping the same ID, so a test would happily pass.
I couldn't agree more. I've seen people blindly replacing testID with accessibilityHint because IDs are "not recommended" and it's just plain wrong in most cases. And if something should have been discouraged, accessibilityHint would be a better candidate now IMHO, since it accompanies accessibilityLabel which can uniquely identify a view in most cases. If not, testID is a way to go then. |
Hello, There are several problems with textual matching that we've seen people repeat over and over; 1. they are localized strings, so sometimes language and/or locale of the simulator on CI doesn't match the language and/or locale of the one writing tests (Detox also offers tools to change language and or locale to test different configurations—this would be impossible to test by text); 2. user facing text changes over time, and tests tend to break; 3. the chances of several elements having the same text or label (or hint for that matter) increases as the size of the application grows. I'm not sure why users not seeing the accessibility identifiers is important. We do offer expectations that test writers can execute to ensure their views have the right text and/or label. If a user is afraid the text of a button might change, they can assert that it is "confirm" instead of "cancel". I'd say that is quite the callous copypasta behavior if someone makes such an error. 😄 Internally, we push everyone to add identifiers. We've experienced many of the issues above multiple times over the years. Anyone should write tests as they see fit, of course, but my recommendations come from our extensive experience both internally and with the open-source community. |
Very true for e2e tests, not a big deal for JS tests because we mock stuff as we like.
We bump into this as well. However as I've spoken to Satya, one of the maintainers, they implement a bunch of accessibility props to hide screens from the users, and we (RNTL) should probably respect that when querying. We have other means to do so as well. I really appreciate the experience-based approach, good callouts 👍. I'm going to adjust the wording around byId queries, or maybe remove the warning entirely. Thanks! |
Thank you for taking the time to discuss! 🙂 I just want to say that Detox might be unique in that we operate within the confines of the native world. We have no access to the JS, nor operate at that level. Once an action or an expectation in Detox is run, it travels to the native world, where we observe the native view hierarchy. A JS testing framework might exhibit a different set of experiences. With react-navigation, the smart thing to do for its users isn't to just hide the views, but to remove them altogether from the hierarchy. Even a hidden view gets laid out by the OS framework, and having several layers of views deep being laid out just doesn't make sense from a perspective standpoint. We have our own navigation library, |
React Navigation integrates with React Native Screens which does remove views from hierarchy. The plan is to enable it by default soon. Regardless, React Navigation also hides these inactive screens from accessibility tree by using relevant accessibility props. Accessibility tools like VoiceOver and TalkBack don't read contents of inactive screens. If the matcher would respect the accessibility tree, then it wouldn't be a problem. I believe tools like Appium work correctly in this case. |
We operate on the view hierarchy level, not accessibility tree on purpose, unlike Apple's own testing framework, for example (which Appium can use as a driver). This gives us a much more accurate method of matching views. In some cases, you want to match a view and assert it isn't visible, for example. But as I said in another comment, even a hidden view gets laid out at the OS level. This, coupled with React Native and its userbase tendency to create layers upon layers of wrapper views, causes HUGE hierarchies to be laid out needlessly. This can cause serious performance issues and battery drain. |
Is your feature request related to a problem? Please describe.
As suggested by react-native-testing-library, I'm trying to move away from using testID when possible in favor of Accessibility options.
Describe the solution you'd like
Add a
by.hint
orby.a11yHint
matcher.The text was updated successfully, but these errors were encountered: