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

10 files changed, 562 insertions, 61 deletions

diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
index 58413b3184..9212b97c9a 100644
--- a/platform/ios/CHANGELOG.md
+++ b/platform/ios/CHANGELOG.md
@@ -10,6 +10,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Reinstates version 11 as the default Mapbox Streets style (as introduced in 4.7.0). ([#13690](https://github.com/mapbox/mapbox-gl-native/pull/13690))
 * 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)
 * `MGLMapView` no longer freezes on external displays connected through AirPlay or CarPlay when the main device’s screen goes to sleep or the user manually locks the screen. ([#13701](https://github.com/mapbox/mapbox-gl-native/pull/13701))
+* 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))
 
 ## 4.7.1 - December 21, 2018
 
diff --git a/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m b/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m
index fefb938773..7ec45de072 100644
--- a/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m
+++ b/platform/ios/Integration Tests/Annotation Tests/MGLAnnotationViewIntegrationTests.m
@@ -2,9 +2,26 @@
 #import "MGLTestUtility.h"
 #import "MGLMapAccessibilityElement.h"
 #import "MGLTestLocationManager.h"
+#import "MGLCompactCalloutView.h"
+
+@interface MGLTestCalloutView : MGLCompactCalloutView
+@property (nonatomic) BOOL implementsMarginHints;
+@end
+
+@implementation MGLTestCalloutView
+- (BOOL)respondsToSelector:(SEL)aSelector {
+    if (!self.implementsMarginHints &&
+        (aSelector == @selector(marginInsetsHintForPresentationFromRect:))) {
+        return NO;
+    }
+    return [super respondsToSelector:aSelector];
+}
+@end
+
 
 @interface MGLMapView (Tests)
 - (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)persist;
+@property (nonatomic) UIView<MGLCalloutView> *calloutViewForSelectedAnnotation;
 @end
 
 @interface MGLAnnotationViewIntegrationTests : MGLMapViewIntegrationTest
@@ -12,6 +29,368 @@
 
 @implementation MGLAnnotationViewIntegrationTests
 
+#pragma mark - Offscreen/panning selection tests
+
+typedef struct PanTestData {
+    CGPoint relativeCoord;
+    BOOL showsCallout;
+    BOOL implementsMargins;
+    BOOL moveIntoView;
+    BOOL expectMapToHavePanned;
+    BOOL calloutOnScreen;
+} PanTestData;
+
+#define PAN_TEST_TERMINATOR {{FLT_MAX, FLT_MAX}, NO, NO, NO, NO, NO}
+static const CGFloat kAnnotationScale = 0.125f;
+
+- (void)internalTestOffscreenSelectionTitle:(NSString*)title withTestData:(PanTestData)test animateSelection:(BOOL)animateSelection {
+    
+    CGPoint relativeCoordinate          = test.relativeCoord;
+    BOOL showsCallout                   = test.showsCallout;
+    BOOL calloutImplementsMarginHints   = test.implementsMargins;
+    BOOL moveIntoView                   = test.moveIntoView;
+    BOOL expectMapToHavePanned          = test.expectMapToHavePanned;
+    BOOL expectCalloutToBeFullyOnscreen = test.calloutOnScreen;
+    
+    // Reset the map to a consistent state - want the map to be zoomed in, so that
+    // it's free to be panned without hitting boundaries.
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(0, 0) zoomLevel:14 animated:NO];
+    [self waitForMapViewToBeRenderedWithTimeout:1.0];
+    
+    XCTAssert(self.mapView.annotations.count == 0);
+    
+    NSString * const MGLTestAnnotationReuseIdentifer = @"MGLTestAnnotationReuseIdentifer";
+    CGSize size = self.mapView.bounds.size;
+    CGSize annotationSize = CGSizeMake(floor(size.width*kAnnotationScale), floor(size.height*kAnnotationScale));
+    
+    self.viewForAnnotation = ^MGLAnnotationView*(MGLMapView *view, id<MGLAnnotation> annotation) {
+        
+        if (![annotation isKindOfClass:[MGLPointAnnotation class]]) {
+            return nil;
+        }
+        
+        // No dequeue
+        MGLAnnotationView *annotationView = [[MGLAnnotationView alloc] initWithAnnotation:annotation reuseIdentifier:MGLTestAnnotationReuseIdentifer];
+        annotationView.bounds             = (CGRect){ .origin = CGPointZero, .size = annotationSize };
+        annotationView.backgroundColor    = UIColor.redColor;
+        annotationView.enabled            = YES;
+        
+        return annotationView;
+    };
+    
+    // Coordinate for annotation screen coordinate
+    CGPoint annotationPoint = CGPointMake(relativeCoordinate.x * size.width, relativeCoordinate.y * size.height);
+    CLLocationCoordinate2D coordinate = [self.mapView convertPoint:annotationPoint toCoordinateFromView:self.mapView];
+    
+    MGLPointAnnotation *point = [[MGLPointAnnotation alloc] init];
+    point.title = title;
+    point.coordinate = coordinate;
+    
+    self.mapViewAnnotationCanShowCalloutForAnnotation = ^BOOL(MGLMapView *mapView, id<MGLAnnotation> annotation) {
+        return showsCallout;
+    };
+    
+    self.mapViewCalloutViewForAnnotation = ^id<MGLCalloutView>(MGLMapView *mapView, id<MGLAnnotation> annotation) {
+        if (!showsCallout)
+            return nil;
+        
+        MGLTestCalloutView *calloutView = [[MGLTestCalloutView alloc] init];
+        calloutView.representedObject = annotation;
+        calloutView.implementsMarginHints = calloutImplementsMarginHints;
+        return calloutView;
+    };
+    
+    [self.mapView addAnnotation:point];
+
+    // Check assumptions before selection
+    UIView *annotationViewBeforeSelection =  [self.mapView viewForAnnotation:point];
+    XCTAssertNotNil(annotationViewBeforeSelection);
+
+    CLLocationCoordinate2D mapCenterBeforeSelection = self.mapView.centerCoordinate;
+    XCTAssert(CLLocationCoordinate2DIsValid(mapCenterBeforeSelection));
+
+    // Also note, that at this point, the internal mechanism that determines if
+    // an annotation view is offscreen and should be put back in the reuse queue
+    // will have run, and `viewForAnnotation` may return nil
+    
+    [self.mapView selectAnnotation:point moveIntoView:moveIntoView animateSelection:animateSelection];
+
+    // Animated selection takes MGLAnimationDuration (0.3 seconds), so wait a little
+    // longer. We don't need to wait as long if we're not animated (but we do
+    // want the runloop to tick over)
+    [self waitFor:animateSelection ? 0.4: 0.05];
+
+    UIView *annotationViewAfterSelection =  [self.mapView viewForAnnotation:point];
+    CLLocationCoordinate2D mapCenterAfterSelection = self.mapView.centerCoordinate;
+    XCTAssert(CLLocationCoordinate2DIsValid(mapCenterAfterSelection));
+
+    // If the annotation is still "offscreen" at this point, then the above annotation view
+    // may be nil, which is expected.
+    BOOL (^CGRectContainsRectWithAccuracy)(CGRect, CGRect, CGFloat) = ^(CGRect rect1, CGRect rect2, CGFloat accuracy) {
+        CGRect expandedRect1 = CGRectInset(rect1, -accuracy, -accuracy);
+        return CGRectContainsRect(expandedRect1, rect2);
+    };
+    
+    CGFloat epsilon = 0.00001;
+    if (expectMapToHavePanned) {
+        CLLocationDegrees latitudeDelta = fabs(mapCenterAfterSelection.latitude - mapCenterBeforeSelection.latitude);
+        CLLocationDegrees longitudeDelta = fabs(mapCenterAfterSelection.longitude - mapCenterBeforeSelection.longitude);
+        
+        XCTAssert( (latitudeDelta > epsilon) || (longitudeDelta > epsilon), @"Deltas: lat=%f, long=%f", latitudeDelta, longitudeDelta); // One of them should have moved
+
+        // If the map panned - the intention is that the annotation is on-screen,
+        // and so should have an annotation view and that it is fully on screen
+        CGRect annotationFrameAfterSelection = annotationViewAfterSelection.frame;
+
+        XCTAssertNotNil(annotationViewAfterSelection);
+        
+        XCTAssert(CGRectContainsRectWithAccuracy(self.mapView.bounds, annotationFrameAfterSelection, 0.25), @"Mapview:%@ frame:%@", NSStringFromCGRect(self.mapView.bounds), NSStringFromCGRect(annotationFrameAfterSelection));
+        
+        // Check the callout
+        if (showsCallout) {
+            UIView *calloutView = self.mapView.calloutViewForSelectedAnnotation;
+            XCTAssertNotNil(calloutView);
+            
+            // If kAnnotationScale == 0.25, then the following assert can fail.
+            // This is really a warning (see https://github.com/mapbox/mapbox-gl-native/issues/13744 )
+            // If you need this NOT to fail the tests, consider replacing with MGLTestWarning
+            XCTAssert(expectCalloutToBeFullyOnscreen == CGRectContainsRectWithAccuracy(self.mapView.bounds, calloutView.frame, 0.25),
+                      @"Expect contains:%d, Mapview:%@ annotation:%@ callout:%@",
+                      expectCalloutToBeFullyOnscreen,
+                      NSStringFromCGRect(self.mapView.bounds),
+                      NSStringFromCGRect(annotationFrameAfterSelection),
+                      NSStringFromCGRect(calloutView.frame));
+        }
+    }
+    else {
+        // The map shouldn't have moved, so use equality (rather than an error check)
+        XCTAssertEqual(mapCenterBeforeSelection.latitude, mapCenterAfterSelection.latitude);
+        XCTAssertEqual(mapCenterBeforeSelection.longitude, mapCenterAfterSelection.longitude);
+        
+        // Annotation shouldn't have moved
+        CGPoint annotationPoint2 = [self.mapView convertCoordinate:point.coordinate toPointToView:self.mapView];
+        CGFloat xDelta = fabs(annotationPoint2.x - annotationPoint.x);
+        CGFloat yDelta = fabs(annotationPoint2.y - annotationPoint.y);
+        
+        XCTAssert((xDelta < epsilon) && (yDelta < epsilon));
+        
+        if (showsCallout) {
+            UIView *calloutView = self.mapView.calloutViewForSelectedAnnotation;
+            
+            if (annotationViewAfterSelection) {
+                XCTAssertNotNil(calloutView);
+
+                // If kAnnotationScale == 0.25, then the following assert can fail.
+                // This is really a warning (see https://github.com/mapbox/mapbox-gl-native/issues/13744 )
+                // If you need this NOT to fail the tests, consider replacing with MGLTestWarning
+                XCTAssert((expectCalloutToBeFullyOnscreen == CGRectContainsRectWithAccuracy(self.mapView.bounds, calloutView.frame, 0.25)),
+                          @"Mapview:%@ annotation:%@ callout:%@",
+                          NSStringFromCGRect(self.mapView.bounds),
+                          NSStringFromCGRect(annotationViewAfterSelection.frame),
+                          NSStringFromCGRect(calloutView.frame));
+            }
+            else {
+                // If there's no annotation view, should we expect a callout?
+                XCTAssertNil(calloutView);
+                XCTAssertFalse(expectCalloutToBeFullyOnscreen);
+            }
+        }
+    }
+   
+    // Remove the annotation
+    [self.mapView removeAnnotation:point];
+    
+    XCTAssert(self.mapView.annotations.count == 0);
+}
+
+// See https://github.com/mapbox/mapbox-gl-native/pull/13727#issuecomment-454028698
+// What follows are tests based on this table.
+// This is not a full-set of possible combinations, just the most important/likely
+// ones
+- (void)internalRunTests:(PanTestData*)testData
+{
+    // Test both animated and not-animated.
+    for (int i = 0; i<2; i++) {
+        int row = 0;
+        PanTestData *test = testData;
+        while (test->relativeCoord.x != FLT_MAX) {
+            NSString *activityTitle = [NSString stringWithFormat:@"Row %d/%d", row, i];
+            [XCTContext runActivityNamed:activityTitle
+                                   block:^(id<XCTActivity>  _Nonnull activity) {
+                                       [self internalTestOffscreenSelectionTitle:activityTitle
+                                                                    withTestData:*test
+                                                                animateSelection:!i];
+                                   }];
+            ++test;
+            ++row;
+        }
+    }
+}
+
+- (void)testBasicSelection  {
+    // Tests moveIntoView:NO
+    // WITHOUT a callout
+    
+    PanTestData tests[] = {
+        //  Coord           showsCallout    impl margins?   moveIntoView    expectMapToPan  calloutOnScreen
+        //  Offscreen
+        {   {-1.0f, 0.5f},  NO,             NO,             NO,             NO,             NO },
+        {   { 2.0f, 0.5f},  NO,             NO,             NO,             NO,             NO },
+        {   { 0.5f,-1.0f},  NO,             NO,             NO,             NO,             NO },
+        {   { 0.5f, 2.0f},  NO,             NO,             NO,             NO,             NO },
+        
+        //  Partial
+        {   { 0.0f, 0.5f},  NO,             NO,             NO,             NO,             NO },
+        {   { 1.0f, 0.5f},  NO,             NO,             NO,             NO,             NO },
+        {   { 0.5f, 0.0f},  NO,             NO,             NO,             NO,             NO },
+        {   { 0.5f, 1.0f},  NO,             NO,             NO,             NO,             NO },
+        
+        //  Onscreen
+        {   { 0.5f, 0.5f},  NO,             NO,             NO,             NO,             NO },
+        
+        PAN_TEST_TERMINATOR
+    };
+    
+    [self internalRunTests:tests];
+}
+
+- (void)testBasicSelectionWithCallout  {
+    // Tests moveIntoView:NO
+    // WITH the default callout (implements marginshint)
+    
+    PanTestData tests[] = {
+        //  Coord           showsCallout    impl margins?   moveIntoView    expectMapToPan  calloutOnScreen
+        {   {-1.0f, 0.5f},  YES,            YES,            NO,             NO,             NO },
+        {   { 0.0f, 0.5f},  YES,            YES,            NO,             NO,             NO },
+        {   { 0.5f, 1.0f},  YES,            YES,            NO,             NO,             YES }, // Because annotation was off the bottom of screen, and callout is above annotation
+        {   { 0.5f, 0.5f},  YES,            YES,            NO,             NO,             YES },
+        
+        PAN_TEST_TERMINATOR
+    };
+    
+    [self internalRunTests:tests];
+}
+
+- (void)testSelectionMoveIntoView  {
+    // Tests moveIntoView:YES
+    // without a callout
+    
+    // From https://github.com/mapbox/mapbox-gl-native/pull/13727#issuecomment-454028698
+    //
+    // | Annotation position | Has callout? | Callout implements `marginInsets...`? | Map pans when selected with moveIntoView=YES? |
+    // |---------------------|--------------|---------------------------------------|-----------------------------------------------|
+    // | Offscreen           | No           | n/a                                   | Yes (no margins)                              |
+    // | Partially           | No           | n/a                                   | No                                            |
+    // | Onscreen            | No           | n/a                                   | No                                            |
+    //
+    
+    PanTestData tests[] = {
+        //  Coord           showsCallout    impl margins?   moveIntoView     expectMapToPan  calloutOnScreen
+        //  Offscreen
+        {   {-1.0f, 0.5f},  NO,             NO,             YES,            YES,            NO },
+        {   { 2.0f, 0.5f},  NO,             NO,             YES,            YES,            NO },
+        {   { 0.5f,-1.0f},  NO,             NO,             YES,            YES,            NO },
+        {   { 0.5f, 2.0f},  NO,             NO,             YES,            YES,            NO },
+        
+        //  Partial
+        {   { 0.0f, 0.5f},  NO,             NO,             YES,            NO,             NO },
+        {   { 1.0f, 0.5f},  NO,             NO,             YES,            NO,             NO },
+        {   { 0.5f, 0.0f},  NO,             NO,             YES,            NO,             NO },
+        {   { 0.5f, 1.0f},  NO,             NO,             YES,            NO,             NO },
+        
+        //  Onscreen
+        {   { 0.5f, 0.5f},  NO,             NO,             YES,            NO,             NO },
+        
+        PAN_TEST_TERMINATOR
+    };
+    
+    [self internalRunTests:tests];
+}
+
+- (void)testSelectionMoveIntoViewWithCallout  {
+    // Tests moveIntoView:YES
+    // WITH the default callout (implements marginshint)
+    
+    // From https://github.com/mapbox/mapbox-gl-native/pull/13727#issuecomment-454028698
+    //
+    // | Annotation position | Has callout? | Callout implements `marginInsets...`? | Map pans when selected with moveIntoView=YES? |
+    // |---------------------|--------------|---------------------------------------|-----------------------------------------------|
+    // | Offscreen           | Yes          | Yes                                   | Yes to ensure callout is fully visible        |
+    // | Partially           | Yes          | Yes                                   | Yes to ensure callout is fully visible        |
+    // | Onscreen            | Yes          | Yes                                   | Yes, but *only* to ensure callout is fully visible |
+    //
+
+    PanTestData tests[] = {
+        //  Coord           showsCallout    impl margins?   moveIntoView    expectMapToPan  calloutOnScreen
+        //  Offscreen
+        {   {-1.0f, 0.5f},  YES,            YES,            YES,            YES,            YES },
+        {   { 2.0f, 0.5f},  YES,            YES,            YES,            YES,            YES },
+        {   { 0.5f,-1.0f},  YES,            YES,            YES,            YES,            YES },
+        {   { 0.5f, 2.0f},  YES,            YES,            YES,            YES,            YES },
+        
+        //  Partial
+        {   { 0.0f, 0.5f},  YES,            YES,            YES,            YES,            YES },
+        {   { 1.0f, 0.5f},  YES,            YES,            YES,            YES,            YES },
+        {   { 0.5f, 0.0f},  YES,            YES,            YES,            YES,            YES },
+        {   { 0.5f, 1.0f},  YES,            YES,            YES,            YES,            YES },
+        
+        //  Onscreen
+        {   { 0.5f, 0.5f},  YES,            YES,            YES,            NO,             YES },
+        
+        //  Just at the edge of the screen.
+        //  Expects to move, because although onscreen, callout would not be.
+        //  However, if the scale is 0.25, then expectToPan should be NO, because
+        //  of the width of the annotation
+        //
+        //  Coord                     showsCallout  impl margins?   moveIntoView    expectMapToPan                  calloutOnScreen
+        {   {kAnnotationScale, 0.5f}, YES,          YES,            YES,            (kAnnotationScale == 0.125f),   YES },
+
+        PAN_TEST_TERMINATOR
+    };
+    
+    [self internalRunTests:tests];
+}
+
+- (void)testSelectionMoveIntoViewWithBasicCallout  {
+    // Tests moveIntoView:YES
+    // WITH a callout that DOES NOT implement marginshint
+    
+    // From https://github.com/mapbox/mapbox-gl-native/pull/13727#issuecomment-454028698
+    //
+    // | Annotation position | Has callout? | Callout implements `marginInsets...`? | Map pans when selected with moveIntoView=YES? |
+    // |---------------------|--------------|---------------------------------------|-----------------------------------------------|
+    // | Offscreen           | Yes          | No                                    | Yes, but only to show annotation (not callout) with no margins |
+    // | Partially           | Yes          | No                                    | No                                            |
+    // | Onscreen            | Yes          | No                                    | No                                            |
+    //
+
+    PanTestData tests[] = {
+        //  Coord           showsCallout    impl margins?   moveIntoView    expectMapToPan  calloutOnScreen
+        //  Offscreen
+        {   {-1.0f, 0.5f},  YES,            NO,             YES,            YES,            NO },
+        {   { 2.0f, 0.5f},  YES,            NO,             YES,            YES,            NO },
+        {   { 0.5f,-1.0f},  YES,            NO,             YES,            YES,            NO },
+        {   { 0.5f, 2.0f},  YES,            NO,             YES,            YES,            YES },   // Because annotation was off the bottom of screen, and callout is above annotation
+        {   { 2.0f, 2.0f},  YES,            NO,             YES,            YES,            NO },
+
+        //  Partial
+        {   { 0.0f, 0.5f},  YES,            NO,             YES,            NO,             NO },
+        {   { 1.0f, 0.5f},  YES,            NO,             YES,            NO,             NO },
+        {   { 0.5f, 0.0f},  YES,            NO,             YES,            NO,             NO },
+        {   { 0.5f, 1.0f},  YES,            NO,             YES,            NO,             YES }, // Because annotation was off the bottom of screen, and callout is above annotation
+        {   { 1.0f, 1.0f},  YES,            NO,             YES,            NO,             NO },
+
+        //  Onscreen
+        {   { 0.5f, 0.5f},  YES,            NO,             YES,            NO,             YES },
+
+        PAN_TEST_TERMINATOR
+    };
+    
+    [self internalRunTests:tests];
+}
+
+#pragma mark - Selection with an offset
+
 - (void)testSelectingAnnotationWithCenterOffset {
 
     for (CGFloat dx = -100.0; dx <= 100.0; dx += 100.0 ) {
@@ -66,8 +445,8 @@
 
     // Check that the annotation is in the center of the view
     CGPoint annotationPoint = [self.mapView convertCoordinate:point.coordinate toPointToView:self.mapView];
-    XCTAssertEqualWithAccuracy(annotationPoint.x, size.width/2.0, epsilon);
-    XCTAssertEqualWithAccuracy(annotationPoint.y, size.height/2.0, epsilon);
+    XCTAssertEqualWithAccuracy(annotationPoint.x, size.width/2.0, 0.25);
+    XCTAssertEqualWithAccuracy(annotationPoint.y, size.height/2.0, 0.25);
 
     // Now test taps around the annotation
     CGPoint tapPoint = CGPointMake(annotationPoint.x + offset.dx, annotationPoint.y + offset.dy);
@@ -117,6 +496,30 @@
     XCTAssertEqual(originalFrame.origin.y + offset.y, offsetFrame.origin.y);
 }
 
+#pragma mark - Utilities
+
+- (void)runRunLoop {
+    [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantFuture]];
+}
+
+- (void)waitFor:(NSTimeInterval)seconds {
+    XCTestExpectation *timerExpired = [self expectationWithDescription:@"Timer expires"];
+    
+    NSTimer *timer = [NSTimer scheduledTimerWithTimeInterval:0.1
+                                                      target:self
+                                                    selector:@selector(runRunLoop)
+                                                    userInfo:nil
+                                                     repeats:YES];
+
+    double duration = seconds * (double)NSEC_PER_SEC;
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)duration), dispatch_get_main_queue(), ^{
+        [timerExpired fulfill];
+    });
+    
+    [self waitForExpectations:@[timerExpired] timeout:seconds + 1.0];
+    [timer invalidate];
+}
+
 - (void)waitForCollisionDetectionToRun {
     XCTAssertNil(self.renderFinishedExpectation, @"Incorrect test setup");
 
@@ -129,6 +532,8 @@
     });
 
     [self waitForExpectations:@[timerExpired, self.renderFinishedExpectation] timeout:5];
