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
191 stars 58 forks source link

Disable pan on initial zoom #44

Closed danielreuterwall closed 1 year ago

danielreuterwall commented 1 year ago

Added prop, disablePanOnInitialZoom, to control if pan is enabled when zoom level is 1. Another take would be to add a panEnabled prop to control this but I figured as this is a common request, a dedicated prop would be justified.

Fixes openspacelabs/react-native-zoomable-view#38

elliottkember commented 1 year ago

This looks great! I think this is a great idea.

Could you please add a brief comment above the change, and update the README in this PR? Happy to approve and merge if so.

danielreuterwall commented 1 year ago

This looks great! I think this is a great idea.

Could you please add a brief comment above the change, and update the README in this PR? Happy to approve and merge if so.

Of course, I'll take care of it next week. Good input 👍

danielreuterwall commented 1 year ago

Isn't disablePanOnInitialZoom misleading since you can change initalZoom, but this feature always checks for zoomLevel === 1?

Yeah, it probably is. Maybe something like disablePanOnDefaultZoom? Or just disablePanOnZoom1? What do you think?

danielreuterwall commented 1 year ago

@elliottkember README updated and code changes commented. What's your input in the prop naming mentioned by @janpe?

janpe commented 1 year ago

Yeah, it probably is. Maybe something like disablePanOnDefaultZoom? Or just disablePanOnZoom1? What do you think?

Would it be difficult to change the logic to compare to the user selected initial zoom? If the user has set initialZoom to 1.5 it wouldn't make much sense to have this option disable panning at zoomLevel 1. But I guess of those two disablePanOnDefaultZoom would probably be better.

danielreuterwall commented 1 year ago

Yeah, it probably is. Maybe something like disablePanOnDefaultZoom? Or just disablePanOnZoom1? What do you think?

Would it be difficult to change the logic to compare to the user selected initial zoom? If the user has set initialZoom to 1.5 it wouldn't make much sense to have this option disable panning at zoomLevel 1. But I guess of those two disablePanOnDefaultZoom would probably be better.

Sound perfectly reasonable, not a difficult change at all. Probably the better choice than renaming the prop.

elliottkember commented 1 year ago

Would it be difficult to change the logic to compare to the user selected initial zoom? If the user has set initialZoom to 1.5 it wouldn't make much sense to have this option disable panning at zoomLevel 1. But I guess of those two disablePanOnDefaultZoom would probably be better.

Comparing to initialZoom is a great call. I'd suggest disablePanAtInitialZoom with at rather than on. The "on" feels like an event listener (onPress), whereas disabling "at" a zoom level feels right to me.

Thank you both for the contribution!!

janpe commented 1 year ago

Yeah, it probably is. Maybe something like disablePanOnDefaultZoom? Or just disablePanOnZoom1? What do you think?

Would it be difficult to change the logic to compare to the user selected initial zoom? If the user has set initialZoom to 1.5 it wouldn't make much sense to have this option disable panning at zoomLevel 1. But I guess of those two disablePanOnDefaultZoom would probably be better.

Sound perfectly reasonable, not a difficult change at all. Probably the better choice than renaming the prop.

@danielreuterwall when do you think you'll have time to make these changes? 😊

danielreuterwall commented 1 year ago

Yeah, it probably is. Maybe something like disablePanOnDefaultZoom? Or just disablePanOnZoom1? What do you think?

Would it be difficult to change the logic to compare to the user selected initial zoom? If the user has set initialZoom to 1.5 it wouldn't make much sense to have this option disable panning at zoomLevel 1. But I guess of those two disablePanOnDefaultZoom would probably be better.

Sound perfectly reasonable, not a difficult change at all. Probably the better choice than renaming the prop.

@danielreuterwall when do you think you'll have time to make these changes? 😊

Thanks for the reminder :) Just pushed an update.

janpe commented 1 year ago

@elliottkember when do you think this could be merged and released? I'd have use for this feature straight away 😁

tauntmachine commented 1 year ago

guys can you please ship this feature on priority? It would great help.

manarfalah commented 1 year ago

is there any way to disable panning beyond edges also when zooming ? I am using disablePanOnInitialZoom but I want this behavior also when I zoom and get to edge of image.