leaflet-d3-layer icon indicating copy to clipboard operation
leaflet-d3-layer copied to clipboard

Only draws first feature when GeoJSON features don't have an ID

Open chrisincambo opened this issue 11 years ago • 4 comments

This code only works when the GeoJSON has the id property set:

  join = paths.data(this.geojson.features, function(d) {
    return d.id;
  });

In the GeoJSON spec the id property of a feature is optional. I would have submitted a patch, but I haven't used CoffeeScript before. Here's a fix in JavaScript:

  join = paths.data(this.geojson.features, function(d) {
    return d.id || Math.floor(Math.random() * 1000001);
  });

But for large feature collections that slows down the initial load so this might be better:

  var idSetter = (this.geojson.features[0].id) ? function(d){ return d.id; } : null;
  join = paths.data(this.geojson.features, idSetter);

Cheers,

Chris

chrisincambo avatar Dec 12 '13 08:12 chrisincambo

Thanks Chris! I'll take a closer look.

rclark avatar Dec 18 '13 02:12 rclark

The problem with randoms here is that if you add features subsequently, you'll end up duplicating them. Ideally that ID should be something unique. What do you think about some kind of one-way hash of the stringified GeoJSON feature? Too slow? Probably.

hashCode = function(s){
  return s.split("").reduce(function(a,b){a=((a<<5)-a)+b.charCodeAt(0);return a&a},0);              
}

join = paths.data(this.geojson.features, function (d) {
  return d.id || hashCode(JSON.stringify(d));
});

The hashCode function is from http://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript-jquery

rclark avatar Dec 18 '13 15:12 rclark

I think that would slow things down on large datasets, which given the context is quite likely. It might be better just to make the ID a requirement for the provided GeoJSON, add something to the docs and instead of letting it fail silently throw a meaningful error.

On Wed, Dec 18, 2013 at 10:58 PM, Ryan Clark [email protected]:

The problem with randoms here is that if you add features subsequently, you'll end up duplicating them. Ideally that ID should be something unique. What do you think about some kind of one-way hash of the stringified GeoJSON feature? Too slow? Probably.

hashCode = function(s){ return s.split("").reduce(function(a,b){a=((a<<5)-a)+b.charCodeAt(0);return a&a},0); } join = paths.data(this.geojson.features, function (d) { return d.id || hashCode(JSON.stringify(d));});

The hashCode function is from http://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript-jquery

— Reply to this email directly or view it on GitHubhttps://github.com/rclark/leaflet-d3-layer/issues/1#issuecomment-30853215 .

chrisincambo avatar Dec 19 '13 01:12 chrisincambo

Hi, I don't know if it's related but the options doesn't seems to work at all when using the extending class GeoJSON.d3. This won't work (won't fit the bounds when clicked ) :

d3.json("data/data.json", function(data) {
        function onEachFeature(feature, layer) {
            layer.on({
                click: zoomToFeature
            });
        }

        function zoomToFeature(e) {
            map.fitBounds(e.target.getBounds());
        }

        var layer = new L.GeoJSON.d3(data, {
            onEachFeature: onEachFeature
        });

        map.addLayer(layer);
    });

This will work :

/* blablabla */

        var layer = new L.GeoJSON(data, { // original object
            onEachFeature: onEachFeature
        });

        map.addLayer(layer);
    });

May it be because of the id ? I used this to bypass the "only-first-feature-drawing" issue :

join = paths.data(this.geojson.features, function(d) {
    return d.id || Math.floor(Math.random() * 1000001);
  });

Thanks

Nael

Firnael avatar Apr 08 '14 14:04 Firnael