+    
+    self.renderFinishedExpectation = nil;
 }
 
 @end
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
index 2f459a5f44..dedafdb83a 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.h
@@ -16,6 +16,13 @@
 #define MGLTestAssertNotNil(myself, expression, ...) \
     _XCTPrimitiveAssertNotNil(myself, expression, @#expression, __VA_ARGS__)
 
+#define MGLTestWarning(expression, format, ...) \
+({ \
+    if (!(expression))  { \
+        NSString *message = [NSString stringWithFormat:format, ##__VA_ARGS__]; \
+        printf("warning: Test Case '%s' at line %d: '%s' %s\n", __PRETTY_FUNCTION__, __LINE__, #expression, message.UTF8String); \
+    } \
+})
 
 @interface MGLMapViewIntegrationTest : XCTestCase <MGLMapViewDelegate>
 @property (nonatomic) MGLMapView *mapView;
@@ -27,6 +34,8 @@
 @property (nonatomic) void (^regionIsChanging)(MGLMapView *mapView);
 @property (nonatomic) void (^regionDidChange)(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated);
 @property (nonatomic) CGPoint (^mapViewUserLocationAnchorPoint)(MGLMapView *mapView);
+@property (nonatomic) BOOL (^mapViewAnnotationCanShowCalloutForAnnotation)(MGLMapView *mapView, id<MGLAnnotation> annotation);
+@property (nonatomic) id<MGLCalloutView> (^mapViewCalloutViewForAnnotation)(MGLMapView *mapView, id<MGLAnnotation> annotation);
 
 // Utility methods
 - (NSString*)validAccessToken;
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
index 0138a255ec..77e678d3ef 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
@@ -108,6 +108,20 @@
     return CGPointZero;
 }
 
+- (BOOL)mapView:(MGLMapView *)mapView annotationCanShowCallout:(id<MGLAnnotation>)annotation {
+    if (self.mapViewAnnotationCanShowCalloutForAnnotation) {
+        return self.mapViewAnnotationCanShowCalloutForAnnotation(mapView, annotation);
+    }
+    return NO;
+}
+
+- (id<MGLCalloutView>)mapView:(MGLMapView *)mapView calloutViewForAnnotation:(id<MGLAnnotation>)annotation {
+    if (self.mapViewCalloutViewForAnnotation) {
+        return self.mapViewCalloutViewForAnnotation(mapView, annotation);
+    }
+    return nil;
+}
+
 #pragma mark - Utilities
 
 - (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout {
diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj
index 2c4d7f2eab..0945d08cc8 100644
--- a/platform/ios/ios.xcodeproj/project.pbxproj
+++ b/platform/ios/ios.xcodeproj/project.pbxproj
@@ -474,6 +474,8 @@
 		CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA0C27932076CA19001CE5B7 /* MGLMapViewIntegrationTest.m */; };
 		CA1B4A512099FB2200EDD491 /* MGLMapSnapshotterTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA1B4A502099FB2200EDD491 /* MGLMapSnapshotterTest.m */; };
 		CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA34C9C2207FD272005C1A06 /* MGLCameraTransitionTests.mm */; };
+		CA45105A21EE2989000C37D5 /* MGLCompactCalloutView.m in Sources */ = {isa = PBXBuildFile; fileRef = DA8848451CBAFB9800AB86E3 /* MGLCompactCalloutView.m */; };
+		CA45105B21EE29B3000C37D5 /* SMCalloutView.m in Sources */ = {isa = PBXBuildFile; fileRef = DA88488A1CBB037E00AB86E3 /* SMCalloutView.m */; };
 		CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA4EB8C620863487006AB465 /* MGLStyleLayerIntegrationTests.m */; };
 		CA55CD41202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		CA55CD42202C16AA00CE7095 /* MGLCameraChangeReason.h in Headers */ = {isa = PBXBuildFile; fileRef = CA55CD3E202C16AA00CE7095 /* MGLCameraChangeReason.h */; settings = {ATTRIBUTES = (Public, ); }; };
