react-native-testing-library
react-native-testing-library copied to clipboard
callstack
feat: ...byRole type queries accepts second arg
@kiranjd is it something you're still interested in getting in? This is a feature that is clearly lacking for our current API, and I'd love to help you getting that in a mergeable state.
@AugustinLF Missed following up on this. I remember I had a few questions. I'll post the questions and follow it up to finish within this week. Thank you 🙏
@kiranjd Do you need any help, is there anything I can do to help you getting that in? :)
Let's pause this PR for couple of days till we get a11y query reorganisation from #325 done. This should allow for easy separate query options type per each query, so that the name options will be allowed only for byRole queries and not all a11y queries.
Yes, those changes caused lot of conflicts
On Wed, 4 May 2022 at 3:58 PM, Maciej Jastrzebski @.***> wrote:
Let's pause this PR for couple of days till we get a11y query reorganisation from #325 https://github.com/callstack/react-native-testing-library/issues/325 done. This should allow for easy separate query options type per each query, so that the name options will be allowed only for byRole queries and not all a11y queries.
— Reply to this email directly, view it on GitHub https://github.com/callstack/react-native-testing-library/pull/875#issuecomment-1117156054, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGFANA3PQMXZBXCJ744WUCDVIJGMTANCNFSM5JHKFNKQ . You are receiving this because you were mentioned.Message ID: @.***>
-- Kiran JD
@mdjastrzebski is this a good time to take this PR further? If so, should I raise a separate PR to continue the work?
@kiranjd I feel like yes we should be able to start working on that once again. I'd however wait for the go from @mdjastrzebski who might have something else planned before that. Nonetheless, feel free to let me know if you need any help on that, I'm very keen in getting this one in.
@kiranjd go ahead 👍🏻 Now is the right time to work on this PR, as we've got a11y properly refactored.
@kiranjd is there anything we can do to help you to get that in? It'd bring so much value, I'm really looking to get that one in <3
@AugustinLF Thanks for the support. I will work on this and hopefully get this in a mergeable state by this weekend 🤞🏻
@kiranjd if you're short on time to finish this PR I can definitely help fixing the last few things to get it merged, let me know if you want help :)
@AugustinLF Have tried to redo the changes from the last state when it was working. There seems to be a lot of refactoring undergone in the meanwhile. I would really appreciate your help in taking this further. Thank you for understanding :)
@kiranjd yeah RNTL codebase did move a lot since the original PR submission :-)
@AugustinLF I would be great if you could help to move this forward, as byRole query with name option has a huge potential to be one of the top recommended queries as in the case of RTL: https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#not-using-byrole-most-of-the-time
This PR has been rebased and updated by @AugustinLF as #1127. Thank you @kiranjd 👏🏻 for the initial implementation!