prscX / react-native-tooltips

React Native: Native Tooltip View

Apache License 2.0

271 stars 40 forks source link

Add parent view reference #16

Closed StevenMasini closed 5 years ago

StevenMasini commented 5 years ago

Change logs

Now users need to give a reference to the parent view where they want the tooltip to fit in. This avoid the tooltip to go off-screen.
Dismiss the tooltips when componentWillUnmount().
Upgrade the version number of the dependency com.github.florent37:viewtooltip to 1.1.6 on Android to take benefit of method on that can retrieve its parent view by itself.

Notes

On Android findNodeHandle might not retrieve the right viewId which end up by having the findViewById to return null. This previously caused a crash which has been fixed in #13.

In order to avoid this bug, developers must set collapsable={false} in the props of their component.

Suggestion

I would like to change the name of the props reference to target to make it more explicit for developers when they set the props to RNTooltips.

prscX commented 5 years ago

Thanks a lot @StevenMasini for the effort and PR.

I believe you have made a good suggestion. We should rename the reference prop to target. Can you please do the required changes. Post that we should be good to merge the raised PR.

Please let me know incase I should work on this.

Thanks </ Pranav >

StevenMasini commented 5 years ago

@prscX No don't worry I can do this. I will update it later tonight and let you know once it's done 👍

StevenMasini commented 5 years ago

@prscX I haven't had time to make the changes. I am quite busy these days. I will update asap.

prscX commented 5 years ago

No Worries @StevenMasini. Please take your time, let me know incase any help is needed.

Thanks </ Pranav >

StevenMasini commented 5 years ago

@prscX I renamed the props reference to target last week, but I forgot to let you know before I go on holiday. Please review the Pull Request and let me know if you have any feedbacks.

I tested my branch against my own project to make sure everything is fine.

As I said on iOS the tooltips are displaying correctly, but I still got issue on Android. Setting the props collapsable to false can help to fix this issue, but not on 100% of the case.

I probably need more time to investigate Android. Also I would suggest to refresh the Example with updated implementation. If that's okay for you I will probably submit another Pull Request about this.

prscX commented 5 years ago

Thanks a lot @StevenMasini for the Effort.

I have merged the same. I will test and release the same.

Thanks </ Pranav >

prscX commented 5 years ago

Hi @StevenMasini: I have pushed the release with your changes V0.0.9.

Can you please verify the same. Please let me know in case you face any issues.

Thanks </ Pranav >