After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 757143 - Support for MultiPoint in GeoJSON
Support for MultiPoint in GeoJSON
Status: RESOLVED FIXED
Product: gnome-maps
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gnome-maps-maint
gnome-maps-maint
Depends on:
Blocks:
 
 
Reported: 2015-10-26 15:59 UTC by Jonas Danielsson
Modified: 2015-12-02 09:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add support for MultiPoint in GeoJSON (2.73 KB, patch)
2015-11-29 18:15 UTC, Alaf
none Details | Review
Add support for MultiPoint in GeoJSON (753 bytes, patch)
2015-11-29 18:19 UTC, Alaf
none Details | Review
Add support for MultiPoint in GeoJSON (2.74 KB, patch)
2015-11-29 18:27 UTC, Alaf
committed Details | Review

Description Jonas Danielsson 2015-10-26 15:59:42 UTC
Right now we do not handle the MultiPoint feature of GeoJSON: http://geojson.org/geojson-spec.html

It should be straight forward to add this as just looping through coords and create placeMarkers, in geoJSONSource.js
Comment 1 Jonas Danielsson 2015-11-02 09:37:49 UTC
You might have to create a geojson file by hand to get examples. Or try to create one with http://geojson.io
Comment 2 Alaf 2015-11-29 18:15:34 UTC
Created attachment 316477 [details] [review]
Add support for MultiPoint in GeoJSON
Comment 3 Alaf 2015-11-29 18:19:27 UTC
Created attachment 316478 [details] [review]
Add support for MultiPoint in GeoJSON
Comment 4 Alaf 2015-11-29 18:27:23 UTC
Created attachment 316479 [details] [review]
Add support for MultiPoint in GeoJSON
Comment 5 Alaf 2015-12-01 08:52:10 UTC
I used this geojson for testing.

https://gist.github.com/Alafazam/94d69b42a3844e7fd8b2

And it correctly creates placeMarkers. 
I think it should also Zoom in to corresponding boundingBox.
Comment 6 Jonas Danielsson 2015-12-01 19:06:57 UTC
Review of attachment 316479 [details] [review]:

Thanks!

This looks really nice, just some nits below  that I will fixup while pushing.
Thanks for the example geoJSON as well!

If you want to do more geoJSON stuff I would love to have some attention to this bug:
https://bugzilla.gnome.org/show_bug.cgi?id=757144

::: src/geoJSONSource.js
@@ +120,3 @@
     },
 
+    _parsePoint: function(coordinates,properties) {

Missing a space here:

coordinates, properties

@@ +169,2 @@
         case 'Point':
+            this._parsePoint(geometry.coordinates,properties);

Here as well.

@@ +173,3 @@
         case 'MultiPoint':
+            geometry.coordinates.forEach((function(coordinate,properties) {
+                this._parsePoint(coordinate,properties);

And here
Comment 7 Alaf 2015-12-01 19:20:53 UTC
Oh, Sorry next time i.ll be more careful. 
Thanks.

I think Map should also zoom into bounding box of place geoJson layer.
Comment 8 Jonas Danielsson 2015-12-01 19:22:46 UTC
Np!

But I think we already do that, right? The parsePoint function extends the bbox of the geoJSON layer and the mapView function that shows the layer zooms to the bbox.
Comment 9 Jonas Danielsson 2015-12-01 19:25:29 UTC
Or you mean that instead of extending the bbox we could do: this._bbox = this._markerLayer.get_bounding_box(); ?
Comment 10 Alaf 2015-12-01 19:32:37 UTC
yes exactly.
Comment 11 Jonas Danielsson 2015-12-02 09:38:28 UTC
But we can have geojson files that include points/multipoints as well as other features, so we can not rely on the markerLayer's bounding box exclusively.