Skip to content

add leaflet to recommended libraries #285

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

Merged
merged 1 commit into from
May 9, 2022
Merged

add leaflet to recommended libraries #285

merged 1 commit into from
May 9, 2022

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented May 4, 2022

Adds Leaflet to the list of recommended libraries (so that it’s easier for us to make a snippet for creating Leaflet maps).

@mbostock mbostock requested a review from shancarter May 4, 2022 18:26
@mbostock mbostock force-pushed the mbostock/leaflet branch from 9889bfa to fe0141f Compare May 4, 2022 18:26
@shancarter
Copy link

The default L construction feels a little unfortunate because it's so arcane in the context of a snippet in observable. It doesn't immediately identify itself as using leaflet, which is especially obscure for these magic std lib cars. Should we use Leaflet instead (like Plot) even though it's not the default global for leaflet?

@shancarter
Copy link

I think this is a very useful snippet though, it seems like something I see people trying and struggling with quite a bit.

@mbostock
Copy link
Member Author

mbostock commented May 4, 2022

I don’t like the L either, but given that it’s the Leaflet convention (and Leaflet defines a global L which is used by some plugins), I think we should stick with that. In the autocomplete menu we can say “L [Leaflet]” like we do with Apache Arrow and Arquero.

I think this is a very useful snippet though, it seems like something I see people trying and struggling with quite a bit.

Yes, people may struggle, but this is as simple as I can make it and it should work out of the box, which I think will encourage users to learn more and experience some success. Just think about how hard it is to do it right now without this snippet and without it being built-in to the standard library! 😄

@mbostock
Copy link
Member Author

mbostock commented May 4, 2022

Specifically I want to use L because if you look on the web for snippets of code using Leaflet, they will all use L, so users won’t have to translate the L into Leaflet to get them to work on Observable. E.g., https://leafletjs.com/examples/quick-start/

@shancarter
Copy link

Sounds good. Having the description in the autocomplete will definitely help. Wonder if we could have the description in the title hover tag or whatever when we do them for standard lib objects?

Copy link

@shancarter shancarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mbostock mbostock force-pushed the mbostock/leaflet branch from fe0141f to 22b497d Compare May 9, 2022 19:45
@mbostock mbostock merged commit 7a74ce9 into main May 9, 2022
@mbostock mbostock deleted the mbostock/leaflet branch May 9, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants