Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leaflet.draw generated a geojson which is not valid #731

Open
fab-girard opened this issue Jun 2, 2017 · 6 comments
Open

Leaflet.draw generated a geojson which is not valid #731

fab-girard opened this issue Jun 2, 2017 · 6 comments

Comments

@fab-girard
Copy link

Hello,
I'm reporting a bug but I'm not sure it's a bug in leaflet.draw or leaflet.

How to reproduce

  • Leaflet version I'm using: 1.0.3
  • Leaflet Draw version I'm using: 0.4.9
  • Scenario:
    . create or update a polygon with leaflet.draw
    . use .toGeoJSON() on the feature created/updated
    => the json hasn't the "type": "Feature" key/value. Geojson is incorrect.

What behaviour I'm expecting and which behaviour I'm seeing

=> geojson should be valid with type key

Minimal example reproducing the issue

I've created jsfiddle to reproduce the problem (geojson is displayed under the map when you create/update the draw).

Thanks,
Fabien

@ddproxy
Copy link
Member

ddproxy commented Jun 2, 2017

So

  1. The type: is NOT feature and it should be?
  2. They type: IS feature and it should not be?

@fab-girard
Copy link
Author

Mmmh strange.
On my app, geojson hasn't "type": "Feature" when I edit a just created draw. Last week I thought jsfiddle demo hadn't neither that part in geojson but it seems it is present when I check today...
I need a couple of days to understand what has changed.

Fab

@fab-girard
Copy link
Author

fab-girard commented Jun 5, 2017

Ok got it...
On my app I use in addition leaflet-pip which uses a function:

function isPoly(l) {
    return l.feature &&
        l.feature.geometry &&
        l.feature.geometry.type &&
        ['Polygon', 'MultiPolygon'].indexOf(l.feature.geometry.type) !== -1;
}

This function doesn't work with a layer (polygon) drawed by leaflet-draw because there is no feature.geometry.type. To fix it I've added feature.geometry.type = 'Feature' to layer created by leaflet-draw in my code. But when I add this attribute manualy, it seems to influence on update command of draw (and geojson hasn't all required attribute as explained previously).

The original issue is in fact: no feature.geometry.type on layer object generated by leaflet-draw. I don't know if it is a defect from leaflet-draw or it's a correct behavior. Maybe isPoly function in leaflet-pip could be more permissive. I found the last update of this function and it seems it has been simplified for support only Leaflet 1.0:
mapbox/leaflet-pip@a66bb22#diff-168726dbe96b3ce427e7fedce31bb0bcL5

Have you an idea of the change maybe between features/geojson management in leaflet <1.0 and >=1.0 ?

Fab

@ddproxy
Copy link
Member

ddproxy commented Jun 6, 2017

Hey @fab-girard,

A Feature geometry does seem to have a geometry.type when using toGeoJson,

{
   "type":"Feature",
   "properties":{

   },
   "geometry":{
      "type":"Polygon",
      "coordinates":[
         [
            [
               -2.5488281250000004,
               45.49094569262732
            ],
            [
               -2.5488281250000004,
               44.43377984606825
            ],
            [
               -1.2744140625000002,
               44.43377984606825
            ],
            [
               -1.3183593750000002,
               45.49094569262732
            ],
            [
               -2.5488281250000004,
               45.49094569262732
            ]
         ]
      ]
   }
}

I'm curious if you are losing the type somewhere between toGeoJSON and your isPoly call? Since I don't have more information I can't really tell?

@fab-girard
Copy link
Author

fab-girard commented Jun 7, 2017

To begin: thanks for your support.
As I've tried to explain I was loosing the type: Feature in geoJson because of a line I've added after a draw is created.
I've updated the jsfiddle to show you the problem: http://jsfiddle.net/kveke1s5/15/ (line 38 has been added : this line allows to return true in isPoly function)
I thought this line could help me to use leaflet-pip which uses (and declare) the isPoly function. If you run the demo you will see with this added line, the geoJson is not valid (type: Feature is missing).
I think now it was not the good way to fix my problem to use leaflet-draw and leaflet-pip plugins together.

Now I'm thinking the isPoly function could be more permissive. Maybe it could be:

function isPoly(l) {
    if (L.MultiPolygon) {
        return l instanceof L.MultiPolygon || l instanceof L.Polygon;
    }
    if (l instanceof L.Polygon) return true; // THIS LINE IS NECESSARY TO WORK WITH leaflet-draw
    // Leaflet >= 1.0
    return l.feature.geometry && l.feature.geometry.type &&
         ['Polygon', 'MultiPolygon'].indexOf(l.feature.geometry.type) !== -1;
}

But maybe I'm wrong another time... Maybe the layer created by leaflet-draw is not correct and should return true for l.feature.geometry && l.feature.geometry.type && ['Polygon', 'MultiPolygon'].indexOf(l.feature.geometry.type) !== -1; which is not the case at this moment...
Thanks again.

@ddproxy
Copy link
Member

ddproxy commented Jul 5, 2017

@fab-girard Try looking at the feature group you are adding to instead...

  div.appendChild(document.createTextNode('Created: ' + JSON.stringify(featureGroup.toGeoJSON())));
  document.getElementById('geojson').appendChild(div);

Rather than referencing the event, this featureGroup returns a FeatureCollection with a different structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants