-
-
Notifications
You must be signed in to change notification settings - Fork 860
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
Add minZoom
property to FitBoundsOptions
#1562
Conversation
Hey @Robbendebiene, thanks for the contribution, it certainly looks like something we'd like to have, and the code looks LGTM I think. I just need to give it a quick test, which I should be able to do in the next few days. At this time, #1551 is taking priority over all other PRs, as it implements significant breaking changes. |
Hi @JaffaKetchup, I think integrating this one way or the other should be rather trivial. So I don't mind resolving conflicts after #1551 has landed. |
Ok, I'll set this as draft for now then, waiting for #1551 to be merged. |
minZoom
property to FitBoundsOptions
Hey @Robbendebiene, if you're around, can you resolve the conflicts please? #1551 has been merged. |
7d67a90
to
2c60d63
Compare
Sorry, I accidentally closed this PR. I'm working on the updates. |
Use 0 as default value since negative zoom values are very unusual
@JaffaKetchup So this is tested and ready for review from my end :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for your contribution!
The
centerZoomFitBounds
is a very powerful method and can be used for a lot of different use cases.Sometimes one doesn't only want to limit the
maxZoom
but theminZoom
level. For example to ensure, that we won't zoom out past a certain zoom level even if this means that the supplied bounds won't fit entirely within.Clamping the zoom afterwards doesn't work, because the center calculation depends on it.