WeAreFairphone / fprsmap

This is a Leaflet map of the local Fairphoners communities
https://map.fairphone.community
GNU General Public License v3.0
10 stars 7 forks source link

Remove markers clustering #63

Closed Roboe closed 5 years ago

Roboe commented 7 years ago

This reverts commit 1dc401bddbb47cbeea77b8a6b4e4bb213f02a0c3.

Reasons, as we (@StefanBrand and I) chatted:

Preview: https://roboe.github.io/fprsmap

StefanBrand commented 7 years ago

Two questions:

This fixes #55.

Roboe commented 7 years ago

Do you think that performance gets worse with this (especially on FP1)?

This should avoid a lot of computations from the clustering addon, doesn't it? I don't have a FP1, but my Moto E (LineageOS 14.1, very similar hardware) loads it just fine.

StefanBrand commented 7 years ago

Not the loading, but the panning. On a FP2 it also feels kinda laggy. I think because of the high number of markers that's displayed at once.

Maybe we can uncluster the markers only at a specific zoom level? -- This message was sent from my Fairphone 1 with K-9 Mail.

Am 27. Mai 2017 15:41:55 MESZ schrieb "Roberto M.F." notifications@github.com:

Do you think that performance gets worse with this (especially on FP1)?

This should avoid a lot of computations from the clustering addon, doesn't it? I don't have a FP1, but my Moto E (LineageOS 14.1, very similar hardware) loads it just fine.

Roboe commented 7 years ago

Let's see it canvas rendering solves the performance fall caused by the bunch of markers

StefanBrand commented 7 years ago

It might be a bit better. Zooming still lags though... -- This message was sent from my Fairphone 1 with K-9 Mail.

Am 28. Mai 2017 01:55:42 MESZ schrieb "Roberto M.F." notifications@github.com:

Let's see it canvas rendering solves the performance fall caused by the bunch of markers

Roboe commented 7 years ago

No, you are right, clustering improves performance. See this: https://github.com/Leaflet/Leaflet.markercluster/blob/master/README.md#handling-lots-of-markers

We can leave clustering enabled, then, but it is still causing confusion —e.j. grouping markers on Spain with those on France, and falsely signaling there are no markers in Spain. How can we fix this?

StefanBrand commented 7 years ago

Leaflet/Leaflet.markercluster#749 and Leaflet/Leaflet.markercluster#521 give interesting ideas on improving this situation.

I thought about a different approach: Don't do clustering, but load country polygons into the map (possibly with a display of the number of markers in that country). The idea would be that the markers don't appear at once, but only after you click on the country or zoom in.

Edit: The more I think about it, the less I like the idea of clustering per country. After all, I'm in favour of a borderless Europe... 😛

Roboe commented 7 years ago

Yeah, I don't like borders neither, but countries are the geographic organizations we have now.

Other solution may be placing the user at first in its aprox location (zoomed in), which feels creepy, IMO, and it's a privacy regression.

StefanBrand commented 7 years ago

According to the Leaflet docs, canvas rendering is on by default when the browser supports it.

Unfortunately I cannot check on Firefox for Android, but on desktop it can't have changed anything since canvas is supported by Firefox. (Though I cannot find a <canvas>-element in the webpage's source code...)

PS.: preferCanvas 🔗 only affects Paths 🔗

Edit: I disabled the marker zoom animation, which works well with the zoom buttons and improves performance during zooming (both on desktop and mobile).

However, there are problems with pinch-zooming, possibly related to zoomSnap 🔗. (I'm starting to wonder whether they might be related to my FP1 too...) @Roboe Does pinch-zooming feel the same like when touching the zoom buttons on your FP2?

Edit2: I think we can revert ac2afd9.