improve maxBounds handling #3683
Conversation
…, fix camera.fitBounds for single points
| @@ -296,7 +296,8 @@ class Camera extends Evented { | |||
| * Pans and zooms the map to contain its visible area within the specified geographical bounds. | |||
| * | |||
| * @memberof Map# | |||
| * @param {LngLatBoundsLike} bounds The bounds to fit the visible area into. | |||
| * @param {LngLatBoundsLike} bounds The bounds to fit the visible area into. If bounds contains | |||
| * only one point, the map center transitions to that point at zoom level options.maxZoom or {@link Map#getMaxZoom}. | |||
There was a problem hiding this comment.
Neat! I didn't know that Map#fitBounds did this. Should we expand this parameter's type signature to LngLatBoundsLike | LatLngLike?
There was a problem hiding this comment.
It used to error on LngLatBounds where ne + sw were the same point, this PR fixes that because that case is technically a valid instance of LngLatBounds. I could make the docs language more clear to that effect. I don't think we should accept LngLatLike because I don't think its a recommended use-case for fitBounds 😕
| @@ -329,6 +329,7 @@ class Camera extends Evented { | |||
| scaleX = (tr.width - options.padding * 2 - Math.abs(offset.x) * 2) / size.x, | |||
| scaleY = (tr.height - options.padding * 2 - Math.abs(offset.y) * 2) / size.y; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Would you mind cleaning up the two whitespace changes in this file? ☝️
| * @param {LngLatBoundsLike} bounds The bounds to fit the visible area into. | ||
| * @param {LngLatBoundsLike} bounds The bounds to fit the visible area into. If bounds contains | ||
| * only one point (i.e. bounds.ne and bounds.sw are the same location, the map center | ||
| * transitions to that point at zoom level options.maxZoom or {@link Map#getMaxZoom}. |
There was a problem hiding this comment.
I understand now. bounds.ne === bounds.sw is an edge case that demonstrates what happens when the zoom that would best fit the bounds in the viewport is greater than Map#getMaxZoom().
Is there a way we can phrase these docs to make the more general pattern evident?
Here's a rough sketch:
Center these bounds in the viewport and use the highest supported zoom level (up to and including
Map#getMaxZoom()) that fits them in the viewport.
remove upper limit of 20 for Map#setMaxZoom, fix map.fitBounds(bounds) for a single point
fixes #3677 #3307
Launch Checklist
cc @lucaswoj @mourner