Skip to content

Conversation

@elliotlewis
Copy link

In the situation of having 1 (or more markers) and fitBounds = false. Being option to override the default zoom level of 10.
*If Google Maps isn't using fitBounds, map requires center-point and zoom: ttps://developers.google.com/maps/documentation/javascript/tutorial#MapOptions

In the situation of having 1 (or more markers) and fitBounds = false. Being option to override the default zoom level of 10.
*If Google Maps isn't using fitBounds, map requires center-point and zoom: ttps://developers.google.com/maps/documentation/javascript/tutorial#MapOptions
@objectivehtml
Copy link
Owner

This code is somewhat redundant. You can see on line 94 of that file you edited that the default options object is merged with a new object, and the options that are passed in the template. So what you are doing there is effectively doing the same thing as below, except its arbitrary to a specific option key. Make sense? Why can't you just override the zoom in the template and set fitBounds to false?

[bugfix] fixed so options argument works correctly for zoom
@elliotlewis
Copy link
Author

@objectivehtml Ah, line 94 is where the bug is. I can see that the underscore.js extend function combines objects, but you're defining this.options by combining defaultMapOptions and the as yet undefined this.options.

It should be combining with the passed-in options or defined options={}

ie.

this.options = _.extend({}, defaultMapOptions, options);

not

this.options = _.extend({}, defaultMapOptions, this.options);

If you're not defining the defaultMapOptions zoom with the passed-in options.zoom then it seems redundant to define center with options.lat and options.lng

I guess the options argument would have to have a center property with lat/lng object but at least that follows Google Maps API docs. eg.

options = {
 center: { 10, 20 },
 zoom: 16
}

*NB I can't see how to squash these commits together in GitHub but I think you can during the merge.

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.

2 participants