-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
Adding some Geoportail France Maps #437
Adding some Geoportail France Maps #437
Conversation
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!
maxZoom: 18, | ||
// Get your own geoportail apikey here : http://professionnels.ign.fr/ign/contrats/ | ||
// NB : 'choisirgeoportail' is a demonstration key that comes with no guarantee | ||
apikey: 'choisirgeoportail', |
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.
Requiring people to edit this JS file to change the api key is not ideal. But it is better than nothing :)
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.
I agree with you. Setting it in Maps_Settings.php would have been the right place for this (as for Mapbox key for instance), I made some unsuccesful tries to do it that way and temporaly gave up.
If you have some hints to do this that way, I will change this in a new PR.
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.
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, I'll cast an eye on it.
} | ||
} | ||
} | ||
} |
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 indent here is rather messed up. I can fix that easily enough after merge so no big deal, but beware that normally proper indent is required.
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.
Sorry for this.
@gcebelieu I did not realize this when merging the PR: you modified an included library. That means when the library is upgraded, we will lose your changes unless we re-apply them. So it is really better to define the layers elsewhere. That might also make it easier to access the settings. |
@JeroenDeDauw you're right ! I didn't realize either that leaflet-providers was an external project. Ideally I may try to push this PR on that project and think back settings stuff here or there. |
Right. Seems like time anyway to update the copy we have here as it is two years old. |
Hi @PeterTheOne, @JeroenDeDauw, |
Has it also been released? Then I can just upgrade the library here to its latest version. Or you can submit a PR doing that if you want it to happen soon. |
Yes it has just been released : https://github.com/leaflet-extras/leaflet-providers/releases/tag/1.3.0 |
This PR adds the possibility to display French Geoportal Maps to Maps extension with leaflet provider.
Four maps types are added :
Note that Geoportal Maps need an access key to be displayed. This PR uses a default public key that can be used as is but comes with no guarantee.
Exemples :