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

Show people in a region as a bulleted list within the popup #133

Closed
jywarren opened this issue Aug 10, 2021 · 7 comments · Fixed by #136
Closed

Show people in a region as a bulleted list within the popup #133

jywarren opened this issue Aug 10, 2021 · 7 comments · Fixed by #136
Assignees

Comments

@jywarren
Copy link
Member

After fixing #123, it's showing that there are "2 people" but we should be instead showing a list of the people, like:

* firstPerson
* secondPerson

Originally posted by @jywarren in #123 (comment)

@jywarren
Copy link
Member Author

image

@jywarren
Copy link
Member Author

That's odd. There should be a list already:

// generates an unordered list of marker links to fill a popup with
function generatePopupContentsFromMarkers(markers) {
let popupContents = "<p><b>People within this region:</b></p>";
popupContents += "<ul>";
if (markers[0].hasOwnProperty('url') && markers[0].hasOwnProperty('url')) {
markers.forEach(function(marker) {
if (marker.hasOwnProperty('url') && marker.hasOwnProperty('url')) {
popupContents += "<li><a href='" + marker.url + "'>@" + marker.title + "</a></li>";
}
});
} else {
popupContents += "<p>" + markers.length + " people</p>";
}
popupContents += "</ul>";
popupContents += "<p>These people have blurred their location. Learn about <a href='https://publiclab.org/location-privacy'>location privacy here</a>.</p>";
return popupContents;
}

@jywarren
Copy link
Member Author

Aha! I think what happens is that it shows the count if there is no "url" property of the people. But on PublicLab.org, we do pass that. So it should be OK when run over there. I think we just need to release a new version here.

@jywarren
Copy link
Member Author

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

@daemon1024
Copy link
Member

We will need to reopen this.

Ref publiclab/plots2#10033 (comment)

it seems that the parsed_data you mentioned here, is only for temporary use.

var parsed_data = options.JSONparser(data) ; // JSONparser defined by user used here !

we use it here, what we store in the markers object is

var m = L.marker([latitude, longitude], {
icon: icon_color
}) ;
SourceUrl_id_map.set(id , m) ;
all_markers_map.set(id , m) ;

is the leaflet marker object mapped to id. So there's no URL at all and hence no list of people names.

@jywarren jywarren reopened this Aug 17, 2021
@jywarren
Copy link
Member Author

Aha! Ok, excellent debugging! Hopefully we can solve this with a PR here and then through LEL to plots2 again?

Great detective work, @daemon1024!!!

@daemon1024
Copy link
Member

Sure, I am on it. Can be assigned to me :)

daemon1024 added a commit to daemon1024/leaflet-blurred-location-display that referenced this issue Aug 17, 2021
jywarren pushed a commit that referenced this issue Aug 17, 2021
* add package json scripts to ease development

* add url and title to marker object

fixes #133

* document the need of additional fields

* bump to v1.3.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants