Add zoom range restriction to keepCurrentZoomLevel

[barryhunter]

Fixes https://github.com/domoritz/leaflet-locatecontrol/issues/246

[domoritz]

Hmm, thinking about this a bit more, I am not sure we want to merge this yet. When I see an array for keepCurrentZoomLevel, I'd think that this is providing the range that the zoom should be in (i.e. the target zoom level). I feel that filtering when to zoom is something that is very specific and should be done outside.

I'd say let's add an event that gets triggered so that you can implement your own logic for zooming in, okay?

I'm going to close this PR for now.

I hope you understand that I want to keep only core functionality as otherwise this library will get too complex to maintain.

[barryhunter]

ok. sure

but I'd think that this is providing the range that the zoom should be in (i.e. the target zoom level).

that is what is doing.

If define as [13,18] then, if outside that range, it WILL zoom to that range. Its only if its already within 13->18 that it doesnt zoom. ie keep current zoom, if already within 13->18.

[domoritz]

Not quite. What I thought a range would be a is a range to clamp to. For example, if the user defines [13,18], then the zoom level would be between 13 and 18 but we might still zoom. But you are right that this isn't quite compatible with keepCurrentZoomLevel as a name, which implies that the range has something to do with keeping the zoom level. I think now I understand it better and updated the wording a bit. Now I also see that this is quite useful and so let's merge it.

Sorry, I needed a moment to understand this proposal fully.

[domoritz]

Could you do these two things to finish the PR?

• [ ] Have a look at https://github.com/domoritz/leaflet-locatecontrol/pull/247#discussion_r303865857 and see whether you want to apply the suggestion.
• [ ] Apply your refactoring to extract zoomOutsideRange as you discussed in zoomOutsideRange.

Thanks!

[barryhunter]

So are thinking to have as a new separate 'option', as opposed to overloading keepCurrentZoomLevel. ie add 'options.zoomOutsideRange'.

Or is it just reflectoring the complicated if(..) clause that concerned about?

[domoritz]

I think keeping it part of keepCurrentZoomLevel is fine (no separate option). I am concerned about the docs (they need to be crystal clear so that others don't run into my confusions ;-)) and the complicated if clause.