@@ -3034,10 +3036,12 @@
 			buildActionMask = 2147483647;
 			files = (
 				CA4EB8C720863487006AB465 /* MGLStyleLayerIntegrationTests.m in Sources */,
+				CA45105B21EE29B3000C37D5 /* SMCalloutView.m in Sources */,
 				CA34C9C3207FD272005C1A06 /* MGLCameraTransitionTests.mm in Sources */,
 				16376B0A1FFD9DAF0000563E /* MBGLIntegrationTests.m in Sources */,
 				CA88DC3021C85D900059ED5A /* MGLStyleURLIntegrationTest.m in Sources */,
 				CA0C27942076CA19001CE5B7 /* MGLMapViewIntegrationTest.m in Sources */,
+				CA45105A21EE2989000C37D5 /* MGLCompactCalloutView.m in Sources */,
 				CAE7AD5520F46EF5003B6782 /* MGLMapSnapshotterSwiftTests.swift in Sources */,
 				CA0C27922076C804001CE5B7 /* MGLShapeSourceTests.m in Sources */,
 				077061DA215DA00E000FEF62 /* MGLTestLocationManager.m in Sources */,
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index d188ffb91f..258f6eea87 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -152,9 +152,6 @@ const CGFloat MGLAnnotationImagePaddingForCallout = 1;
 
 const CGSize MGLAnnotationAccessibilityElementMinimumSize = CGSizeMake(10, 10);
 
-/// Padding to edge of view that an offscreen annotation must have when being brought onscreen (by being selected)
-const UIEdgeInsets MGLMapViewOffscreenAnnotationPadding = UIEdgeInsetsMake(-20.0f, -20.0f, -20.0f, -20.0f);
-
 /// An indication that the requested annotation was not found or is nonexistent.
 enum { MGLAnnotationTagNotFound = UINT32_MAX };
 
@@ -229,6 +226,11 @@ public:
 
 /// Currently shown popover representing the selected annotation.
 @property (nonatomic) UIView<MGLCalloutView> *calloutViewForSelectedAnnotation;
+
+/// Anchor coordinate from which to present callout views (for example, for shapes this
+/// could be the touch point rather than its centroid)
+@property (nonatomic) CLLocationCoordinate2D anchorCoordinateForSelectedAnnotation;
+
 @property (nonatomic) MGLUserLocationAnnotationView *userLocationAnnotationView;
 
 /// Indicates how thoroughly the map view is tracking the user location.
@@ -4537,7 +4539,7 @@ public:
 
 - (BOOL)isMovingAnnotationIntoViewSupportedForAnnotation:(id<MGLAnnotation>)annotation animated:(BOOL)animated {
     // Consider delegating
-    return animated && [annotation isKindOfClass:[MGLPointAnnotation class]];
+    return [annotation isKindOfClass:[MGLPointAnnotation class]];
 }
 
 - (id <MGLAnnotation>)selectedAnnotation
@@ -4644,12 +4646,15 @@ public:
         CGPoint originPoint = [self convertCoordinate:origin toPointToView:self];
         calloutPositioningRect = { .origin = originPoint, .size = CGSizeZero };
     }
