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

91 spatial form widget, upgrade leaflet and leaflet.draw #93

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

florianm
Copy link

Here's an attempt at providing a spatial widget, addressing #91 . There are implications and I'm not sure whether my percussive implementation approach (hitting it with a wrench until it works) was the most graceful way, so I'd be grateful for feedback and suggestions.

Main motivation: My end users love geo-referencing their datasets, and being able to search by map. But many (especially non-GIS-trained staff) struggle with GeoJSON, let alone a GeoJSON geometry. At the same time, my end users have expert knowledge about the true extent of their dataset. This means, letting them draw rather than paste or derive the extent would both make their lives easier (actually, enable them to provide geo-reference at all), and capture the most accurate study extent. Having accurate enough dataset extents would enable an agency to use the search tool to identify possible conflicts of planned field work with existing field sites. E.g., some of my end users manage controlled burns, while others monitor vegetation through 40+ years of non-burning.

So all we need is a map on which we can draw and edit polygons and rectangles, and the map needs to be data-bound to the "spatial" input. Bonus if one could pick spatial extents from a dropdown menu of favourite areas. Even better (haven't solved that one yet) if that list could be maintained through an API and would auto-complete like the tags do.

  • Updated Leaflet to 0.8-dev and Leaflet.draw to 0.2.3 (is there any better way than overwriting files in /vendor/?)
  • Used a patched geoalchemy as per Patched Geoalchemy for PostGIS 2 #92
  • Updated spatial-query's interaction with Leaflet.draw (required due to breaking changes in Leaflet.draw)
  • Provided a new module spatial-form (added usage docs directly into the file, to be moved into a better place later)
    and, over at ckanext-scheming, there's a new spatial input widget which will be a separate pull request for ckanext-scheming.

Currently, the dropdown comes out of ckanext-scheming, while the map comes from ckanext-spatial. To keep things simple, I've only data-bound map->text, dropdown->text, but neither dropdown->map nor text->map.

Screenshots of working examples are at ckanext-scheming issue 11, but feel free to take my sandbox for a whirl (request a login from me) or, if you need your hands free (depending on how much you enjoy spatial stuff, I'm not judging), watch the thing in action at https://vimeo.com/116324887

Review on Reviewable

@florianm florianm changed the title 91 spatial form widget 91 spatial form widget, upgrade leaflet and leaflet.draw Dec 22, 2014
@adonm
Copy link

adonm commented Jan 11, 2015

+1 =)

@amercader
Copy link
Member

This looks fantastic, I'll try and merge it as soon as I sort out the 2.3 support

@@ -0,0 +1 @@
!function(){function a(b,c,d){var e=a.resolve(b);if(null==e){d=d||b,c=c||"root";var f=new Error('Failed to require "'+d+'" from "'+c+'"');throw f.path=d,f.parent=c,f.require=!0,f}var g=a.modules[e];return g.exports||(g.exports={},g.client=g.component=!0,g.call(this,g.exports,a.relative(e),g)),g.exports}a.modules={},a.aliases={},a.resolve=function(b){"/"===b.charAt(0)&&(b=b.slice(1));for(var c=[b,b+".js",b+".json",b+"/index.js",b+"/index.json"],d=0;d<c.length;d++){var b=c[d];if(a.modules.hasOwnProperty(b))return b;if(a.aliases.hasOwnProperty(b))return a.aliases[b]}},a.normalize=function(a,b){var c=[];if("."!=b.charAt(0))return b;a=a.split("/"),b=b.split("/");for(var d=0;d<b.length;++d)".."==b[d]?a.pop():"."!=b[d]&&""!=b[d]&&c.push(b[d]);return a.concat(c).join("/")},a.register=function(b,c){a.modules[b]=c},a.alias=function(b,c){if(!a.modules.hasOwnProperty(b))throw new Error('Failed to alias "'+b+'", it does not exist');a.aliases[c]=b},a.relative=function(b){function c(a,b){for(var c=a.length;c--;)if(a[c]===b)return c;return-1}function d(c){var e=d.resolve(c);return a(e,b,c)}var e=a.normalize(b,"..");return d.resolve=function(d){var f=d.charAt(0);if("/"==f)return d.slice(1);if("."==f)return a.normalize(e,d);var g=b.split("/"),h=c(g,"deps")+1;return h||(h=0),d=g.slice(0,h+1).join("/")+"/deps/"+d},d.exists=function(b){return a.modules.hasOwnProperty(d.resolve(b))},d},a.register("calvinmetcalf-setImmediate/lib/index.js",function(a,b,c){"use strict";function d(){var a,b=0,c=g;for(g=[];a=c[b++];)a()}var e,f=[b("./nextTick"),b("./mutation"),b("./postMessage"),b("./messageChannel"),b("./stateChange"),b("./timeout")],g=[];f.some(function(a){var b=a.test();return b&&(e=a.install(d)),b});var h=function(a){var b,c;return arguments.length>1&&"function"==typeof a&&(c=Array.prototype.slice.call(arguments,1),c.unshift(void 0),a=a.bind.apply(a,c)),1===(b=g.push(a))&&e(d),b};h.clear=function(a){return a<=g.length&&(g[a-1]=function(){}),this},c.exports=h}),a.register("calvinmetcalf-setImmediate/lib/nextTick.js",function(a){"use strict";a.test=function(){return"object"==typeof process&&"[object process]"===Object.prototype.toString.call(process)},a.install=function(){return process.nextTick}}),a.register("calvinmetcalf-setImmediate/lib/postMessage.js",function(a,b){"use strict";var c=b("./global");a.test=function(){if(!c.postMessage||c.importScripts)return!1;var a=!0,b=c.onmessage;return c.onmessage=function(){a=!1},c.postMessage("","*"),c.onmessage=b,a},a.install=function(a){function b(b){b.source===c&&b.data===d&&a()}var d="com.calvinmetcalf.setImmediate"+Math.random();return c.addEventListener?c.addEventListener("message",b,!1):c.attachEvent("onmessage",b),function(){c.postMessage(d,"*")}}}),a.register("calvinmetcalf-setImmediate/lib/messageChannel.js",function(a,b){"use strict";var c=b("./global");a.test=function(){return!!c.MessageChannel},a.install=function(a){var b=new c.MessageChannel;return b.port1.onmessage=a,function(){b.port2.postMessage(0)}}}),a.register("calvinmetcalf-setImmediate/lib/stateChange.js",function(a,b){"use strict";var c=b("./global");a.test=function(){return"document"in c&&"onreadystatechange"in c.document.createElement("script")},a.install=function(a){return function(){var b=c.document.createElement("script");return b.onreadystatechange=function(){a(),b.onreadystatechange=null,b.parentNode.removeChild(b),b=null},c.document.documentElement.appendChild(b),a}}}),a.register("calvinmetcalf-setImmediate/lib/timeout.js",function(a){"use strict";a.test=function(){return!0},a.install=function(a){return function(){setTimeout(a,0)}}}),a.register("calvinmetcalf-setImmediate/lib/global.js",function(a,b,c){c.exports="object"==typeof global&&global?global:this}),a.register("calvinmetcalf-setImmediate/lib/mutation.js",function(a,b){"use strict";var c=b("./global"),d=c.MutationObserver||c.WebKitMutationObserver;a.test=function(){return d},a.install=function(a){var b=new d(a),e=c.document.createElement("div");return b.observe(e,{attributes:!0}),c.addEventListener("unload",function(){b.disconnect(),b=null},!1),function(){e.setAttribute("drainQueue","drainQueue")}}}),a.register("lie/lie.js",function(a,b,c){function d(a){function b(a,b){return d(function(c,d){k.push({resolve:a,reject:b,resolver:c,rejecter:d})})}function c(a,c){return l?l(a,c):b(a,c)}function h(a,b){for(var d,h,i=a?"resolve":"reject",j=0,m=k.length;m>j;j++)d=k[j],h=d[i],"function"==typeof h?g(f,h,b,d.resolver,d.rejecter):a?d.resolver(b):d.rejecter(b);l=e(c,b,a)}function i(a){l||h(!0,a)}function j(a){l||h(!1,a)}if(!(this instanceof d))return new d(a);var k=[],l=!1;this.then=c;try{a(function(a){a&&"function"==typeof a.then?a.then(i,j):i(a)},j)}catch(m){j(m)}}function e(a,b,c){return function(e,h){var i=c?e:h;return"function"!=typeof i?d(function(b,c){a(b,c)}):d(function(a,c){g(f,i,b,a,c)})}}function f(a,b,c,d){try{var e=a(b);e&&"function"==typeof e.then?e.then(c,d):c(e)}catch(f){d(f)}}var g=b("immediate");c.exports=d}),a.alias("calvinmetcalf-setImmediate/lib/index.js","lie/deps/immediate/lib/index.js"),a.alias("calvinmetcalf-setImmediate/lib/nextTick.js","lie/deps/immediate/lib/nextTick.js"),a.alias("calvinmetcalf-setImmediate/lib/postMessage.js","lie/deps/immediate/lib/postMessage.js"),a.alias("calvinmetcalf-setImmediate/lib/messageChannel.js","lie/deps/immediate/lib/messageChannel.js"),a.alias("calvinmetcalf-setImmediate/lib/stateChange.js","lie/deps/immediate/lib/stateChange.js"),a.alias("calvinmetcalf-setImmediate/lib/timeout.js","lie/deps/immediate/lib/timeout.js"),a.alias("calvinmetcalf-setImmediate/lib/global.js","lie/deps/immediate/lib/global.js"),a.alias("calvinmetcalf-setImmediate/lib/mutation.js","lie/deps/immediate/lib/mutation.js"),a.alias("calvinmetcalf-setImmediate/lib/index.js","lie/deps/immediate/index.js"),a.alias("calvinmetcalf-setImmediate/lib/index.js","immediate/index.js"),a.alias("calvinmetcalf-setImmediate/lib/index.js","calvinmetcalf-setImmediate/index.js"),a.alias("lie/lie.js","lie/index.js"),L.Util.Promise=a("lie")}(),L.Util.ajax=function(url,options){"use strict";if(options=options||{},options.jsonp)return L.Util.ajax.jsonp(url,options);var request,cancel,out=L.Util.Promise(function(resolve,reject){var Ajax;cancel=reject,Ajax=void 0===window.XMLHttpRequest?function(){try{return new ActiveXObject("Microsoft.XMLHTTP.6.0")}catch(a){try{return new ActiveXObject("Microsoft.XMLHTTP.3.0")}catch(b){reject("XMLHttpRequest is not supported")}}}:window.XMLHttpRequest;var response;request=new Ajax,request.open("GET",url),request.onreadystatechange=function(){4===request.readyState&&(request.status<400&&options.local||200===request.status?(window.JSON?response=JSON.parse(request.responseText):options.evil&&(response=eval("("+request.responseText+")")),resolve(response)):request.status?reject(request.statusText):reject("Attempted cross origin request without CORS enabled"))},request.send()});return out.then(null,function(a){return request.abort(),a}),out.abort=cancel,out},L.Util.jsonp=function(a,b){b=b||{};var c,d,e,f,g=document.getElementsByTagName("head")[0],h=L.DomUtil.create("script","",g),i=L.Util.Promise(function(i,j){f=j;var k=b.cbParam||"callback";b.callbackName?c=b.callbackName:(e="_"+(""+Math.random()).slice(2),c="L.Util.jsonp.cb."+e),h.type="text/javascript",e&&(L.Util.jsonp.cb[e]=function(a){g.removeChild(h),delete L.Util.jsonp.cb[e],i(a)}),d=-1===a.indexOf("?")?a+"?"+k+"="+c:a+"&"+k+"="+c,h.src=d}).then(null,function(a){return g.removeChild(h),delete L.Util.ajax.cb[e],a});return i.abort=f,i},L.Util.jsonp.cb={},L.GeoJSON.AJAX=L.GeoJSON.extend({defaultAJAXparams:{dataType:"json",callbackParam:"callback",local:!1,middleware:function(a){return a}},initialize:function(a,b){this.urls=[],a&&("string"==typeof a?this.urls.push(a):"function"==typeof a.pop?this.urls=this.urls.concat(a):(b=a,a=void 0));var c=L.Util.extend({},this.defaultAJAXparams);for(var d in b)this.defaultAJAXparams.hasOwnProperty(d)&&(c[d]=b[d]);this.ajaxParams=c,this._layers={},L.Util.setOptions(this,b),this.on("data:loaded",function(){this.filter&&this.refilter(this.filter)},this);var e=this;this.urls.length>0&&L.Util.Promise(function(a){a()}).then(function(){e.addUrl()})},clearLayers:function(){return this.urls=[],L.GeoJSON.prototype.clearLayers.call(this),this},addUrl:function(a){var b=this;a&&("string"==typeof a?b.urls.push(a):"function"==typeof a.pop&&(b.urls=b.urls.concat(a)));var c=b.urls.length,d=0;b.fire("data:loading"),b.urls.forEach(function(a){"json"===b.ajaxParams.dataType.toLowerCase()?L.Util.ajax(a,b.ajaxParams).then(function(a){var c=b.ajaxParams.middleware(a);b.addData(c),b.fire("data:progress",c)},function(a){b.fire("data:progress",{error:a})}):"jsonp"===b.ajaxParams.dataType.toLowerCase()&&L.Util.jsonp(a,b.ajaxParams).then(function(a){var c=b.ajaxParams.middleware(a);b.addData(c),b.fire("data:progress",c)},function(a){b.fire("data:progress",{error:a})})}),b.on("data:progress",function(){++d===c&&b.fire("data:loaded")})},refresh:function(a){a=a||this.urls,this.clearLayers(),this.addUrl(a)},refilter:function(a){"function"!=typeof a?(this.filter=!1,this.eachLayer(function(a){a.setStyle({stroke:!0,clickable:!0})})):(this.filter=a,this.eachLayer(function(b){a(b.feature)?b.setStyle({stroke:!0,clickable:!0}):b.setStyle({stroke:!1,clickable:!1})}))}}),L.geoJson.ajax=function(a,b){return new L.GeoJSON.AJAX(a,b)};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed? If so it should go to vendor

@amercader
Copy link
Member

@florianm I had a look at this and it looks really good, see some comments below:

  • No need to deal with GeoAlchemy versions on this PR, this has been dealt with on master (Use GeoAlchemy2 instead of GeoAlchemy (Support for CKAN>=2.3 and PostGIS 2.x) #97)
  • Can we stick with the latest stable version for Leaflet (0.7.x) instead of 0.8-dev? Or do you need this specific version? BTW updating the libraries on vendor is fine
  • The changes in Leaflet.draw have introduced a bug on the spatial query widget, where the old bbox (eg from a previous search) is kept when drawing a new one:

g3go8b3

  • The new icon to draw a bbox seems a bit confusing to me. I think that the pencil one is more intuitive for users. I realize this is the default from the new Leaflet.draw version, but maybe it can be overridden with a config option.

voka1ny
oj1uv58

  • Regarding the discussion you and @wardi had on Feature request: spatial widget ckanext-scheming#11, my view is that this feature would be more useful to users if it was shipped on ckanext-spatial. The pattern followed by other widgets is to provide a single snippet that wraps all markup, libraries, etc and users only have to add one line to their templates. I can't see why this should not be the case for the extent editor as well. We will also avoid duplicating libraries in different extensions. That doesn't mean of course it shouldn't place nicely with ckanext-scheming. I'm still not familiar enough with how ckanext-scheming works, but it looks like using presets and/or form snippets could be an easy way for users to integrate.
  • AFAICT the spatial_form.html snippet requires you to manually add a spatial field elsewhere in the form. Let's make things even easier for users and add that field in the snippet as well so you don't have to worry about creating it. Also handling how to receive a previously added extent when editing, etc.
  • Related to the previous point, I might be wrong, but it seems to me that the widget assumes that the field used to store the geometry is a root field called spatial (ie that you are using a custom schema). If that is the case we should also consider the case where users just use a normal extra with a key spatial (or always assume this case)

Also if you can give me permissions to create a dataset on your sandbox site that would be great (user amercader)

@florianm
Copy link
Author

@amercader - apologies for the delay and thanks for your feedback!
You have now admin access to http://data-demo.dpaw.wa.gov.au/organization/ckan-developers

I agree with all your points and will try and implement them, with the only exception being that I need leaflet-draw for the drawing functionality, which in turn requires Leaflet-0.8.

I'll probably need some advice around refactoring the code into a snippet later!

@deniszgonjanin
Copy link

This is amazing. Works well on 2.3 as is (merge conflicts notwithstanding). I'm using it with ckanext-scheming, with the relevant display and form snippets

@florianm @amercader would love to see this merged (as well as see a PR to ckanext-scheming), let me know if there is any way I can help.

@deniszgonjanin
Copy link

One small issue:

  • The new snippets/spatial_query.html does not clear the bbox between queries, so if you do multiple spatial queries with the widget, the extent only gets bigger and bigger and there is no way to clear it.

EDIT: Except the umm.. clear button. But I don't know if this was a desired effect? It seems the extent should be cleared between searches.

@florianm
Copy link
Author

Denis, Adria,

thanks for your feedback, very appreciated!

There are some issues with my PR:

  • I've changed more files than necessary as I got a little overexcited when
    trying to update third party libraries (Leaflet and Leaflet-draw)
  • The GeoAlchemy problem has been fixed
  • I provide three separate input methods, but did not data-bind all of them
    both ways to keep things simple
  • both you and Adria spotted the immortal "old search box" bug
  • I've written this PR late last year, so it's quite a few commits behind
    master now
  • Joel has a fourth, awesome "calculate extent from existing attached
    spatial resources" georeferencing method, although I couldn't find the code
    yet (could be proprietary)

I suggest to not merge this PR, but for me to try and re-implement it
starting from the current master, trying to follow all feedback I've gotten
so far. I will definitely run into problems - especially writing tests (can
Selenium draw on maps?) where I'll have to ask you for advice :-)

I would value your input on functional specs - which additional use cases
would you like to see implemented?

Cheers,
Florian

On Sat, Jun 13, 2015 at 3:45 AM, Denis Zgonjanin notifications@github.com
wrote:

One small issue:

  • The new snippets/spatial_query.html does not clear the bbox between
    queries, so if you do multiple spatial queries with the widget, the extent
    only gets bigger and bigger and there is no way to clear it.


Reply to this email directly or view it on GitHub
#93 (comment).

@Stephen-Gates
Copy link

Is there a status update for this. It looks very promising and exactly what we're chasing. Thanks.

@florianm
Copy link
Author

@Stephen-Gates we've been using this version in production internally in my Department without problems (setup docs here), but unfortunately I haven't had resources to follow up on this PR.

One improvement apart from the suggestions in this thread is to look up geometries by name from an API endpoint, like data.gov.au does (but with polygons) - similar to CKAN's tag autocomplete.

@amercader
Copy link
Member

amercader commented Jun 17, 2016

If someone is willing to take this one on and address the issues mentioned on the thread I'd be more than happy to review again and merge if ok.

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

5 participants