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

Idea: Replace Leaflet map with Mapbox #43

Closed
ilyabo opened this issue Apr 10, 2022 · 6 comments
Closed

Idea: Replace Leaflet map with Mapbox #43

ilyabo opened this issue Apr 10, 2022 · 6 comments

Comments

@ilyabo
Copy link
Contributor

ilyabo commented Apr 10, 2022

Another idea:

I tried to throw the data onto a Mapbox map and enabled clustering. The interaction feels smoother: https://ukraine-civilian-harm.vercel.app/

What do you think? Would you consider replacing the Leaflet map in the app?

image

@fspoettel
Copy link
Contributor

fspoettel commented Apr 10, 2022

(This might be an interesting discussion for the upstream repository as well.)

This is a neat demo, thank you for putting it together and sharing it! MapboxGL does appear to render smoother, especially when zooming.

Some questions:

  • one nice feature of timemap is the filter system that produces clusters colored as radial pie charts which our map has as well. Is MapboxGL flexible enough to allow sth. like that?
  • MapboxGL recently switched to a non-OSS license which might be an issue. Is there a OSS fork that can be used?

There are a few decisions I really like in your implementation that we should consider for this project in any case:

  1. containing map moves to a bounding box around ukraine makes a lot of sense.
  2. using satellite-streets layer istd. of just satellite for the satellite view.
  3. the lighter blue works very well on the satellite layer, imo a lot better than the purple we are currently using.
  4. having less text on higher zoom levels works quite well. I wonder if we could do the same for our default map style.

@ilyabo
Copy link
Contributor Author

ilyabo commented Apr 10, 2022

one nice feature of timemap is the filter system that produces clusters colored as radial pie charts which our map has as well. Is MapboxGL flexible enough to allow sth. like that?

Aha, I hadn't realized that after setting the filters one can see the pie charts before you mentioned it. It's a bit of a hidden feature :)

Rendering pie charts isn't straightforward with Mapbox, but it's possible to do it in an SVG overlay.

MapboxGL recently switched to a non-OSS license which might be an issue. Is there a OSS fork that can be used?

Yes, there's MapLibre GL JS which is behind the latest Mapbox developments but should also work in principle. But can you elaborate on why is the licensing problematic?

  1. the lighter blue works very well on the satellite layer, imo a lot better than the purple we are currently using.

I also set the opacity of the satellite raster layer to 0.5 so that the event locations are more salient.

@fspoettel
Copy link
Contributor

But can you elaborate on why is the licensing problematic?

It's admittedly more of an issue for the upstream project that this timemap is forked from as we already use mapbox tiles here.

The new license forces users to have a mapbox account and thereby adhere to their ToS before the library can be used to embed a map. This locks the OSS project pretty firmly to mapbox and might prevent some people from using it in the future if they cannot comply with the tos for w/e reason.

@ilyabo
Copy link
Contributor Author

ilyabo commented Apr 11, 2022

Okay, I will try then to see how it works with MapLibre

@ilyabo
Copy link
Contributor Author

ilyabo commented Apr 14, 2022

I tried. It works with MapLibre too:
https://ukraine-civilian-harm-hyw9qjpub-ilyabo.vercel.app/

image

The only issue is that it doesn't seem to support the new mapbox styles, so I had to resort to MapTiler. Satellite resolution there is not as good as with MapBox even for Kiev:

image

@msramalho
Copy link
Contributor

These are not pressing changes and we'd like to keep leaflet for now, not sure if @breezykermo sees it as a good addition to the upstream though.

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

No branches or pull requests

3 participants