-
-    CGRect expandedPositioningRect = UIEdgeInsetsInsetRect(calloutPositioningRect, MGLMapViewOffscreenAnnotationPadding);
+    
+    CGRect expandedPositioningRect = calloutPositioningRect;
 
     // Used for callout positioning, and moving offscreen annotations onscreen.
-    CGRect constrainedRect = UIEdgeInsetsInsetRect(self.bounds, self.contentInset);
+    CGRect constrainedRect = self.contentFrame;
+    CGRect bounds = constrainedRect;
 
+    BOOL expandedPositioningRectToMoveCalloutIntoViewWithMargins = NO;
+    
     UIView <MGLCalloutView> *calloutView = nil;
 
     if ([annotation respondsToSelector:@selector(title)] &&
@@ -4720,37 +4725,55 @@ public:
         if (moveIntoView && [calloutView respondsToSelector:@selector(marginInsetsHintForPresentationFromRect:)]) {
             UIEdgeInsets margins = [calloutView marginInsetsHintForPresentationFromRect:calloutPositioningRect];
             expandedPositioningRect = UIEdgeInsetsInsetRect(expandedPositioningRect, margins);
+            expandedPositioningRectToMoveCalloutIntoViewWithMargins = YES;
         }
     }
+    
+    if (!expandedPositioningRectToMoveCalloutIntoViewWithMargins)
+    {
+        // We don't have a callout (OR our callout didn't implement
+        // marginInsetsHintForPresentationFromRect: - in this case we need to
+        // ensure that partially off-screen annotations are NOT moved into view.
+        //
+        // We may want to create (and fallback to) an `MGLMapViewDelegate` version
+        // of the `-[MGLCalloutView marginInsetsHintForPresentationFromRect:]
+        // protocol method.
+        bounds = CGRectInset(bounds, -calloutPositioningRect.size.width, -calloutPositioningRect.size.height);
+    }        
 
     if (moveIntoView)
     {
         moveIntoView = NO;
 
-        // Need to consider the content insets.
-        CGRect bounds = UIEdgeInsetsInsetRect(self.bounds, self.contentInset);
-
         // Any one of these cases should trigger a move onscreen
-        if (CGRectGetMinX(calloutPositioningRect) < 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(calloutPositioningRect) > 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(calloutPositioningRect) < 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(calloutPositioningRect) > 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;
+            }
         }
     }
 
