openspacelabs / react-native-zoomable-view

A view component for react-native with pinch to zoom, tap to move and double tap to zoom capability.
MIT License
191 stars 58 forks source link

BindToBorders is not functioning after backgrounded app. #21

Open professorkolik opened 2 years ago

professorkolik commented 2 years ago

First of all, Want to say huge thank you for the chance to use this library, which saved me a lot of boilerplate code which was impossible to maintain.

So my discovery is after interaction with ZoomableView and afterwards unmounted component, moving back from backgrounded APP (backgrounded more than 1 minute) makes bindToBorders not shifting back to borders.

One asumption is missing removeListeners on componendDidUnmount at all. But not really sure that this can cause the wrong behavior.

Other assumption is that ZoomableView is placed inside the react-native Modal component in my APP, have to double check it.

Any advices would be helpful

update react-native-modal has no effect on issue

professorkolik commented 2 years ago

Anything you can advice here? @elliottkember @thomasttvo

thomasttvo commented 2 years ago

@professorkolik this is strange. I'm not able to reproduce it. Are you able to reproduce with this snack https://snack.expo.dev/@thomasttvo/e5b899?

OleksiiKuchma-Wix commented 2 years ago

I think this issue is related to one I've experienced with react-native-tab-view@3. It uses react-native-pager-view underneath which removes view from window natively without unmounting the component. So what happening is you call measureInWindow on component which detached from any window. This call returns empty values for width and height which means originalWidth and originalHeight becomes empty and bindToBorders doesn't work any more.

https://github.com/openspacelabs/react-native-zoomable-view/blob/9732fe20ba9734ef372e53339b4eaad0aed853a1/src/ReactNativeZoomableView.tsx#L324

professorkolik commented 2 years ago

@OleksiiKuchma-Wix That make sense, do you have any ideas how that can be solved?

I think if I execute grabZoomSubjectOriginalMeasurements on AppState change might help to resolve that

OleksiiKuchma-Wix commented 2 years ago

I don't think it would be enough. I'm not sure why measureInWindow is being used. Probably measure method or values from onLayout could be used. Or at least originalWidth and originalHeight should be updated if measureInWindow returns non-empty values.

danielreuterwall commented 1 year ago

I stumbled upon this issue as well. Can't really figure out when it happens for me but in some occasions when after the component has been unmounted the measurements fails to update from undefined values, that being originalWidth and originalHeight as mentioned above. This lead to strange behaviours when zooming as well as bindToBorders not working.

If I only update the state values in the onLayout call using measureInWindow and skip the InteractionManager.runAfterInteractions and setTimeout used in grabZoomSubjectOriginalMeasurements it seems to work fine. I also skipped the measurements on componentDidMount and the periodical call in setInterval.

Although it seems to work I not totally comfortable with the changes as I guess there was a reason for them in the first place. Any input @thomasttvo, @OleksiiKuchma-Wix?

Update: I was a little too quick with my comment, seems to be working just fine on Android but not on iOS. Will investigate... Update2: What happed on iOS was the measureInWindow got a wrong value for y, originalPageY when called from onLayout. Reverted my changes and it got fixed in the interval updater. Changing it to measure and it gets the correct value. On Android I found the sometimes state isn't updated in grabZoomSubjectOriginalMeasurements, seems like runAfterInteractions is not fired. Haven't found out exactly when this happens but it does after some mount/unmount. If I update the state directly in onLayout with out runAfterInteractions it seems to go away. Will do more tests :)