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

implementing toJSON for LatLng and LatLngBounds #1369

Closed
wants to merge 1 commit into from
Closed

implementing toJSON for LatLng and LatLngBounds #1369

wants to merge 1 commit into from

Conversation

diwu1989
Copy link

It's useful to have LatLng and LatLngBounds toJSON properly.

@diwu1989
Copy link
Author

This change is really small, any chance getting it merged in anytime soon?

@mourner
Copy link
Member

mourner commented Feb 19, 2013

Maybe, but I'm always hesitant to add new methods if the use case is not apparent. What's the use case for you?

@tmcw
Copy link
Contributor

tmcw commented Feb 19, 2013

toGeoJSON would be much more useful in this case.

@diwu1989
Copy link
Author

My use case for toJSON is when I serialize the state of the map for use with html5 push state.

@mourner
Copy link
Member

mourner commented Feb 21, 2013

@tmcw I didn't work or accept pulls for toGeoJSON yet because it seems to me that they're only useful in context of editing shapes, so Leaflet.draw (dedicated editing/drawing plugin) would be a better place for it. What do you think?

@uberbuilder
Copy link

@mourner I am working on this application: http://uberbuilder.github.com/choropleth/tests/2013/02/13/hunger-coalition-version-0.1.9/

Which brings in a bunch of data from a google spreadsheet and loads it into the map. I'm actually loading statistical data into the geoJSON which may be an incorrect use - and is a different topic all together.

I'm just saying, it would be nice to have a toGeoJSON function to be able to "save" a geoJSON with all of the information after I've already brought this into the app from all the different sources and after I've massaged the data like I have to create a more "cacheable" version of the geoJSON for my purposes. Since my app has nothing to do with drawing at all I would actually like to see this in the core of LeafLet for this kind of project. I don't want to have to load a whole other plugin just to get this functionality. To me, because this is a geoJSON thing, which can have more to do than just drawing I feel like this is more like a core responsibility.

But, seriously... you guys know way more about this than I do. I'm just offering my 2¢ on how I'd like to use leaflet.

@mourner
Copy link
Member

mourner commented Feb 21, 2013

@uberbuilder thanks for your feedback! You have a valid point for this, and it's very important to know opinion of people who actually build complex apps using Leaflet rather than just developing the library. ;) So much appreciated.

@mourner mourner mentioned this pull request Feb 21, 2013
@mourner
Copy link
Member

mourner commented Feb 21, 2013

@uberbuilder awesome project by the way! Is it ready to be announced publicly (Twitter etc.)?

@uberbuilder
Copy link

@mourner Trying not to be too chatty here in the back-end of your github, but I wanted to tell you that I've learned a lot from watching you manage your leaflet library. I'm very grateful for all of the things that I've learned here. A you may have guess from looking at my project I'm still very new at this building apps thing. I've learned a lot on this last project as I've been bringing it to completion.

And since you asked (literally as I was typing this I saw the page update with you asking when this project will go public)

I'm actually doing the finishing touches for my clients needs on this today. I then plan on re-writing all of my documentation on this. After that I will be identifying several topics that I'll write blogs on and post them all today as the first set of blogs for my new blog that I will host at uberbuilder.com. Much of what I will be writing about is my experience building this app, things I've learned and how I would approach doing it on the other side of all of this learning. Much of what I have learned has actually been from using Leaflet and contributing in a small way back to leaflet.

I'd like to re-design the app and release it for others to contribute and use for themselves. Half of the point of this project was to design an interactive choropleth for my client (a non-profit, this is an unpaid project) and the other half was to release a how-to on how I created this and maybe even package up a "starter" package for others to use. Maybe even maintain some kind of project on github to extend the use of this app. MAYBE I'd even consider creating a "plugin" for leaflet for some of the more advanced functions of the app (for example, using google spreadsheets to serve up the data used to drive the statistical data side of the app) which was a HUGE bonus for my client.

Any tips you could give me on how to do this for this project, perhaps how you would split up the code and package it for others (which is what I have to do in a few hours) would really great advice to hear. Maybe that could save me a bunch of refactoring of the project in the next few weeks.

So, yes - this is going public. And I'm releasing it in a few hours from now :)

@mourner
Copy link
Member

mourner commented Feb 21, 2013

Thanks Jeremy, sounds great! I'll wait until your announcement and try to find some time to review the project next week and share any advice that comes to mind.

@uberbuilder
Copy link

Vladimir, awesome. That will give me some time to organize it myself and get it into a presentable project. Thank you very much for your encouragement. Most of all, thank you for Leaflet.

@jmkelly
Copy link

jmkelly commented Feb 21, 2013

Thought I would put in my thoughts here, as I too would find a toGeoJson method very useful.

For me, if I could serialize FeatureGroups / LayerGroups / Layers as geojson, that would be very handy from an editing point of view. That said, to me, it feels like the toGeoJson is a natural method on the ILayer interface (or perhaps another interface which LayerGroup implements).

Given all this though, I am only interested in geojson when pushing / pulling data from some API, and the only time I am serializing to GeoJson is when pushing back, which is only after either creating or editing some layer.

@paulovieira
Copy link
Contributor

+1 for a "toGeoJson" method in leaflet core. I would use it immediately in my projects. I think the natural place for it would be L.Path, and then implement in L.Polyline, L.Polygon, etc.

@jmkelly
Copy link

jmkelly commented Feb 22, 2013

For me, I am not only interested in the just geometries to geojson, but also the access to the properties, because to transfer edited geometries back to a server, I need some sort of identifier to check against a primary. That is why I suggested on a LayerGroup, as these key properties can be serialized onto the layer when the geojson is loaded, and then deserilzed off, hopefully with Leaflets help, into geojson prior to sending updated data back server side.

@ghuntley
Copy link

I am not only interested in the just geometries to geojson, but 
also the access to the properties, because to transfer edited 
geometries back to a server, I need some sort of identifier to 
check against a primary.

Completely agree, persist geometries (w/ unique identifying key for each) and properties of the object so that the state/user interaction of the objects can be reapplied. Solving this at the library level [either in L.draw, ILayer or L.Path] will remove people re-inventing a solution again and again for what will be a common problem/use case. As there currently is no primary identifiable key when working with L.Polygon I end up sending the entire collection back per object type for persistence after the :save or :edit event has been emitted, would be much cleaner to be able to persist just the object (which is what has changed) individually and not to mention safer due to the async nature of ajax if rate of change is high and connection quality is low (yes I could append a timestamp as a etag or something similar but now we are in workaround/invent your own solution territory again...)

@mourner
Copy link
Member

mourner commented Apr 20, 2013

Merged John's #1462 toGeoJSON so closing this pull.

@mourner mourner closed this Apr 20, 2013
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.

None yet

7 participants