Blocking map gestures delegate example

[fabian-guerra]

No description provided.

[1ec5]

This assumes the physical distance between the old center and old northeast corner is equal to the physical distance between the new center and the new northeast corner. Because the map uses Mercator projection, the latitude lines aren’t evenly spaced. So the center may change by a different amount than the top and bottom edges of the view. I’d imagine this issue would be most apparent when dramatically swiping the map north or south to either pole. You can mitigate this issue by restricting the map to high zoom levels, but that would limit the example’s usefulness.

Moreover, if the user rotates the map, these calculations can become severely inaccurate. You could mitigate this issue by turning off rotation.

An alternative approach would be to “set and revert” within mapView(_:shouldChangeFrom:to:): store the current camera as camera, set mapView.camera to newCamera, store the visibleCoordinateBounds in a local variable, and set mapView.camera back to camera. (I’d avoid relying on oldCamera to equal mapView.camera, because we may figure out a way to batch up calls to mapView(_:shouldChangeFrom:to:) in the future.)

[1ec5]

Use MGLCoordinateBounds(sw:ne:) instead of MGLCoordinateBoundsMake(_:_:).

[boundsj]

I think it'd be interesting to do a performance comparison with the approach already in this PR and the approach outlined in #65 (review). A less accurate, limited solution may still be useful for cases involving offline maps that usually involve higher zoom levels.

[1ec5]

Is it still necessary to disable rotation and cap the zoom level, now that you’re doing the set-and-revert dance?

[fabian-guerra]

The example I made deals with rotation but I considered was not important for restricting boundaries, I reverted this.

[1ec5]

These comments say what the developer is expected to do, but not why. An example needs to explain the rationale behind each non-obvious step, in order to avoid the phenomenon of cargo cult programming. In particular, we should be explicit about the fact that we’re changing and resetting the map view’s camera in order to predict the extents of the new viewport.

[fabian-guerra]

I changed the comments to make it more explicit.

[1ec5]

Ah, I see why you’ve restricted the zoom level and disabled rotation. Can you think of a way to test whether the new viewport would fit inside the allowed region (Colorado) while accounting for rotation? Perhaps the logic in turf-inside would be helpful here, although it might be overkill.

[fabian-guerra]

I'm already taking a similar approach like function inBBox and also accounts for restricting rotation if its out of boundaries.

[1ec5]

Yes, this code is fine. I was mistaken previously in thinking that this logic would produce incorrect results when the map is rotated, but the visible coordinate bounds will extend beyond the allowed region. This logic would not work if the map is tilted or if the allowed region itself is rotated.

[fabian-guerra]

@1ec5 @captainbarbosa @jmkiley could you please take a look.

[1ec5]

A couple minor style notes, but otherwise this is good to go.

[1ec5]

Nit: drop self., since it’s implied.

[1ec5]

The use of a period at the end of a comment is inconsistent in this file. Here there’s no period at the end of the sentence; above, “Starting point” does end with a period, even though it isn’t a sentence. Normally I end all the comments with periods if any of them is a sentence.

[1ec5]

Nit: centerCoordinate’s type is inferred as CLLocationCoordinate2D, so there’s no need to declare newCameraCenter’s type here. (Same with inside and intersects below.)