-
Notifications
You must be signed in to change notification settings - Fork 273
feat: introduce ByLabelText, ByRole and ByHintText a11y aliases #445
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
Conversation
…m @testing-library/react-native
… @testing-library/react-native
… @testing-library/react-native
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.
Great start! Now we need a simple test (copy what we have already) and update typings alongside with their tests (for TypeScript, check typings
directory)
Thanks @thymikee! I duplicated the tests and updated the types. I'm not quite sure what |
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.
Overall LGTM.
nit: No sure if I would actually duplicate the unit tests for function aliases though. People reading code might be confused whether these are aliases or actual functions. Please put the expect
s from new query aliases next to expect
s from original A11y queries in the respective test suits.
Duplicated typescript typing "tests" are fine. As this is different.
Fair point @mdjastrzebski. I think they should fall under the same test blocks since they are covering the same functionality. I'll update this now. |
I have consolidated the tests, but there were some issues with this. I added some Also, I updated the docs to include these new queries. |
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.
Spotted few aliases to rename.
Thanks for your review @thymikee and @mdjastrzebski! I just pushed an updated a11yAPI test file that asserts referential equality for the query aliases. The test suite is passing, and the diff is a lot simpler now. 😄 Please let me know if there are any other changes either of you feel are necessary. |
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.
Looks good, left one more note to consider 👍
…into their own tests
Thank you very much for you contribution @ryanjwessel, appreciated! |
Thank you both for your help! This was my first ever contribution to OSS, so I am really grateful for your guidance in contributing to this project. Glad I was able to contribute! |
Summary
Users migrating from
@testing-library/react-native
will have to update their accessibility queries due to a slightly different naming convention. This PR adds aliases for the@testing-library/react-native
queries to be usable in this library.This is motivated by the thread on Issue #442
Test plan
I am not sure about testing this. I can make additions to this PR if needed. The test suite passes, and I see
a11yAPI.test.js
uses only thea11y
version of the queries.To Note
The queries for placeholder text are not one-to-one between the two libraries, but I don't think those are set up for aliasing yet. All of the
@testing-library/react-native
placeholder queries are appended withText
, whereas this library does not do that.