search

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
205 stars 57 forks source link

Removed potentially brittle logic #7

Closed thomasttvo closed 2 years ago

elliottkember commented 2 years ago

The only concern I have with this is that it removes functionality from the original library. It's not a problem in itself, but can you remind me of the reason for this change? Might be best to document the change more extensively.

thomasttvo commented 2 years ago

The only concern I have with this is that it removes functionality from the original library. It's not a problem in itself, but can you remind me of the reason for this change? Might be best to document the change more extensively.

The function I removed is no longer called. When we removed TouchableWithoutFeedback, we've been handling tap gestures by ourselves now. To handle tap gestures, we're now relying on _handleStartShouldSetPanResponder to return true, which means, on "touch start", the pan responder is already set. This leads to _handleMoveShouldSetPanResponder never being called by React Native, which makes sense because the pan responder was set before "touch move", so RN doesn't have to ask whether it needs to set pan responder again when touch moves.

If what I said above makes sense, I can add those details to the comment here https://github.com/openspacelabs/react-native-zoomable-view/pull/7/files#diff-1404f9bb246f0f5898432be5a8aa3279875b89070ef41a5c790ebbddcf8079a5R326-R330

elliottkember commented 2 years ago

Oh right, that makes sense. I forgot we did that. Adding it to the comment sounds good!