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 zoom range restriction to keepCurrentZoomLevel #247

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Possible options are listed in the following table. More details are [in the cod
| `layer` | [`ILayer`](http://leafletjs.com/reference.html#ilayer) | The layer that the user's location should be drawn on. | a new layer |
| `setView` | `boolean` or `string` | Set the map view (zoom and pan) to the user's location as it updates. Options are `false`, `'once'`, `'always'`, `'untilPan'`, or `'untilPanOrZoom'` | `'untilPanOrZoom'` |
| `flyTo` | `boolean` | Smooth pan and zoom to the location of the marker. Only works in Leaflet 1.0+. | `false` |
| `keepCurrentZoomLevel` | `boolean` | Only pan when setting the view. | `false` |
| `keepCurrentZoomLevel` | `boolean` or `Array` | Only pan when setting the view. Set to True, or a zoom range like [13,18] which will only zoom the map when outside the rage. Allows keeping the current view at these scales | `false` |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct the grammar of this sentence. Also, end with a ..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the grammer issue is, but have just spotted the rage=>range typo. I'll try fixing this - I think as a new PR.

Copy link
Owner

@domoritz domoritz Jul 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this (it wasn't clear what the which clause refers to):

Suggested change
| `keepCurrentZoomLevel` | `boolean` or `Array` | Only pan when setting the view. Set to True, or a zoom range like [13,18] which will only zoom the map when outside the rage. Allows keeping the current view at these scales | `false` |
| `keepCurrentZoomLevel` | `boolean` or `Array` | Only pan when setting the view. Setting a zoom range such as `[13,18]` will only zoom the map if the current zoom level is outside the specified range. | `false` |

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just saw reply. I've updated the wording, try to keep clearer. It only zooms when outside the range. It keeps the zoom when within the range! ie keep zoom (if True, or within set range) barryhunter@1eaee57
not clear if that commit, is feeding back to pull request. if not will make a new one, after resolving other comment)

| `clickBehavior` | `object` | What to do when the user clicks on the control. Has three options `inView`, `inViewNotFollowing` and `outOfView`. Possible values are `stop` and `setView`, or the name of a behaviour to inherit from. | `{inView: 'stop', outOfView: 'setView', inViewNotFollowing: 'inView'}` |
| `returnToPrevBounds` | `boolean` | If set, save the map bounds just before centering to the user's location. When control is disabled, set the view back to the bounds that were saved. | `false` |
| `cacheLocation` | `boolean` | Keep a cache of the location after the user deactivates the control. If set to false, the user has to wait until the locate API returns a new location before they see where they are again. | `true` |
Expand Down
5 changes: 4 additions & 1 deletion src/L.Control.Locate.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,10 @@ You can find the project at: https://github.com/domoritz/leaflet-locatecontrol
this._event = undefined; // clear the current location so we can get back into the bounds
this.options.onLocationOutsideMapBounds(this);
} else {
if (this.options.keepCurrentZoomLevel) {
if (this.options.keepCurrentZoomLevel && (
typeof this.options.keepCurrentZoomLevel == "boolean" ||
(typeof this.options.keepCurrentZoomLevel == "object" && this._map.getZoom() <= this.options.keepCurrentZoomLevel[1] && this._map.getZoom() >= this.options.keepCurrentZoomLevel[0])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you extract this into a variable zoomOutsideRange and then write if (this.options.keepCurrentZoomLevel || zoomOutsideRange)?

Copy link
Author

@barryhunter barryhunter Jul 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what mean, make something like

  var zoomOutsideRange = typeof this.options.keepCurrentZoomLevel == "object" && this._map.getZoom() <= this.options.keepCurrentZoomLevel[1] && this._map.getZoom() >= this.options.keepCurrentZoomLevel[0];

if (this.options.keepCurrentZoomLevel) would still be true in case of Array, so the if(..) still complicated.

although maybe could do if (this.options.keepCurrentZoomLevel === true || zoomOutsideRange)? (not used === in javascript!)


Or to have new this.options.zoomOutsideRange to store the range?

  var zoomOutsideRange = typeof this.options.zoomOutsideRange == "object" && this._map.getZoom() <= this.options.zoomOutsideRange[1] && this._map.getZoom() >= this.options.zoomOutsideRange[0];

Then could just do... keepCurrentZoomLevel is back to just true/false

if (this.options.keepCurrentZoomLevel || zoomOutsideRange )

)) {
var f = this.options.flyTo ? this._map.flyTo : this._map.panTo;
f.bind(this._map)([this._event.latitude, this._event.longitude]);
} else {
Expand Down