This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Delegate method to restrict movement #5584

Merged

fabian-guerra merged 14 commits into master from 1ec5-should-change-camera-2457

Feb 9, 2017

Contributor

1ec5 commented Jul 5, 2016 •

edited

Loading

Added a way for the delegate to restrict where the user can move within the map using gestures. This approach allows the developer to restrict to a non-rectangular region or vary the region by zoom level or pitch as necessary, such as to prevent the user from seeing beyond a certain region regardless of the degree of tilt.

This PR intentionally keeps things simple to unblock some key use cases. Future improvements could include implementing a “rubber band” effect, allowing the delegate to “redirect” the camera to some other camera (#2457 (comment)), and automatically restricting the camera to downloaded regions (#5042).

Before we can land this feature:

Restrict all the other gestures on iOS: Delegate method to restrict movement #5584 (comment)
Add a warning about performance: Delegate method to restrict movement #5584 (comment)
- Or maybe find a way to call the method less frequently?
- Or calculate the final camera beforehand and avoid the set-and-revert dance?

Fixes #2457.

/cc @boundsj @frederoni @friedbunny

1ec5 added feature iOS macOS offline labels

1ec5 added this to the ios-v3.4.0 milestone

1ec5 self-assigned this

Contributor Author

1ec5 commented Jul 5, 2016

I think it’s too late for this feature to make the impending v3.3.0 final release, but we could easily backport it to v3.3.1 if it takes awhile to release v3.4.0.

This was referenced Jul 5, 2016

Let MGLMapViewDelegate cancel region changes #2457

Closed

[ios] Restrict panning of map with offline downloaded map #5042

Closed

Contributor Author

1ec5 commented Jul 6, 2016 •

edited

Loading

Note that this API does not affect programmatic movement of the viewport, such as via -flyToCamera:…, so we won’t run into mapbox/mapbox-gl-js#2521. If the developer wants to move to a viewport that the user wouldn’t be able to move to on their own, that’s the developer’s prerogative.

1ec5 modified the milestones: ios-v3.3.1, ios-v3.4.0

friedbunny reviewed

View reviewed changes

platform/macos/src/MGLMapViewDelegate.h

    
               This method is called as soon as the user gesture is recognized. It is not

               called in response to a programmatic camera change, such as by setting the

               `centerCoordinate` property or calling `-flyToCamera:completionHandler:`.

Contributor

friedbunny Jul 6, 2016

Should we add the standard, “this method is called many times during gesticulation so you should keep it as lightweight as possible”?

Contributor

friedbunny commented Jul 6, 2016 •

edited

Loading

This only seems to prevent simple pans, so far. With the new delegate method set to always return NO, it’s still possible to move the map via flick, pinch + pan, and rotate + pan. Quick zoom also works.

Contributor Author

1ec5 commented Jul 6, 2016

With the new delegate method set to always return NO, it’s still possible to move the map via flick, pinch + pan, and rotate + pan. Quick zoom also works.

Oh, you’re right! I took care of all the gesture recognizers for macOS but forgot to follow up for iOS.

Contributor Author

1ec5 commented Jul 6, 2016

Another caveat about this approach is that, if the delegate decides to block the camera change, the camera actually does change (even sending the -mapView:didChangeRegionAnimated: message to the delegate) before the map view resets the camera.

Zakay commented Jul 7, 2016

👍

1ec5 force-pushed the 1ec5-should-change-camera-2457 branch from 3366aa8 to 11253ec Compare

July 10, 2016 22:24

1ec5 added the GL JS parity label

1ec5 mentioned this pull request

MGLMapviewDelegate pan event protocol #5636

Closed

1ec5 added the ⚠️ DO NOT MERGE label

1ec5 force-pushed the 1ec5-should-change-camera-2457 branch from 11253ec to ce3d847 Compare

July 19, 2016 05:39

1ec5 modified the milestones: ios-v3.3.1, ios-v3.4.0

1ec5 added MapKit parity and removed MapKit parity labels

1ec5 mentioned this pull request

Limit Viewport #3602

Closed

Contributor

maciekish commented Sep 14, 2016

Any chance to see this in v3.3.1 please?

1ec5 commented

View reviewed changes

platform/ios/src/MGLMapView.mm Outdated

    
                  return camera;

              }

              - (double)currentCameraZoom

Contributor Author

1ec5 Feb 7, 2017

This method is unnecessary; use self.zoomLevel instead.

platform/ios/src/MGLMapView.mm Outdated

    
                  return camera;

              }

              - (MGLMapCamera *)currentCameraWithEstimatedDegrees:(CGFloat)degrees anchorPoint:(CGPoint)anchorPoint

Contributor Author

1ec5 Feb 7, 2017

Use CLLocationDirection to represent the map’s rotation in degrees.

fabian-guerra force-pushed the 1ec5-should-change-camera-2457 branch from 41c6a5b to 97e99e8 Compare

