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

fix: support retainZoomLevel for providers not including bounds #270

Closed Dusty211 closed 3 years ago

Dusty211 commented 3 years ago

Hello, Whether or not this is actually needed or how it is implemented depends on the intended functionality, but I ran into an issue where it was impossible for me to get my zoomLevel option to affect anything when creating a new instance of GeoSearchControl. This was because I was using the HereProvider, and HereAPI does not return bounds. The provider returns null for result.bounds. Because result.bounds is null, this causes the first if statement in SearchControl.centerMap() to always evaluate to true, which disregards the zoomLevel option and uses const resultBounds, which in the case of hereAPI always evaluates to 'new L.LatLng(result.y, result.x).toBounds(10)' because result.bounds is null with HereProvider.

This effectively makes it so that you can never go by options.zoomLevel because the only way to miss the first if statement is to set retainZoomLevel = true or to have invalid resultBounds, which will cause SearchControl.getZoom() to use the current zoom level anyways: return retainZoomLevel ? this.map.getZoom() : zoomLevel;

My proposed solution adds another if statement at the beginning of SearchControl.centerMap() which executes the same code in contained in its else block if results.bounds and retainZoomLevel are both falsey.

I am probably missing some use cases, but I just wanted to help out anyway with your great library! Thank you!

Dusty211 commented 3 years ago

https://github.com/smeijer/leaflet-geosearch/issues/233

smeijer commented 3 years ago

Thanks @Dusty211 !

Published as 3.2.3.