Padding options are not passed to fitBounds call

[nicksellen]

Description

padding options are not always used, e.g.

Vue2Leaflet/src/components/LMap.vue

Line 206 in 5726da2

this.mapObject.fitBounds(this.bounds);

It should be:

this.mapObject.fitBounds(this.bounds, this.fitBoundsOptions);

Live Demo

http://jsfiddle.net/Lje19v7w/2/

Steps to Reproduce

• provide padding-top-left option to l-map component.
• observe map

Expected Results

A padding should be applied to the map view.

Actual Results

No padding is applied.

Browsers Affected

(must be all of them as it's missing the js code, but I only saw it on firefox)

• Chrome
• Firefox
• Edge
• Safari 9
• Safari 8
• IE 11

Versions

(the versions from the forked jsfiddle)

• Leaflet: vX.X.X
• Vue: v1.0.3
• Vue2Leaflet: v1.0.1 (oh the jsfiddle uses quite an old version, but it's also happening in my projects version which is 2.6.0)

[mikeu]

Thanks for the issue report, @nicksellen ! I see that indeed the computed fitBoundsOptions are not being used in a number of calls to fitBounds in the component.

One thing I'll point out is that making the update you suggest wouldn't actually change the behaviour of your fiddle, since this.bounds will be null in that example, so the call to fitBounds in mounted isn't getting made anyway. Nevertheless, I agree that it would be more intuitive if the bounds options applied on mount when the bounds were specified.

@DonNicoJs do you know if there was a historical reason that this wasn't done? Or why the LMap methods setCrs and fitBounds both call mapObject.fitBounds with hard coded options objects instead of using this.fitBoundsOptions?

Vue2Leaflet/src/components/LMap.vue

Lines 321 to 331 in 5726da2

	setCrs(newVal, oldVal) {
	const mapObject = this.mapObject,
	prevBounds = mapObject.getBounds();
	mapObject.options.crs = newVal;
	mapObject.fitBounds(prevBounds, { animate: false, padding: [0, 0] });
	},
	fitBounds(bounds) {
	this.mapObject.fitBounds(bounds, {
	animate: this.noBlockingAnimations ? false : null,
	});
	},

What are your thoughts on changing some or all of these calls? I can put a PR together quickly enough if we decide we want to, but I feel I could use a little more context on why the existing options are set the way they are, first. Especially since it's a change that would alter the behaviour of maps in existing apps.

[Ruschio]

@mikeu I have the same problem of course, are there any news or update about it?
Because that can be very useful to apply default padding on the map

[Ruschio]

Ehi guys, any update about this issue?
I also try to replace all the missing calls with force {paddingBottomRight: [300, 300]}, but I didn't see any change on map.

[nicksellen]

Seems we're waiting on @DonNicoJs to inform which approach to use. Unless @mikeu feels they can proceed without that extra context?

I'd be willing to help out where it's useful (experienced js dev).

[nmezzopera]

Hey everyone! sorry for the long long wait 🙇 I tried to check the code and think if there is any historical reason for hardcoding those value, but nothing comes to mind

@nicksellen if you would be able to create a PR to change those it would be great! then we can move on from there!

I promise to be more active, life got in the way a bit in the last months!

As usual @mikeu thank you so much for all the hard work!

[nicksellen]

@mikeu had said:

I can put a PR together quickly enough if we decide we want to, but I feel I could use a little more context on why the existing options are set the way they are, first.

Are you still up for that?

Otherwise, I can add it to my own todo list or maybe find someone else in our project that is up for it.

[mikeu]

@nicksellen yep, thanks for the reminder, I should be able to get that out in the next few days here.