publiclab / leaflet-blurred-location-display

A library to display points which have been "blurred" for privacy with leaflet-blurred-location
https://publiclab.github.io/leaflet-blurred-location-display/examples/index.html
GNU General Public License v3.0
11 stars 12 forks source link

Only add popup if rect contains markers #123

Closed jywarren closed 3 years ago

jywarren commented 3 years ago

We may need to check if we need a popup to exist upon modifying the markers on the map with subsequent data loads; should we instead make an empty popup? Or, does it later detect that no popup exists and creates one, or does it flush all popups and regenerate them on new data being loaded?

jywarren commented 3 years ago

Also I'm not seeing the people getting displayed. We should write a test for this:

image

(took some zooming in and out at https://stable.publiclab.org/map#9/41.75603585779898/-71.4263141155243)

Screen Shot 2021-02-23 at 3 26 38 PM

daemon1024 commented 3 years ago

@jywarren Can you explain what is it that we intend to do here? What's the blocker here?

daemon1024 commented 3 years ago

https://user-images.githubusercontent.com/47106543/128615069-1ec77818-35a1-4dac-8316-27f8da4a31c1.mp4

I tried it out after a popup fix https://github.com/daemon1024/leaflet-blurred-location-display/commit/6564091799a0b05a7877ded20a63433c0fd748f2 seemed to work fine.

jywarren commented 3 years ago

I think this is good, we may just be waiting for the GitHub Actions work by @aliciapaz in https://github.com/publiclab/plots2/issues/9641 to get to this repository before merging.

I do notice that in the fix you cite, it's showing that there are "2 people" but we should be instead showing a list of the people, like:

* firstPerson
* secondPerson
jywarren commented 3 years ago

Aha, ok your fix looks great! So we can probably merge this already, esp since you've manually tested. Then let's address the list of users next in a separate issue/PR.

jywarren commented 3 years ago

OK, i brought it in with a PR. Now merging this. Thank you!!!

jywarren commented 3 years ago

Published as v1.3.1! https://www.npmjs.com/package/leaflet.blurred-location-display

jywarren commented 3 years ago

OK this should now show up on plots2 -- let's link when it does.

jywarren commented 3 years ago

https://github.com/publiclab/leaflet-environmental-layers/pull/536