Esri / esri-leaflet-cluster

Plugin for Esri Leaflet that clusters a Feature Layer using Leaflet.markercluster
http://esri.github.io/esri-leaflet/
Apache License 2.0
23 stars 24 forks source link

style method is not called for polygons and linear features #23

Open nothingisnecessary opened 5 years ago

nothingisnecessary commented 5 years ago

Reference doc shows a style method which is similar to what vanilla Leaflet uses for styling vector features: https://esri.github.io/esri-leaflet/api-reference/layers/cluster-feature-layer.html

The style method works fine with L.esri.featureLayer, but I do not see it ever getting called when using a polygon or linear feature service. Is there some trick to get it to work - or some known issues? I couldn't find any code examples of it working with the Esri cluster plugin.

And yes - I know it doesn't really make sense to cluster vectors, but I have a generic utility that is not aware of the geometry type of the feature service... though if there is a way to auto detect that I could use L.esri.featureLayer instead, I suppose... but since the documentation says it works, and since I do see implementations of options.style and setStyle / resetStyle in the source, I figured I would try to use it.

Sample code:

var bikePaths = L.esri.Cluster.featureLayer({
    url: 'https://services.arcgis.com/uCXeTVveQzP4IIcx/ArcGIS/rest/services/Bike_Routes/FeatureServer/0',
    onEachFeature: function(feature, layer)
    {
        // this works - called for each feature
    },
    pointToLayer: function (feature, latlng)
    {
        // this works - called for each point feature
    },
    style: function (feature, layer)
    {
        // never called - despite having linear and polygon features
        // the linear and polygon features do show up fine.
        // they don't cluster, but that's to be expected (they are vectors)
        // wth documentation?
    },
  }).addTo(map)
nothingisnecessary commented 5 years ago

I debugged a bit - seems to be the problem is that features are loaded asynchronously and so the initial call to options.style never happens because the layer is empty.

My workaround hack was to simply call layer.resetStyle from within onEachFeature for vector features.

...
onEachFeature: function(feature, layer)
{
    if (feature.geometry.type !== "Point" && feature.geometry.type !== "MultiPoint")
    {
        // explicit call to resetStyle to force initial default custom style
        bikePaths.resetStyle(feature.id);
    }
}
...
nothingisnecessary commented 5 years ago

Also (originally) posted on GIS stackexchange

https://gis.stackexchange.com/questions/328171/l-esri-cluster-featurelayer-does-not-call-style-function

jgravois commented 5 years ago

I have a generic utility that is not aware of the geometry type of the feature service... though if there is a way to auto detect that I could use L.esri.featureLayer instead, I suppose.

something like this would work:

const url = 'https://services.arcgis.com/P3ePLMYs2RVChkJx/arcgis/rest/services/World_Time_Zones/FeatureServer/0'

L.esri.request(url, {}, function (err, response) {
  if (response.geometryType === "esriGeometryPoint") {
    L.esri.Cluster.featureLayer({ url }).addTo(map)
  } else {
    L.esri.featureLayer({ url }).addTo(map);
  }
})

The style method works fine with L.esri.featureLayer, but I do not see it ever getting called when using a polygon or linear feature service.

Internally we're calling Leaflet's own L.GeoJSON.geometryToLayer, and that method is ignoring style.

L.GeoJSON.geometryToLayer({
  "type": "LineString",
  "coordinates": [
    [20.0, 0.0],
    [100.0, 45.0]
  ]
}, {
  style: function (feature) {
    return { color: 'red' }
  }
}).addTo(map);

I think that's a bug, but I'm also happy to just remove style from our own documentation.

nothingisnecessary commented 5 years ago

Thanks for clarifying - style does work, it turns out, but there were no features to style when it was initially invoked.

So my solution was to call resetStyle on the feature's ID when it loads and that makes it style correctly. This does require defining the style method on the cluster layer's options (see original question for example.. though of course with returning the actual style object).

For my first attempt I was using setStyle but that styles the whole layer, which seemed wasteful & redundant, especially when there are lots of features that come into view as you pan & zoom the map. After profiling a bit with resetStyle in onAddFeature I found the performance to be acceptable, and it certainly achieves the desired result.

I propose to maybe just update the documentation to note that style may not be used until after the features load. It is useful! Users like the pretty colors ;)

I could push a proposed docs update, but I didn't find the docs in the repo; maybe I'm just looking in wrong place? Seems to be maintained on Esri's website. Anyway - thanks for a cool library!

Update: the solution for checking the feature service layer's geometry type also works and is viable. I am not very familiar with Esri's nuances. I didn't realize that ArcGIS tends to have (or can only support?) just one geometry per layer. I've worked with other geodatabases that separate feature layers into classes based on business data & purpose rather than by geometry type. For example, when working with bridges, some are linear features / linear referenced, and some are point features. (Don't blame the messenger here; that may just be my clients being weird..) I spoke to a colleague who works with ArcGIS and he also showed me (1) a way to interrogate feature service layer for geometry type, and (2) that it was not a convention for ArcGIS to use different geometries in a single layer. (Though I'm still not clear on whether this applies to mixing Polygons and MultiPolgons in a single layer.. but moot point because they're both vector types in Leaflet.)

Should I close this issue? Standing by on decision about revising the docs.. thanks.

jgravois commented 5 years ago

I could push a proposed docs update, but I didn't find the docs in the repo

you can find it here: https://github.com/Esri/esri-leaflet-doc/blob/master/src/pages/api-reference/layers/cluster-feature-layer.md

I didn't realize that ArcGIS tends to have (or can only support?) just one geometry per layer.

That's correct. Polygons and MultiPolygons can live side by side in a single layer, but LineStrings and Polygons can not.

jgravois commented 5 years ago

cc/ @patrickarlt for visibility, since he isn't listed as watching this repo.

jwasilgeo commented 3 years ago

@nothingisnecessary regarding

Should I close this issue? Standing by on decision about revising the docs.. thanks.

I leave it up to you. We welcome pull requests for documentation changes but we can also close out this issue if you wish.