Skip to content

Conversation

jfirebaugh
Copy link
Member

Two shortcomings of this approach:

  1. Features and FeatureCollections aren't round tripped. L.GeoJSON.geometryToLayer
    discards feature properties and I'm not seeing a clear way to keep those around for
    reserialization.
  2. MultiPoints aren't round tripped. L.GeoJSON.geometryToLayer creates a FeatureGroup
    of Markers when it encounters a MultiPoint, but my implementation of
    FeatureGroup#toGeoJSON always creates a GeometryCollection.

@jfgirard's implementation in #712 contains a heuristic: if converting each layer
in a FeatureGroup to GeoJSON produces only Points, it returns a MultiPoint, otherwise
it returns a GeometryCollection. I don't like this solution, because it results in
the opposite problem: a GeometryCollection containing only Points can't be round tripped.

Instead I think Leaflet should have a dedicated MultiMarker class in the style of
MultiPolyline and MultiPolygon. But implementing that is tricky, because it needs to
support L.GeoJSON's pointToLayer option.

@ghuntley
Copy link

💓

@mourner
Copy link
Member

mourner commented Feb 27, 2013

Thanks John, very nice work! 👍 Let me think about the shortcomings a bit before I merge this...
Regarding MultiPolygon and MultiPolyline, I'm planning to make a different implementation for them in future as a part of vector layers refactoring (started here some time ago) — implement them as one path with multiple M points instead of a FeatureGroup.

@gamb
Copy link

gamb commented Mar 14, 2013

+1

@mourner
Copy link
Member

mourner commented Apr 4, 2013

@jfirebaugh please rebase and update specs to Mocha style.

@jfirebaugh
Copy link
Member Author

Done.

@ghost ghost assigned mourner Apr 19, 2013
@mourner
Copy link
Member

mourner commented Apr 19, 2013

Reviewed this again, good to merge. Regarding Feature/FeatureCollection round-tripping — feature is saved as a feature property in the layer, so it can be used to form a Feature GeoJSON if it's present (extending it with an updated geometry).

mourner added a commit that referenced this pull request Apr 19, 2013
Add #toGeoJSON to various layer types (#712)
@mourner mourner merged commit 412f047 into Leaflet:master Apr 19, 2013
@mourner
Copy link
Member

mourner commented Apr 19, 2013

@jfirebaugh see 7533a46 for initial work on roundtripping features.

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

Successfully merging this pull request may close these issues.

4 participants