-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Allow bounds fitting options to be provided for initial bounds. #7681
Conversation
6506698
to
cd4af61
Compare
Complements mapbox#5518 by allowing additional options (notably padding) to be provided for the initial bounds for a map.
cd4af61
to
c450be7
Compare
Resolved initial lint/test issues, pushed amended commit. |
@@ -197,6 +197,7 @@ const defaultOptions = { | |||
* @param {number} [options.bearing=0] The initial bearing (rotation) of the map, measured in degrees counter-clockwise from north. If `bearing` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`. | |||
* @param {number} [options.pitch=0] The initial pitch (tilt) of the map, measured in degrees away from the plane of the screen (0-60). If `pitch` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`. | |||
* @param {LngLatBoundsLike} [options.bounds] The initial bounds of the map. If `bounds` is specified, it overrides `center` and `zoom` constructor options. | |||
* @param {Object} [options.fitBoundsOptions] A [`fitBounds`](#Map#fitBounds) options object to use when fitting the initial bounds provided above. |
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.
How about emphasizing that this is only used for the initial bounds. Maybe something like:
A
fitBounds
options object to fit the initial bounds of the map when thebounds
option is specified.
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.
A
fitBounds
options object to use when fitting the initial bounds provided above.
The version already in there does say that it's for the initial bounds, but perhaps adding only and providing the right syntax note for the bounds
would clarify further?
A
fitBounds
options object to use only when fitting the initialbounds
provided above.
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.
The anchors in the link to the fitBounds API should be all lower-case: #map#fitbounds
. It's currently broken on https://docs.mapbox.com/mapbox-gl-js/api/ :(
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.
Ah, sorry, I'll open another PR for that. TIL that it's legit to have an id
with a hash in it!
I did just copy that link from other documentation, so I'll check around - likely that it's broken elsewhere too.
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.
The one I copied it from has already been fixed in #7791, so just this one to fix :)
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.
Thanks @asheemmamoowala. I didn't see your thumbs up to my more detailed description of the option (GH notifications don't mention those!), so the merged version only had my original description. Would you like a new PR with the change? |
So how could I use it? This doesn't seem to work:
Padding is not applied. Am I doing something wrong here? |
@matyushen this will be available in the next release. @elyobo - a new PR would be great! |
As per discussion with @asheemmamoowala[1] [1] mapbox#7681 (review)
NP @asheemmamoowala, see #7743 |
As per discussion with @asheemmamoowala[1] [1] #7681 (review)
the correct format for this is: fitBoundsOptions: {
padding: 100
} |
Complements #5518 by allowing additional options (notably padding) to be
provided for the initial bounds for a map.
Launch Checklist
@mapbox/studio
and/or@mapbox/maps-design
if this PR includes style spec changesI'm not sure how to manually test the debug page, but have implemented a simple unit test (that adding padding decreases the zoom vs not adding it).
fixes #7553