smeijer / leaflet-geosearch

A geocoding/address-lookup library supporting various api providers.
https://smeijer.github.io/leaflet-geosearch/
MIT License
1.02k stars 270 forks source link

Add handling of lat, lon coordinate queries #368

Closed alexandervlpl closed 9 months ago

alexandervlpl commented 1 year ago

Quick, pure js implementation to handle coordinate queries (#147). If a user enters valid coordinates, they get a single result with that point (as in Google Maps and other services). It's a useful feature for power users. image

Any other queries are sent to the Provider. Happy to see this in TypeScript and otherwise improved, it's probably not ready for merging.

smeijer commented 9 months ago

Sorry for missing this. This looks great! Thanks.

Unfortunately, CI is failing. Can you look into that?

smeijer commented 9 months ago

Lol, I did it. Fixed it from my phone. 😆

Thanks again!

smeijer commented 9 months ago

:tada: This PR is included in version 3.10.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

alexandervlpl commented 9 months ago

Awesome, thanks for improving this. :+1:

pmaga commented 9 months ago

This broke a standard submit: handleSubmit: (result) => this.onSubmit(result),. It tries to trim a result object that is not a string.

  async onSubmit(query) {
    this.resultList.clear();
    const { provider } = this.options;

    let results = [];
    const coords = validateCoords(query);             //    <---- here

    (...)
  },
alexandervlpl commented 2 months ago

Apologies for what was clearly a bad PR, I did say it isn't ready for merging and was planning to work on it after feedback. Probably should not have submitted something so half baked. I see the concerns from @gjvoosten in #384 and they're valid.

That said, I don't think this should be abandoned because "some search providers already support coordinate searches". I think most do not, and offloading this to providers when it can be done instantly in the front end seems very inefficient.

Is it worth revisiting this, getting it TypeScripted and properly tested?

smeijer commented 1 month ago

No worries, I was the one that merged the PR, so I took the responsibility :)

Happy to merge (after proper testing 😅) any other contribution you make. Thanks!