@@ -4760,6 +4783,17 @@ public:
                       constrainedToRect:constrainedRect
                                animated:animateSelection];
 
+    // Save the anchor coordinate
+    if ([annotation isKindOfClass:[MGLPointAnnotation class]]) {
+        self.anchorCoordinateForSelectedAnnotation = annotation.coordinate;
+    }
+    else {
+        // This is used for features like polygons, so that if the map is dragged
+        // the callout doesn't ping to its coordinate.
+        CGPoint anchorPoint = CGPointMake(CGRectGetMidX(calloutPositioningRect), CGRectGetMidY(calloutPositioningRect));
+        self.anchorCoordinateForSelectedAnnotation = [self convertPoint:anchorPoint toCoordinateFromView:self];
+    }
+        
     // notify delegate
     if ([self.delegate respondsToSelector:@selector(mapView:didSelectAnnotation:)])
     {
@@ -4824,8 +4858,18 @@ public:
         return CGRectNull;
     }
     
+    CLLocationCoordinate2D coordinate;
+    
+    if ((annotation == self.selectedAnnotation) &&
+        CLLocationCoordinate2DIsValid(self.anchorCoordinateForSelectedAnnotation)) {
+        coordinate = self.anchorCoordinateForSelectedAnnotation;
+    }
+    else {
+        coordinate = annotation.coordinate;
+    }
+    
     if ([annotation isKindOfClass:[MGLMultiPoint class]]) {
-        CLLocationCoordinate2D origin = annotation.coordinate;
+        CLLocationCoordinate2D origin = coordinate;
         CGPoint originPoint = [self convertCoordinate:origin toPointToView:self];
         return CGRectMake(originPoint.x, originPoint.y, MGLAnnotationImagePaddingForHitTest, MGLAnnotationImagePaddingForHitTest);
     }
