[ios] Partially offscreen annotations (without callouts) are no longer moved on-screen (#13727)

3 files changed, 46 insertions, 27 deletions

diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
index fab8e972fc..d1930cb940 100644
--- a/platform/macos/CHANGELOG.md
+++ b/platform/macos/CHANGELOG.md
@@ -6,7 +6,7 @@
 * Fixed a crash when casting large numbers in `NSExpression`. ([#13580](https://github.com/mapbox/mapbox-gl-native/pull/13580))
 * Added the `-[MGLShapeSource leavesOfCluster:offset:limit:]`, `-[MGLShapeSource childrenOfCluster:]`, `-[MGLShapeSource zoomLevelForExpandingCluster:]` methods for inspecting a cluster in an `MGLShapeSource`s created with the `MGLShapeSourceOptionClustered` option. Feature querying now returns clusters represented by `MGLPointFeatureCluster` objects (that conform to the `MGLCluster` protocol). ([#12952](https://github.com/mapbox/mapbox-gl-native/pull/12952)
 * Fixed a bug with `MGLMapView.visibleAnnotations` that resulted in incorrect results and performance degradation. ([#13745](https://github.com/mapbox/mapbox-gl-native/pull/13745))
-
+* Fixed a bug where selecting partially on-screen annotations (without a callout) would move the map. ([#13727](https://github.com/mapbox/mapbox-gl-native/pull/13727))
 
 ## 0.13.0 - December 20, 2018
 
diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm
index a5779aa602..56e768584d 100644
--- a/platform/macos/src/MGLMapView.mm
+++ b/platform/macos/src/MGLMapView.mm
@@ -2281,7 +2281,7 @@ public:
 
 - (BOOL)isMovingAnnotationIntoViewSupportedForAnnotation:(id<MGLAnnotation>)annotation animated:(BOOL)animated {
     // Consider delegating
-    return animated && [annotation isKindOfClass:[MGLPointAnnotation class]];
+    return [annotation isKindOfClass:[MGLPointAnnotation class]];
 }
 
 - (void)selectAnnotation:(id <MGLAnnotation>)annotation
@@ -2326,22 +2326,28 @@ public:
         positioningRect.origin = [self convertCoordinate:origin toPointToView:self];
     }
 
-    if (!moveIntoView && NSIsEmptyRect(NSIntersectionRect(positioningRect, self.bounds))) {
-        if (!NSEqualPoints(gesturePoint, NSZeroPoint)) {
+    BOOL shouldShowCallout = ([annotation respondsToSelector:@selector(title)]
+                              && annotation.title
+                              && !self.calloutForSelectedAnnotation.shown
+                              && [self.delegate respondsToSelector:@selector(mapView:annotationCanShowCallout:)]
+                              && [self.delegate mapView:self annotationCanShowCallout:annotation]);
+    
+    if (NSIsEmptyRect(NSIntersectionRect(positioningRect, self.bounds))) {
+        if (!moveIntoView && !NSEqualPoints(gesturePoint, NSZeroPoint)) {
             positioningRect = CGRectMake(gesturePoint.x, gesturePoint.y, positioningRect.size.width, positioningRect.size.height);
         }
     }
+    // Onscreen or partially on-screen
+    else if (!shouldShowCallout) {
+        moveIntoView = NO;
+    }
 
     self.selectedAnnotation = annotation;
 
     // For the callout to be shown, the annotation must have a title, its
     // callout must not already be shown, and the annotation must be able to
     // show a callout according to the delegate.
-    if ([annotation respondsToSelector:@selector(title)]
-        && annotation.title
-        && !self.calloutForSelectedAnnotation.shown
-        && [self.delegate respondsToSelector:@selector(mapView:annotationCanShowCallout:)]
-        && [self.delegate mapView:self annotationCanShowCallout:annotation]) {
+    if (shouldShowCallout) {
         NSPopover *callout = [self calloutForAnnotation:annotation];
 
         // Hang the callout off the right edge of the annotation image’s
@@ -2365,39 +2371,52 @@ public:
 
         NSRect (^edgeInsetsInsetRect)(NSRect, NSEdgeInsets) = ^(NSRect rect, NSEdgeInsets insets) {
             return NSMakeRect(rect.origin.x + insets.left,
-                              rect.origin.y + insets.top,
+                              rect.origin.y + insets.bottom,
                               rect.size.width - insets.left - insets.right,
                               rect.size.height - insets.top - insets.bottom);
         };
 
         // Add padding around the positioning rect (in essence an inset from the edge of the viewport
-        NSRect expandedPositioningRect = edgeInsetsInsetRect(positioningRect, MGLMapViewOffscreenAnnotationPadding);
+        NSRect expandedPositioningRect = positioningRect;
+        
+        if (shouldShowCallout) {
+            // If we have a callout, expand this rect to include a buffer
+            expandedPositioningRect = edgeInsetsInsetRect(positioningRect, MGLMapViewOffscreenAnnotationPadding);
+        }
 
         // Used for callout positioning, and moving offscreen annotations onscreen.
         CGRect constrainedRect = edgeInsetsInsetRect(self.bounds, self.contentInsets);
         CGRect bounds          = constrainedRect;
 
         // Any one of these cases should trigger a move onscreen
-        if (CGRectGetMinX(positioningRect) < CGRectGetMinX(bounds))
-        {
-            constrainedRect.origin.x = expandedPositioningRect.origin.x;
+        CGFloat minX = CGRectGetMinX(expandedPositioningRect);
+        
+        if (minX < CGRectGetMinX(bounds)) {
+            constrainedRect.origin.x = minX;
             moveIntoView = YES;
         }
-        else if (CGRectGetMaxX(positioningRect) > CGRectGetMaxX(bounds))
-        {
-            constrainedRect.origin.x = CGRectGetMaxX(expandedPositioningRect) - constrainedRect.size.width;
-            moveIntoView = YES;
+        else {
+            CGFloat maxX = CGRectGetMaxX(expandedPositioningRect);
+            
+            if (maxX > CGRectGetMaxX(bounds)) {
+                constrainedRect.origin.x = maxX - CGRectGetWidth(constrainedRect);
+                moveIntoView = YES;
+            }
         }
-
-        if (CGRectGetMinY(positioningRect) < CGRectGetMinY(bounds))
-        {
-            constrainedRect.origin.y = expandedPositioningRect.origin.y;
+        
+        CGFloat minY = CGRectGetMinY(expandedPositioningRect);
+        
+        if (minY < CGRectGetMinY(bounds)) {
+            constrainedRect.origin.y = minY;
             moveIntoView = YES;
         }
-        else if (CGRectGetMaxY(positioningRect) > CGRectGetMaxY(bounds))
-        {
-            constrainedRect.origin.y = CGRectGetMaxY(expandedPositioningRect) - constrainedRect.size.height;
-            moveIntoView = YES;
+        else {
+            CGFloat maxY = CGRectGetMaxY(expandedPositioningRect);
+            
+            if (maxY > CGRectGetMaxY(bounds)) {
+                constrainedRect.origin.y = maxY - CGRectGetHeight(constrainedRect);
+                moveIntoView = YES;
+            }
         }
 
         if (moveIntoView)
diff --git a/platform/macos/test/MGLAnnotationTests.m b/platform/macos/test/MGLAnnotationTests.m
index 36a7aef9f0..9657ae0a44 100644
--- a/platform/macos/test/MGLAnnotationTests.m
+++ b/platform/macos/test/MGLAnnotationTests.m
@@ -11,7 +11,7 @@
 - (void)setUp
 {
     [super setUp];
-    _mapView = [[MGLMapView alloc] initWithFrame:CGRectMake(0, 0, 64, 64)];
+    _mapView = [[MGLMapView alloc] initWithFrame:CGRectMake(0, 0, 256, 256)];
     _mapView.delegate = self;
 }