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

Add handling of lat, lon coordinate queries #368

Merged
merged 15 commits into from
Oct 4, 2023

Conversation

alexandervlpl
Copy link
Contributor

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.

@alexandervlpl alexandervlpl changed the title Add handling of <lat>, <lon> coordinate queries Add handling of lat, lon coordinate queries Jun 10, 2023
@smeijer
Copy link
Owner

smeijer commented Oct 4, 2023

Sorry for missing this. This looks great! Thanks.

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

src/coords.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/SearchControl.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
src/coords.ts Outdated Show resolved Hide resolved
@smeijer
Copy link
Owner

smeijer commented Oct 4, 2023

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

Thanks again!

@smeijer smeijer enabled auto-merge (squash) October 4, 2023 21:59
@smeijer smeijer disabled auto-merge October 4, 2023 21:59
@smeijer smeijer merged commit 1f19d1a into smeijer:develop Oct 4, 2023
8 checks passed
@smeijer
Copy link
Owner

smeijer commented Oct 4, 2023

🎉 This PR is included in version 3.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@alexandervlpl
Copy link
Contributor Author

Awesome, thanks for improving this. 👍

@pmaga
Copy link

pmaga commented Oct 5, 2023

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
  
    (...)
  },

gjvoosten added a commit to gjvoosten/leaflet-geosearch that referenced this pull request Oct 19, 2023
Revert "fix: don't invoke trim when query is undefined (smeijer#383)"
This reverts commit 036eb33.

Revert "feat: add handling of lat, lon coordinate queries (smeijer#368)"
This reverts commit 1f19d1a.
@alexandervlpl
Copy link
Contributor Author

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?

alexandervlpl added a commit to alexandervlpl/leaflet-geosearch that referenced this pull request May 3, 2024
Quick, *pure js* implementation to handle coordinate queries (smeijer#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](https://github.com/smeijer/leaflet-geosearch/assets/8945883/775ac7a2-3802-44e8-a3d5-c59d91d579e6)

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

---------

Co-authored-by: Stephan Meijer <stephan.meijer@gmail.com>
@smeijer
Copy link
Owner

smeijer commented May 17, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants