PR for #29 Geocoder widget #95
Conversation
…nc resp handling
There was a problem hiding this comment.
Very cool @justb4. I like this a lot, especially the architecture with the different providers 👍 I added a few little remarks as inline comments. At least the test suite should be running (see my hint how to inject an app-config to the test spec.) before merging this.
What's also nice to have would be a clearing button to remove the search text. This could be done by clearable option of the Vuetify combobox. But this raises some layout issues. So I pledge that we postpone this.
The ability to add a result point or a result bbox to the map is already discussed in #29 and we also do this later on.
I'll open issues for these 2 points oncve this is merged.
Thanks again! Great work!
| describe('props', () => { | ||
| let comp; | ||
| beforeEach(() => { | ||
| comp = shallowMount(Geocoder); |
There was a problem hiding this comment.
Please look at test/unit/specs/components/ol/Map.spec.js or test/unit/specs/components/measuretool/MeasureWin.spec.js how to inject an app-config for the tests.
There was a problem hiding this comment.
Took some time (and sweat) but added more unit tests, in particular invoking the search input and combobox and verifying results even on map. Still could not get fake XHR and Timers working with Sinon, so faked with a setTimeout() in the test. So a real "Nominatim" search is performed for a well-known place/address in Bonn...
Also merged with latest version upgrades from #104 like OL6, Vue, Vuetify, and still works.
Made a slight implementation change: replacing callback from the GeocoderControllerto the Geocoder with a real async/await approach. Made testing also easier.
So unless more tests are required this is done, possibly additional issues for enhancements.
…nfig for single specfile test and README updates
There was a problem hiding this comment.
Thanks a lot. This component is great enhancement for Wegue 👍 . I just has a look at the code and tested the Geocoder in several apps. Works really well 🎉 🎆 . This is good to go IMHO.
I'll create some follow-up issues / task regrading the geocoder UI as discussed here (e.g. clear button, etc.).
| // // Do the query via Ajax XHR, returning JSON. Async callback via Promise. | ||
| // json(ajax) | ||
| // .then(response => { | ||
| // // Call back parent with data formatted by Provider | ||
| // this.parent.onQueryResult(this.provider.handleResponse(response)); | ||
| // }) | ||
| // .catch(err => { | ||
| // this.parent.onQueryResult(undefined, err); | ||
| // }); |
|
I'll merge now. Thanks again @justb4! |
…-panel Fixes wegue-oss#91 add header with close button to all Side Drawer Panels, merged in wegue-oss#95 from master (bigger icons LayerList). Looks good.
A first rough version, bit in the dark on Unit testing, seems many existing tests fail because of config missing?
Implementation was still a bit more involved than foreseen, mainly because of some Vuetify
v-combobox(dropdown state) quirks and dealing with the different Geocoder Providers behaviour.Three Geocoders supported:
'osm'(OSM Nominatim),'photon'(Photon) and'opencage'(OpenCage, needs API key). We can extend with other later such as MapQuest and Bing.Multiple commits, but may be squashed.