react-native-popover-view icon indicating copy to clipboard operation
react-native-popover-view copied to clipboard

Please add testID prop to improve testability

Open effinsky opened this issue 3 years ago • 7 comments

Please add testID prop to improve testability with jest and react testing library

effinsky avatar Oct 29 '21 10:10 effinsky

Can you link to an example or docs about how you would use this prop? I'm not familiar with react testing library, and thus don't know what I would do with the value you would pass in this new prop.

SteffeyDev avatar Nov 09 '21 17:11 SteffeyDev

Hi, as far as I am aware now, a testID prop--standard for both native RN components and 3rd party libs--is pretty much the only way in this case to "grab" this component in a test so as to verify its behavior. TestID is not the optimal way to test components, but in this case I cannot see another way to refer to the popover directly so as to check its properties etc. For example, I can search for certain text that a child of Popover is supposed to render, but then I will have grabbed the component that has that text, not the Popover itself. Meanwhile, testID, if it were possible to give it to the Popover, would let me do that.

Screenshot 2021-11-09 at 20 16 09

https://reactnative.dev/docs/view#testid https://reactnativetesting.io/component/testing.html#interaction

Thanks in advance--

effinsky avatar Nov 09 '21 19:11 effinsky

Ok, so it sounds like I just need to add the testID property to the TypeScript type so that you can pass it as a prop without it getting upset right? It doesn't look like I have to actually do anything with the string you pass in.

SteffeyDev avatar Nov 28 '21 20:11 SteffeyDev

Yes, it would seem so, thanks!

On 11/28/2021 9:26 PM, Peter Steffey wrote:

Ok, so it sounds like I just need to add the |testID| property to the TypeScript type so that you can pass it as a prop without it getting upset right? It doesn't look like I have to actually do anything with the string you pass in.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SteffeyDev/react-native-popover-view/issues/126#issuecomment-981146392, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANACK57YXF5372V5WACITG3UOKF7LANCNFSM5G66DDWA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

effinsky avatar Nov 28 '21 22:11 effinsky

Seems like it's not just a TS thing. Seems like you actually need to add the testID to the props accepted by the component, same as on the React Native's other built-in components and those provided by community libs like React Native Elements or Native Base. Thanks in advance!

effinsky avatar Dec 05 '21 08:12 effinsky

So, technically React and JS allows you to pass any props to a component, because props is just a JS object, so there is no concept of accepting a prop. I can choose to read the value of a prop, but if I'm not going to do anything with it there's no need to read it. TypeScript is the only part of this that doesn't allow props to be passed in unless they are defined within the component. Hopefully that makes sense.

If you want me to implement it the same way as another open source project, please link me to the place in their code where they use this prop so that I can see what they are doing.

SteffeyDev avatar Dec 09 '21 04:12 SteffeyDev

@effinsky I see examples like this where they add a testID prop to an underlying component, such as a view, so that the view can be grabbed. This is very different from adding a testID prop to the Popover component itself. Is this still a feature you are looking for?

SteffeyDev avatar Feb 24 '22 19:02 SteffeyDev

I can't speak for the OP, but I came across this thread when searching for a way to do the same thing. Perhaps PopoverProps can extend off of ViewProps so any "common" props (such as testID or accessibilityRole) can be applied to the Popover component as well? I don't think applying a testID to a Popover child would help the case I'm trying to cover (that some values I'm calculating are being passed as expected to <Popover> specifically)

ginnymin avatar Sep 02 '22 01:09 ginnymin

I don't want to accept arbitrary view props, but I'm fine with accepting a subset. So, for your use case, would it be sufficient to just include Pick<ViewProps, 'testID'> into the public Popover props?

SteffeyDev avatar Sep 07 '22 23:09 SteffeyDev

That would work for me! Thanks so much

ginnymin avatar Sep 08 '22 05:09 ginnymin

Ok, released as 5.1.4

SteffeyDev avatar Sep 20 '22 14:09 SteffeyDev

Feel free to re-open if this doesn't work for you @ginnymin

SteffeyDev avatar Sep 20 '22 14:09 SteffeyDev

Hi @effinsky and @ginnymin. Sorry that this is unrelated to this issue, but have either of you successfully tested this Popover component with react-native-testing-library? I created issue #159 since I wasn't able to get it working, but was hoping you could share some advice if you did get it working

myou11 avatar Mar 06 '23 22:03 myou11

@myou11 I haven't tested this update yet, sorry I can't be of any help yet!

ginnymin avatar Mar 08 '23 00:03 ginnymin