@@ -4840,7 +4884,7 @@ public:
         return CGRectZero;
     }
 
-    CGRect positioningRect = [self frameOfImage:image centeredAtCoordinate:annotation.coordinate];
+    CGRect positioningRect = [self frameOfImage:image centeredAtCoordinate:coordinate];
     positioningRect.origin.x -= 0.5;
 
     return CGRectInset(positioningRect, -MGLAnnotationImagePaddingForCallout,
@@ -4895,6 +4939,7 @@ public:
         // clean up
         self.calloutViewForSelectedAnnotation = nil;
         self.selectedAnnotation = nil;
+        self.anchorCoordinateForSelectedAnnotation = kCLLocationCoordinate2DInvalid;
 
         // notify delegate
         if ([self.delegate respondsToSelector:@selector(mapView:didDeselectAnnotation:)])
@@ -6158,7 +6203,9 @@ public:
             annotationView = self.userLocationAnnotationView;
         }
 
-        CGRect positioningRect = annotationView ? annotationView.frame : [self positioningRectForCalloutForAnnotationWithTag:tag];
+        CGRect positioningRect = annotationView ?
+            annotationView.frame :
+            [self positioningRectForCalloutForAnnotationWithTag:tag];
 
         MGLAssert( ! CGRectIsNull(positioningRect), @"Positioning rect should not be CGRectNull by this point");
 
