Esri / esri-leaflet-vector

Display ArcGIS Online vector basemaps w/ Esri Leaflet
Apache License 2.0
56 stars 57 forks source link

Raster basemap support #26

Closed shadc closed 7 years ago

shadc commented 7 years ago

Added support for hybrid basemaps (#23 and #13). Not sure it is implemented the way you would prefer? I added a handful of raster services from Esri, Mapbox, Stamen, OSM, and OCM. Not sure of any licensing issues including non-Esri services?

jgravois commented 7 years ago

i really appreciate you tackling this!

I added a handful of raster services from Esri, Mapbox, Stamen, OSM, and OCM

I don't have any problem conceptually with including non-Esri services as options, but

  1. we'll need to handle including the appropriate attribution for services automatically. its not sufficient for the plugin to expect folks to take care of it on their own.
  2. its not appropriate for us to hardcode a mapbox access_token. this is something that'd either need to be exposed as a configurable option or something that we just ensure can be achieved manually (and perhaps demonstrate in a sample).
  3. i was only planning on adding raster tilesets for aerial photography (at least initially). what is the use case for mixing and matching rasterized vector basemaps and vector tile basemaps together?
shadc commented 7 years ago

we'll need to handle including the appropriate attribution for services automatically. its not sufficient for the plugin to expect folks to take care of it on their own.

Ok, I can add this.

its not appropriate for us to hardcode a mapbox access_token. this is something that'd either need to be exposed as a configurable option or something that we just ensure can be achieved manually (and perhaps demonstrate in a sample).

Yea, I figured that was probably not a good ideal. I'm just a superfan of their basemaps! Will remove.

i was only planning on adding raster tilesets for aerial photography (at least initially). what is the use case for mixing and matching rasterized vector basemaps and vector tile basemaps together?

Good point. I can't really think of use case... might as well use the vector basemaps. Probably a good start is to just include raster tile services not derived from vectors.

shadc commented 7 years ago

Would it be better to just pass in the raster basemap information?? Something like this:

    var myRasterBasemap = {
        url: 'https://{subDomain}.arcgisonline.com/arcgis/rest/services/World_Imagery/MapServer/tile/{z}/{y}/{x}',
        subdomains: ['services', 'server'],
        copyright: 'Source: Esri, DigitalGlobe, GeoEye, Earthstar Geographics, CNES/Airbus DS, USDA, USGS, AeroGRID, IGN, and the GIS User Community'
    };

    L.esri.Vector.layer({id:'8c5c4728e14b4dae836ee4ea42b8c086',
        rasterBasemap: myRasterBasemap
    }).addTo(map); //Ref Map

Or better to abstract this away for users and just pass in a basemap name?

jgravois commented 7 years ago

my instinct is that we should:

  1. provide a dead simple option to mix and match Esri aerial imagery.
  2. provide a generic hook so that developers can customize the style.json as deeply as they'd like.

we could demonstrate how to include Mapbox aerial photography (w/ token) to document the second improvement. how does that sound?

shadc commented 7 years ago

Sounds great. Closing this pull request and will request a new one (if needed) when I have time to work on it. Still pretty busy digging out of the snow!