Skip to content
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

Fixed client-dots and labels not showing on zoom levels >= 19 #12

Merged
1 commit merged into from
Mar 28, 2016
Merged

Fixed client-dots and labels not showing on zoom levels >= 19 #12

1 commit merged into from
Mar 28, 2016

Conversation

yayachiken
Copy link
Contributor

This commits checks whether a maxZoom is set for a mapLayer in config.json and sets maxZoom on the client and label layers to the maximum value of these maxZoom values.

If no maxZoom was given, the leaflet default maxZoom of 18 is assumed.

var maxLayerZoom = Math.max.apply(Math, config.mapLayers.map(
function(d) {
return (typeof d.config !== "undefined" && typeof d.config.maxZoom !== "undefined") ? d.config.maxZoom : 18
}))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea whether you can do this in a nicer way. Anyone?

@ghost
Copy link

ghost commented Mar 23, 2016

First please shorten the commit name and prefix a "map: ".
Also does this use the maxZoom of the current map source?

@yayachiken
Copy link
Contributor Author

What do you mean with "current map source"?

* client dots and labels only showed up on zoom
  levels up to 18
@ghost
Copy link

ghost commented Mar 23, 2016

"mapLayers": [
{ "name": "MapQuest",
"url": "https://otile{s}-s.mqcdn.com/tiles/1.0.0/osm/{z}/{x}/{y}.jpg",
"config": {
"subdomains": "1234",
"type": "osm",
"attribution": "Tiles © <a href="https://www.mapquest.com/" target="_blank">MapQuest, Data CC-BY-SA OpenStreetMap",
"maxZoom": 18
}
},
}

@ghost ghost closed this Mar 23, 2016
@ghost ghost reopened this Mar 23, 2016
@yayachiken
Copy link
Contributor Author

Yes, this is an element of config.mapLayers in my code.

@ghost
Copy link

ghost commented Mar 23, 2016

I see now.
Description still could be a bit more precise, but it is OK.
I guess you have tested it. I don't have my computer with me right now.
As soon as you give an OK I will merge.

@yayachiken
Copy link
Contributor Author

I tested it, but I would appreciate another double-check.

My current dev environment is a bit strange, and it could have happened that something broke when transferring the patch to somewhere where I could push :)

@ghost
Copy link

ghost commented Mar 23, 2016

Then I will test it in the evening and merge it then.

@ghost ghost merged commit 5fce722 into hopglass:master Mar 28, 2016
This pull request was closed.
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.

1 participant