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

Adding some Geoportail France Maps #437

Merged

Conversation

gcebelieu
Copy link
Contributor

@gcebelieu gcebelieu commented Jun 10, 2018

This PR adds the possibility to display French Geoportal Maps to Maps extension with leaflet provider.
Four maps types are added :

  • GeoportailFrance,
  • GeoportailFrance.orthos,
  • GeoportailFrance.parcels,
  • GeoportailFrance.ign_maps

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 :

{{#display_map: 
 |layer=GeoportailFrance
 |service=leaflet
}}

gpmaps

{{#display_map: 
 |layer=GeoportailFrance.orthos
 |service=leaflet
}}

gporthos

{{#display_map: 
 |layer=GeoportailFrance.parcels
 |service=leaflet
}}

gpparcels

{{#display_map: 
 |layer=GeoportailFrance.ign_maps
 |service=leaflet
}}

ignmaps

Copy link
Member

@JeroenDeDauw JeroenDeDauw left a 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',
Copy link
Member

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 :)

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 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.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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.

}
}
}
}
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for this.

@JeroenDeDauw JeroenDeDauw merged commit 5067b73 into ProfessionalWiki:master Jun 11, 2018
@JeroenDeDauw
Copy link
Member

@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.

@gcebelieu
Copy link
Contributor Author

@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.

@JeroenDeDauw
Copy link
Member

Right. Seems like time anyway to update the copy we have here as it is two years old.

@PeterTheOne
Copy link
Contributor

@gcebelieu
Copy link
Contributor Author

Hi @PeterTheOne, @JeroenDeDauw,
It has been merged into leaflet-providers master.

@JeroenDeDauw
Copy link
Member

JeroenDeDauw commented Jun 19, 2018

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.

@gcebelieu
Copy link
Contributor Author

Yes it has just been released :

https://github.com/leaflet-extras/leaflet-providers/releases/tag/1.3.0

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.

3 participants