crazycapivara / mapboxer

An R Interface to Mapbox GL JS
https://crazycapivara.github.io/mapboxer/
Other
55 stars 7 forks source link

Discussion: support for Mapbox GL JS v2 #103

Open walkerke opened 2 years ago

walkerke commented 2 years ago

Hi @crazycapivara -

First of all - thanks for putting this package together. Mapbox GL JS is such a large library to interface with as an htmlwidget, I appreciate the work you've done!

I'm bringing my discussion in https://github.com/crazycapivara/mapboxer/pull/101 over here. Mapbox GL JS v2 has a number of features that I think would be of interest to users, including Globe view and non-Mercator projections. You've designed the package API to accommodate those features automatically which is excellent.

The hang-up may be that Mapbox GL JS is now commercially licensed, locking users into their payment structure (see https://github.com/mapbox/mapbox-gl-js/issues/10162). I'm a Mapbox customer so that doesn't bother me but I know it may be a bigger issue for some users.

I've forked mapboxer and written a small implementation that gives users the option to use v1 (which remains the default) or v2, and bundles both libraries. It works in a similar way to sf::sf_use_s2(). For example, by default users requesting the globe view will get the regular Mercator projection, because they are using v1:

library(mapboxer)

mapboxer(
  center = c(-73.9165, 40.7114),
  style = basemaps$Mapbox$streets_v11,
  zoom = 2,
  projection = "globe"
)

image

A mapboxer_use_v2() function in my fork switches the GL JS version to v2, enabling the globe feature:

mapboxer_use_v2(TRUE)

mapboxer(
  center = c(-73.9165, 40.7114),
  style = basemaps$Mapbox$streets_v11,
  zoom = 2,
  projection = "globe"
)
Using Mapbox GL JS v2.
Please review the Mapbox terms of service for version 2
at https://www.mapbox.com/legal/tos/

image

Would you be interested in a PR to support this? I'd understand if supporting two versions is too confusing for users (and I'm fine using my fork in my own projects). However, I think users might really like this feature.

gregleleu commented 2 years ago

Hi @walkerke I started looking at this after your comment on my PR. I think your implementation of the switch makes sense. Two things:

I'll downgrade to the last 1.x.x in my fork

crazycapivara commented 2 years ago

Hi @walkerke yes, that's why I pointed out in the README that mapboxer provides bindings to v.1.x.x of Mapbox GL JS. But I like the idea to give users the chance to use v2 with a function like use_v2. I would not like to bundle the js file with {mapboxer}. One option would be to load it from CDN as @gregleleu suggested and another option could be a function to download the library if wanted. This could be interesting for users deploying apps in air gaped systems who need it locally.

walkerke commented 2 years ago

@crazycapivara sounds good. I'll prepare a PR that implements your suggestions and you can take a look.