Add Retina Display Support (issue #206)

[BibMartin]

I have no retina display ; so I have no possibility to see how Leaflet reacts.
Maybe @francisbautista can confirm that this works.

[andrewgiessel]

I can try it too

On Oct 30, 2015, at 08:24, Martin Journois notifications@github.com wrote:

I have no retina display ; so I have no possibility to see how Leaflet reacts.
Maybe @francisbautista can confirm that this works.

You can view, comment on, or merge this pull request online at:

#222

Commit Summary

Fix issue #206
File Changes

M folium/map.py (18)
Patch Links:

https://github.com/python-visualization/folium/pull/222.patch
https://github.com/python-visualization/folium/pull/222.diff
—
Reply to this email directly or view it on GitHub.

[ocefpaf]

@andrewgiessel can you review and merge this one too?

[andrewgiessel]

Ya let me try tonight!
On Fri, Oct 30, 2015 at 08:44 Filipe notifications@github.com wrote:

@andrewgiessel https://github.com/andrewgiessel can you review and
merge this one too?

—
Reply to this email directly or view it on GitHub
#222 (comment)
.

[andrewgiessel]

Ok, this looks good to go! I verified this 2 different ways:

1. on my laptop and on an external screen, I loaded a test notebook and refreshed the page on both screens. I could see the size of the tiles were smaller on my laptop than on the external screen.

2. This is kinda cool- you can use chrome's developer tools to go into 'device' mode, where you can simulate changing the resolution, size and network speed (!) the device that loads the page. Here are two screen shots of doing this with high and low density screen (and low speed to aid in catching the tiles loading)

So, I think this is good. Can I merge? It's one commit- do I need to clean it up at all?

[ocefpaf]

So, I think this is good. Can I merge?

Merge away!

Thanks @BibMartin for the implementation and @andrewgiessel for the review.

[themiurgo]

Do you guys know why Leaflet defaults detect_retina to false? It shouldn't have any drawbacks so I would be tempted to default it to True on folium.

[andrewgiessel]

My guess is bandwidth. Downloading 2x the tiles you can see

On Fri, Nov 13, 2015 at 08:00 Antonio Lima notifications@github.com wrote:

Do you guys know why Leaflet defaults detect_retina to false? It
shouldn't have any drawbacks so I would be tempted to default it to True on
folium.

—
Reply to this email directly or view it on GitHub
#222 (comment)
.

[themiurgo]

Mapbox-powered maps (FourSquare & co.) and GMaps default to True, despite bandwidth. 😀

[ocefpaf]

I guess that there are other reasons:

Leaflet/Leaflet#1985

[themiurgo]

After all, it seems it's because of this: "detectRetina is optional (false by default) to prevent map features (like city titles) from being too small to read on retinal tiles" (source: Leaflet/Leaflet@443181a). I don't think we care about that, should we turn it on by default?

[ocefpaf]

How about the performance issue I mentioned above?

Ironically it happens only on the IPad using Safari:

I test it too on Android mobile with detectRetina: true, and no problem. Is only for Safari on iOs.

[themiurgo]

Sorry, I thought it was solved because I saw "closed". I'll test it and see if still applies, that's an issue from 2013 so it might have been solved from iOS side.

[ocefpaf]

I guess it is worth asking that question in the leaflet issue page.

Leaflet/Leaflet#4014

[themiurgo]

On my iPhone there were no performance issues. But yes, the text labels are incredibly small and unreadable, I think that is the reason behind that. Let's see what they say. 👍

[ocefpaf]

We have to bare in mind that setting false as the default we ensure the results will be the same in any platform, retina or no. By forcing the user to set it to true the user acknowledges that he is targeting platforms where that is desired.

[ocefpaf]

We have an official answer:

Leaflet/Leaflet#4014 (comment)

[themiurgo]

Cool, let's leave it as it is. 👍

[ocefpaf]

We should improve the docs though. Highlighting the option and say why false is the default.