smeijer / leaflet-geosearch

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

should not requery for location details, if there is enough info to act upon #288

Open olzraiti opened 2 years ago

olzraiti commented 2 years ago

Hi,

I created a custom provider, and noticed that the plugin works different from what I'd expect.

Situation

  1. When I type a query "a", and get results displayed as:

"a" (has coordinates [1, 2]) "a b" (has coordinates [2, 3]) "a b c" (has coordinates [3, 4])

  1. When I click the 2nd result "a b"...:

What happens?

A new query is made to the endpoint with search "a b" and the first result from that query is selected, which might result in different coordinates than what the existing result (2nd in the result list) had.

What I'd expect to happen?

I'd expect the map to zoom to coordinates of the 2nd result in the list [2 3], and not make any additional queries.

Why? Because that's what the user selected, and now the new query is made based on the label on the result list item, which might find some other result than what user selected. The API I'm using for my custom provider doesn't provide enough details so that I could display a label that could be used as a search term to get that exact same location. I don't see why a new query would be needed, if the result with the coordinates is already in the list.

I didn't find any way to control this via the options, and didn't find any existing issues about this. I studied the code, and am positive that it works the way explained here (I can elaborate if needed).

olzraiti commented 2 years ago

Here's a monkey patch for fixing this if anyone needs one:

const control = new SearchControl(options);
control.__proto__.onSubmit = function({data}) {
  this.showResult(data);
}
smeijer commented 2 years ago

You're correct about the way it operates. Not all providers include coordinates in that first search result, that's why it queries a second time. We should indeed fix it so that it doesn't require for those providers that already include the coordinates in the first result set.

olzraiti commented 2 years ago

Thanks for the quick response! That sounds good. Maybe either (1) skip making an extra query if the result has coordinates or (2) add a new option to control the behavior?

smeijer commented 2 years ago

I don't think we need to make it an option. If there is enough data available to complete the action (zoom to area, and place marker) we should just skip that extra query.

*edit, maybe we do need to make it an option. As the detail query might return more data than the list query. That additional data isn't used by the leaflet control, but can be useful for people that use the providers directly.

olzraiti commented 2 years ago

Yea, that's reasonable.