diff --git a/platform/ios/vendor/SMCalloutView/SMCalloutView.m b/platform/ios/vendor/SMCalloutView/SMCalloutView.m
index 7f987c3355..66a7b5a7a7 100755
--- a/platform/ios/vendor/SMCalloutView/SMCalloutView.m
+++ b/platform/ios/vendor/SMCalloutView/SMCalloutView.m
@@ -269,6 +269,8 @@ NSTimeInterval const kMGLSMCalloutViewRepositionDelayForUIScrollView = 1.0/3.0;
 
 - (UIEdgeInsets)marginInsetsHintForPresentationFromRect:(CGRect)rect {
 
+    const CGFloat defaultMargin = 20.0f;
+    
     // form our subviews based on our content set so far
     [self rebuildSubviews];
 
@@ -281,16 +283,16 @@ NSTimeInterval const kMGLSMCalloutViewRepositionDelayForUIScrollView = 1.0/3.0;
     CGFloat horizontalMargin = fmaxf(0, ceilf((CALLOUT_MIN_WIDTH-rect.size.width)/2));
 
     UIEdgeInsets insets = {
-        .top = 0.0f,
-        .right = -horizontalMargin,
+        .top    = 0.0f,
+        .right  = -defaultMargin - horizontalMargin,
         .bottom = 0.0f,
-        .left = -horizontalMargin
+        .left   = -defaultMargin - horizontalMargin
     };
 
     if (self.permittedArrowDirection == MGLSMCalloutArrowDirectionUp)
-        insets.bottom -= size.height;
+        insets.bottom -= (defaultMargin + size.height);
     else
-        insets.top -= size.height;
+        insets.top -= (defaultMargin + size.height);
 
     return insets;
 }
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;
 }