February 8, 2017 18:43

1ec5 commented

View reviewed changes

Contributor Author

1ec5 left a comment

Thanks for hanging in there – just a few nits related to coding style.

platform/ios/src/MGLMapView.mm Outdated

+                      // Calculates the final camera zoom, has no effect within current map camera.
+                      MGLMapCamera *toCamera;
+                      double zoom = log2(newScale);
+                      toCamera = [self cameraByZoomingToZoomLevel:zoom aroundAnchorPoint:centerPoint];

Contributor Author

1ec5 Feb 8, 2017

Declare and set toCamera on the same line.

platform/ios/src/MGLMapView.mm Outdated

-                      _mbglMap->moveBy({ delta.x, delta.y });
-                      [pan setTranslation:CGPointZero inView:pan.view];
+                      toCamera = [self cameraByPanningWithTranslation:delta panGesture:pan];

Contributor Author

1ec5 Feb 8, 2017

Move the declaration of toCamera down to this line.

platform/ios/src/MGLMapView.mm Outdated

+                      // Calculates the final camera zoom, has no effect within current map camera.
+                      MGLMapCamera *toCamera;
+                      double zoom = log2(newScale);

Contributor Author

1ec5 Feb 8, 2017

Nit: declare this a few lines further up to avoid saying log2(newScale) twice.

platform/ios/src/MGLMapView.mm Outdated


		[self animateWithDelay:MGLAnimationDuration animations:^

		MGLMapCamera *toCamera = [self cameraByZoomingToZoomLevel:newZoom aroundAnchorPoint:gesturePoint];

Contributor Author

1ec5 Feb 8, 2017

Nit: s/newZoom/self.zoomLevel + 1/

fabian-guerra force-pushed the 1ec5-should-change-camera-2457 branch 2 times, most recently from e6cec13 to a3e7277 Compare

February 9, 2017 16:30

1ec5 commented

View reviewed changes

Contributor Author

1ec5 left a comment

One nit left, but otherwise this is good to go.

platform/ios/src/MGLMapView.mm Outdated

    
                      MGLMapCamera *toCamera = [self cameraByPanningWithTranslation:delta panGesture:pan];

                      if (![self.delegate respondsToSelector:@selector(mapView:shouldChangeFromCamera:toCamera:)] ||

                          ([self.delegate respondsToSelector:@selector(mapView:shouldChangeFromCamera:toCamera:)]

Contributor Author

1ec5 Feb 9, 2017 •

edited

Loading

This is redundant: !A || (A && C) is equivalent to !A || C.

1ec5 dismissed incanus’s stale review

February 9, 2017 19:04

My comments in #5584 (review) have been addressed; we’re OK with reporting metrics for aborted gestures; and @incanus’ nits have been addressed as well.

1ec5 mentioned this pull request

Move cameraBy* methods from MGLMapView to MGLMapCamera #7990

Open

1ec5 and others added 13 commits

February 9, 2017 14:55


          [ios, macos] Added delegate method to restrict movement

9181aeb

Added a way for the delegate to restrict where the user can move within the map using gestures.

Fixes #2457.


          [ios] Added support to restrict movement in pinch/rotate gestures

0639eb1


          [ios] Added support to restrict movement in double tap/quick zoom/two…

e683853

… finger drag gestures


          [ios] fixed camera reset before two finger drag gesture is complete

631ea0a


          [ios] fixed camera comparison in double tap gestures

a171073


          [ios] Changelog update

f7824ec


          [macos] Changelog updated

9933e4a


          [ios, macos] Changelog cleanup

57b6d4a


          [ios, macos] Added documentation to clarify performance impact

133cb22


          [ios] clarified variable name

2450cb6


          [ios] blocking gestures implementation changed to a predictive approach

18b73dd


          [ios] gesture delegate methods refactoring

7eebd83


          [ios] Removed duplicated methods, improved code readability

be62f8c

fabian-guerra force-pushed the 1ec5-should-change-camera-2457 branch from a3e7277 to f9fe145 Compare

February 9, 2017 20:00


          [ios] code refactoring to clarify the conditions to execute a gesture

082e898

fabian-guerra force-pushed the 1ec5-should-change-camera-2457 branch from f9fe145 to 082e898 Compare

February 9, 2017 20:15

fabian-guerra merged commit 272dc3f into master

1ec5 deleted the 1ec5-should-change-camera-2457 branch

February 11, 2017 22:15

friedbunny mentioned this pull request

Constraining the map to a predefined geographic region #2329

Closed

1ec5 removed the ⚠️ DO NOT MERGE label

1ec5 mentioned this pull request

-[MGLMapViewDelegate mapView: regionDidChangeAnimated:] redundant calls #7119

Closed

1ec5 mentioned this pull request

Adds a delegate to notify when a gesture is canceled. mapbox/mapbox-gl-native-ios#284

Open

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature GL JS parity iOS macOS offline