esri / leaflet.shapemarkers Goto Github PK

talkin bout a CHANGELOG.MD!

@ngoldman

Gday @jgravois

I did just find one minor which effects both of our plugins. If I do something like this in leaflet 1.0

var myLayer = L.geoJson(data, {
    pointToLayer: function (feature, latlng) {
    return L.shapeMarkers.xMarker(latlng, 50)
      })

myLayer.toGeoJSON()

my toGeoJSON comes back as empty.... I think it relates to roughly these lines in the L.GeoJSON , it's a bit of an edge case but it's effected my app, if I work out a good way to fix it I'll let you know!

to match the casing convention of other Leaflet plugins here

Gday @jgravois

So it turns out we've made almost identical leaflet addins, whoops, not enough googling on my part (although perhaps you should add your addin to the leaflet addin page)!
https://github.com/rowanwins/Leaflet.SvgShapeMarkers

I'm all for merging (slash decommissioning mine) the addins given that I think we're largely trying to solve the same problem in pretty much the same way.

One significant difference worth discussing/thinking about is that rather than creating a new class for every marker type, I instead had an option specifying the type (eg shape: "square"). The reason I went this approach was when I was thinking about how I'd use it I was thinking of a large point dataset loaded as geojson and doing pointToLayer, it seemed a bit more logical to have everything as one

    pointToLayer: function (feature, latlng) {
      return L.shapeMarker(latlng, {
        shape: getShape(feature.properties.myCategory),
        color: 'white'
      })

with your're current approach it would be something more like

    pointToLayer: function (feature, latlng) {
      if (feature.properties.myCategory == "whatever"){
           return L.shapeMarkers.xMarker(latlng, {
               shape: getShape(feature.properties.myCategory),
               color: 'white'
            })
      }
      if (feature.properties.myCategory == "somethingElse"){
            return L.shapeMarkers.squareMarker(latlng, {
                 shape: getShape(feature.properties.myCategory),
                 color: 'white'
            })
     }
}

Obviously you could restructure my example above of your's abit so it's all a bit tidier etc so I guess I'm neither here nor there.

Generally speaking I think your plugin looks a bit more robust, I hadn't thought about canvas at all, it was just the result of a couple of hours of effort yesterday.

Anyway just thought I'd raise the flag
Cheers
Rowan

The marker tests are still in https://github.com/Esri/esri-leaflet-renderers, they should be ported over to here.

While using this package with preferCanvas={true}, I am getting error as "layer._containsPoint is not a function" .
TypeError: layer._containsPoint is not a function
NewClass._handleMouseHover
/src/layer/vector/Canvas.js:391
388 |
389 | for (var order = this._drawFirst; order; order = order.next) {
390 | layer = order.layer;

391 | if (layer.options.interactive && layer._containsPoint(point)) {
| ^ 392 | candidateHoveredLayer = layer;
393 | }
394 | }