Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable pan on initial zoom #44

Merged

Conversation

danielreuterwall
Copy link

@danielreuterwall danielreuterwall commented Oct 14, 2022

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 #38

@danielreuterwall danielreuterwall changed the title Disable pan on initial zoom Disable pan on initial zoom, fixes #38 Oct 14, 2022
@danielreuterwall danielreuterwall changed the title Disable pan on initial zoom, fixes #38 Disable pan on initial zoom Oct 14, 2022
@elliottkember
Copy link
Collaborator

elliottkember commented Oct 16, 2022

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
Copy link
Author

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 👍

Copy link

@janpe janpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@danielreuterwall
Copy link
Author

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
Copy link
Author

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

@janpe
Copy link

janpe commented Oct 17, 2022

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link

janpe commented Oct 20, 2022

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
Copy link
Author

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
Copy link

janpe commented Oct 22, 2022

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

@tauntmachine
Copy link

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

Copy link
Collaborator

@elliottkember elliottkember left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@elliottkember elliottkember merged commit 15bc4e2 into openspacelabs:master Oct 26, 2022
@manarfalah
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable Panning when Zoom 1
6 participants