Follow-up issues for LBLD docs

jywarren commented 5 years ago

Just collecting some here!!! 👍

Progress to date is really tremendous, @sagarpreet-chadha! You've done an incredible job. I'm now giving it a fresh look, and thinking about what could come next. Would love your thoughts on these!!!

[x] Color code the markers on the map according to the precision. -- actually this was just to demonstrate the principle in the demo; we could keep it as a feature but I had imagined not being a default behavior. What do you think, @sagarpreet-chadha ?
[x] Can we add a couple screenshots from the blog post into the README to illustrate?
[x] Under https://github.com/publiclab/leaflet-blurred-location-display/blob/main/README.md#basic, we should maybe say First, you need to make some blurred locations. Let's create 3 of them and add them to the map. and then show how to create three and add them in as short a code segment as possible. One thing I'm wondering is how the options_display is passed into the constructor and we could show that more clearly in the "Basic" section!
[x] could we have a constructor option to switch between heatmap and marker mode? Like:

blurredLocations = new BlurredLocations({
  style: 'heatmap' // or 'markers' or 'both' ?
});

Possible changes

[x] does options.locations auto-generate the BlurredLocations? Why do we make only one blurredLocation? It kind of seems like we should make one for each coordinate pair, using this syntax?
[x] Actually, perhaps we need to change the syntax of the BlurredLocation constructor so it can be added to a map the same way a standard Leaflet object is added, with `addTo(map) as in these instructions -- sorry, that would be a breaking change, I suppose... but in the spirit of making it a 'normal' Leaflet object?
[x] locations_markers_array - maybe we should rename this getVisibleLocations()?
[x] SourceUrl_markers_array is this the same as above but for remote locations? What if we instead just added a property to each location like source: 'remote' and then only used the above getVisibleLocations(); method instead of having 2?
[x] could getMarkersOfPrecision likewise return just 1 array?
[x] could we illustrate precision by referencing the chart in https://github.com/publiclab/leaflet-blurred-location#human-readable-blurring ?
[x] let's open an issue to add the ability to pass an array of precisions or a range (not sure how, maybe { min: 0, max: 11 } into getMarkersOfPrecision() -- we don't have to implement it, but it'd be a nice next step later.

OK, this is a lot! But it's so exciting to have come this far!!!

jywarren commented 5 years ago

Follow-up from #17 !!!

sagarpreet-chadha commented 5 years ago

Adding these here :

[x] one was, to try to ensure that pages tagged with place get a full-width map across the top of the page (instead of in the sidebar), as we're trying to do in this page (but truly edge-to-edge and right up against the top menubar): https://publiclab.org/wiki/puerto-rico (This could feature LEL as well.) [https://github.com/publiclab/plots2/pull/5518]
[x] A second one is - tell me what you think, but the idea of "recently used locations" as a way to speed up the Leaflet Blurred Location modal; we could, on plots2, look up recently added location tags and offer them as a way to quickly add location. [https://github.com/publiclab/plots2/pull/5522]
[x] We might also use the most recent location as the default location that appears when you open the LBL modal, so it sort of assumes "near the last place you input". [https://github.com/publiclab/plots2/pull/5522]
[x] Finally, I wanted to try to get the human-readable placename that's shown to become a tag in the format: place:new-york-city (for example). [https://github.com/publiclab/plots2/pull/5518]

sagarpreet-chadha commented 5 years ago

Okay so all the above issues are solved in these PR's 😄 :

⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️ ⭐️

jywarren commented 5 years ago

Awesome, let's close this then!

publiclab / leaflet-blurred-location-display

Follow-up issues for LBLD docs #64

Possible changes