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

Do not use retina suffix on some Stamen basemaps #287

Closed

Conversation

IvanSanchez
Copy link

@IvanSanchez IvanSanchez commented Jun 20, 2018

This should fix #283 by explicitly setting the retina suffix to an empty string (instead of using the Leaflet default hard-coded in the L.TileLayer code), but I haven't really tested it.

This is an alternative to #286.

@brunob
Copy link
Member

brunob commented Jun 20, 2018

This bug come form #282 & i'm not sure about the better option to fix it : #286 seems more KISS but it feel strange to have an url for a variant VS #287 seems better but i got a doubt on the retinaSuffix introduction. (imho, #286 is better for its "clarity")

Any thought about this @jieter ?

@jieter
Copy link
Contributor

jieter commented Jun 20, 2018

Hi @IvanSanchez / @JLuc, nice to have two solutions to choose from, thanks!

I think I like #286 better because it is more explicit and thus closer to the purpose of leaflet-providers to just maintain a list of free providers. Being too smart (like we did before with homegrown retina detection) also makes it harder to maintain while the leaflet API evolves.
I think it also provides a clearer layer definition in the preview, which is how I mainly use leaflet-providers.

@brunob
Copy link
Member

brunob commented Jun 20, 2018

Closing this one, anyway thx @IvanSanchez :)

@brunob brunob closed this Jun 20, 2018
@IvanSanchez
Copy link
Author

No problem at all!

I just didn't realize @JLuc did a pull request while I was hacking together my solution :-D

@IvanSanchez IvanSanchez deleted the stamen-no-retina branch June 20, 2018 21:48
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.

Stamen TopOSMRelief broken on Retina
3 participants