summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulian Rex <julian.rex@gmail.com>2018-04-24 11:01:04 -0400
committerGitHub <noreply@github.com>2018-04-24 11:01:04 -0400
commit5b5549d644d026e093243b13e0770b9d34172ef9 (patch)
tree58aa4bc9cbc7cd78f00f4b2aed38570a21568717
parentfa6c6a3733323e855c1b825c9546093200cf0d62 (diff)
downloadqtlocation-mapboxgl-5b5549d644d026e093243b13e0770b9d34172ef9.tar.gz
[ios, macos] Fix for camera movement when selecting visible annotations (#11731)
-rw-r--r--platform/ios/ios.xcodeproj/xcshareddata/xcschemes/CI.xcscheme10
-rw-r--r--platform/ios/src/MGLMapView.mm21
-rw-r--r--platform/ios/test/MGLAnnotationViewTests.m89
-rw-r--r--platform/macos/macos.xcodeproj/project.pbxproj4
-rw-r--r--platform/macos/src/MGLMapView.mm12
-rw-r--r--platform/macos/test/MGLAnnotationTests.m52
6 files changed, 182 insertions, 6 deletions
diff --git a/platform/ios/ios.xcodeproj/xcshareddata/xcschemes/CI.xcscheme b/platform/ios/ios.xcodeproj/xcshareddata/xcschemes/CI.xcscheme
index 4679378126..afba168676 100644
--- a/platform/ios/ios.xcodeproj/xcshareddata/xcschemes/CI.xcscheme
+++ b/platform/ios/ios.xcodeproj/xcshareddata/xcschemes/CI.xcscheme
@@ -54,7 +54,6 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
- language = ""
shouldUseLaunchSchemeArgsEnv = "YES">
<Testables>
<TestableReference
@@ -66,6 +65,14 @@
BlueprintName = "test"
ReferencedContainer = "container:ios.xcodeproj">
</BuildableReference>
+ <SkippedTests>
+ <Test
+ Identifier = "MGLAnnotationViewTests/testAnnotationViewInitWithFrame">
+ </Test>
+ <Test
+ Identifier = "MGLAnnotationViewTests/testSelectingADisabledAnnotationView">
+ </Test>
+ </SkippedTests>
</TestableReference>
</Testables>
<MacroExpansion>
@@ -84,7 +91,6 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
- language = ""
launchStyle = "0"
useCustomWorkingDirectory = "NO"
ignoresPersistentStateOnLaunch = "NO"
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index b7d0974872..c679979d37 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -4290,6 +4290,15 @@ public:
moveOnscreen = [self isBringingAnnotationOnscreenSupportedForAnnotation:annotation animated:animateSelection];
}
+ // If we have an invalid positioning rect, we need to provide a suitable default.
+ // This (currently) happens if you select an annotation that has NOT yet been
+ // added. See https://github.com/mapbox/mapbox-gl-native/issues/11476
+ if (CGRectIsNull(calloutPositioningRect)) {
+ CLLocationCoordinate2D origin = annotation.coordinate;
+ CGPoint originPoint = [self convertCoordinate:origin toPointToView:self];
+ calloutPositioningRect = { .origin = originPoint, .size = CGSizeZero };
+ }
+
CGRect expandedPositioningRect = UIEdgeInsetsInsetRect(calloutPositioningRect, MGLMapViewOffscreenAnnotationPadding);
// Used for callout positioning, and moving offscreen annotations onscreen.
@@ -4442,7 +4451,11 @@ public:
{
MGLAnnotationTag annotationTag = [self annotationTagForAnnotation:annotation];
CGRect positioningRect = [self positioningRectForCalloutForAnnotationWithTag:annotationTag];
-
+
+ if (CGRectIsNull(positioningRect)) {
+ return positioningRect;
+ }
+
// For annotations which `coordinate` falls offscreen it will use the current tap point as anchor instead.
if ( ! CGRectIntersectsRect(positioningRect, self.bounds) && annotation != self.userLocation)
{
@@ -4462,15 +4475,15 @@ public:
id <MGLAnnotation> annotation = [self annotationWithTag:annotationTag];
if ( ! annotation)
{
- return CGRectZero;
+ return CGRectNull;
}
if ([annotation isKindOfClass:[MGLMultiPoint class]]) {
CLLocationCoordinate2D origin = annotation.coordinate;
CGPoint originPoint = [self convertCoordinate:origin toPointToView:self];
return CGRectMake(originPoint.x, originPoint.y, MGLAnnotationImagePaddingForHitTest, MGLAnnotationImagePaddingForHitTest);
-
}
+
UIImage *image = [self imageOfAnnotationWithTag:annotationTag].image;
if ( ! image)
{
@@ -5743,6 +5756,8 @@ public:
rect = annotationView.frame;
}
+ NSAssert(!CGRectIsNull(rect), @"Positioning rect should not be CGRectNull by this point");
+
CGPoint point = CGPointMake(CGRectGetMidX(rect), CGRectGetMinY(rect));
if ( ! CGPointEqualToPoint(calloutView.center, point)) {
diff --git a/platform/ios/test/MGLAnnotationViewTests.m b/platform/ios/test/MGLAnnotationViewTests.m
index 2f5963e66e..a2cc4227ed 100644
--- a/platform/ios/test/MGLAnnotationViewTests.m
+++ b/platform/ios/test/MGLAnnotationViewTests.m
@@ -3,6 +3,13 @@
static NSString * const MGLTestAnnotationReuseIdentifer = @"MGLTestAnnotationReuseIdentifer";
+
+@interface MGLMapView (Tests)
+@property (nonatomic) MGLCameraChangeReason cameraChangeReasonBitmask;
+@end
+
+
+
@interface MGLCustomAnnotationView : MGLAnnotationView
@end
@@ -58,6 +65,7 @@ static NSString * const MGLTestAnnotationReuseIdentifer = @"MGLTestAnnotationReu
@property (nonatomic) MGLMapView *mapView;
@property (nonatomic, weak) MGLAnnotationView *annotationView;
@property (nonatomic) NSInteger annotationSelectedCount;
+@property (nonatomic) void (^prepareAnnotationView)(MGLAnnotationView*);
@end
@implementation MGLAnnotationViewTests
@@ -152,6 +160,83 @@ static NSString * const MGLTestAnnotationReuseIdentifer = @"MGLTestAnnotationReu
XCTAssertEqual(selectionCount, self.annotationSelectedCount, @"-mapView:didSelectAnnotation: should be called for each selection");
}
+- (void)testSelectingOnscreenAnnotationThatHasNotBeenAdded {
+ // See https://github.com/mapbox/mapbox-gl-native/issues/11476
+
+ // This bug occurs under the following conditions:
+ //
+ // - There are content insets (e.g. navigation bar) for the compare against
+ // CGRectZero (now CGRectNull)
+ // - annotationView.enabled == NO - Currently this can happen if you use
+ // `-initWithFrame:` rather than one of the provided initializers
+ //
+
+ self.prepareAnnotationView = ^(MGLAnnotationView *view) {
+ view.enabled = NO;
+ };
+
+ self.mapView.contentInset = UIEdgeInsetsMake(10.0, 10.0, 10.0, 10.0);
+
+ MGLCameraChangeReason reasonBefore = self.mapView.cameraChangeReasonBitmask;
+ XCTAssert(reasonBefore == MGLCameraChangeReasonNone, @"Camera should not have moved at start of test");
+
+ // Create annotation
+ MGLPointFeature *point = [[MGLPointFeature alloc] init];
+ point.title = NSStringFromSelector(_cmd);
+ point.coordinate = CLLocationCoordinate2DMake(0.0, 0.0);
+
+ MGLCoordinateBounds coordinateBounds = [self.mapView convertRect:self.mapView.bounds toCoordinateBoundsFromView:self.mapView];
+ XCTAssert(MGLCoordinateInCoordinateBounds(point.coordinate, coordinateBounds), @"The test point should be within the visible map view");
+
+ // Select on screen annotation (DO NOT ADD FIRST).
+ [self.mapView selectAnnotation:point animated:YES];
+
+ // Expect - the camera NOT to move.
+ MGLCameraChangeReason reasonAfter = self.mapView.cameraChangeReasonBitmask;
+ XCTAssert(reasonAfter == MGLCameraChangeReasonNone, @"Camera should not have moved");
+}
+
+- (void)checkDefaultPropertiesForAnnotationView:(MGLAnnotationView*)view {
+ XCTAssertNil(view.annotation);
+ XCTAssertNil(view.reuseIdentifier);
+ XCTAssertEqual(view.centerOffset.dx, 0.0);
+ XCTAssertEqual(view.centerOffset.dy, 0.0);
+ XCTAssertFalse(view.scalesWithViewingDistance);
+ XCTAssertFalse(view.rotatesToMatchCamera);
+ XCTAssertFalse(view.isSelected);
+ XCTAssert(view.isEnabled);
+ XCTAssertFalse(view.isDraggable);
+ XCTAssertEqual(view.dragState, MGLAnnotationViewDragStateNone);
+}
+
+- (void)testAnnotationViewInitWithFrame {
+ CGRect frame = CGRectMake(10.0, 10.0, 100.0, 100.0);
+ MGLAnnotationView *view = [[MGLAnnotationView alloc] initWithFrame:frame];
+ [self checkDefaultPropertiesForAnnotationView:view];
+}
+
+- (void)testAnnotationViewInitWithReuseIdentifier {
+ MGLAnnotationView *view = [[MGLAnnotationView alloc] initWithReuseIdentifier:nil];
+ [self checkDefaultPropertiesForAnnotationView:view];
+}
+
+- (void)testSelectingADisabledAnnotationView {
+ self.prepareAnnotationView = ^(MGLAnnotationView *view) {
+ view.enabled = NO;
+ };
+
+ // Create annotation
+ MGLPointFeature *point = [[MGLPointFeature alloc] init];
+ point.title = NSStringFromSelector(_cmd);
+ point.coordinate = CLLocationCoordinate2DMake(0.0, 0.0);
+
+ XCTAssert(self.mapView.selectedAnnotations.count == 0, @"There should be 0 selected annotations");
+
+ [self.mapView selectAnnotation:point animated:NO];
+
+ XCTAssert(self.mapView.selectedAnnotations.count == 0, @"There should be 0 selected annotations");
+}
+
#pragma mark - MGLMapViewDelegate -
- (MGLAnnotationView *)mapView:(MGLMapView *)mapView viewForAnnotation:(id<MGLAnnotation>)annotation
@@ -163,6 +248,10 @@ static NSString * const MGLTestAnnotationReuseIdentifer = @"MGLTestAnnotationReu
annotationView = [[MGLAnnotationView alloc] initWithAnnotation:annotation reuseIdentifier:MGLTestAnnotationReuseIdentifer];
}
+ if (self.prepareAnnotationView) {
+ self.prepareAnnotationView(annotationView);
+ }
+
_annotationView = annotationView;
return annotationView;
diff --git a/platform/macos/macos.xcodeproj/project.pbxproj b/platform/macos/macos.xcodeproj/project.pbxproj
index d49e5e6f06..6a97794d85 100644
--- a/platform/macos/macos.xcodeproj/project.pbxproj
+++ b/platform/macos/macos.xcodeproj/project.pbxproj
@@ -95,6 +95,7 @@
9654C12B1FFC38E000DB6A19 /* MGLPolyline_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 9654C12A1FFC38E000DB6A19 /* MGLPolyline_Private.h */; };
9654C12D1FFC394700DB6A19 /* MGLPolygon_Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 9654C12C1FFC394700DB6A19 /* MGLPolygon_Private.h */; };
96E027311E57C9A7004B8E66 /* Localizable.strings in Resources */ = {isa = PBXBuildFile; fileRef = 96E027331E57C9A7004B8E66 /* Localizable.strings */; };
+ CA9461A620884CCB0015EB12 /* MGLAnnotationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA9461A520884CCB0015EB12 /* MGLAnnotationTests.m */; };
DA00FC8A1D5EEAC3009AABC8 /* MGLAttributionInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = DA00FC881D5EEAC3009AABC8 /* MGLAttributionInfo.h */; settings = {ATTRIBUTES = (Public, ); }; };
DA00FC8B1D5EEAC3009AABC8 /* MGLAttributionInfo.mm in Sources */ = {isa = PBXBuildFile; fileRef = DA00FC891D5EEAC3009AABC8 /* MGLAttributionInfo.mm */; };
DA0CD58E1CF56F5800A5F5A5 /* MGLFeatureTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = DA0CD58D1CF56F5800A5F5A5 /* MGLFeatureTests.mm */; };
@@ -394,6 +395,7 @@
96E027391E57C9B9004B8E66 /* sv */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sv; path = sv.lproj/Localizable.strings; sourceTree = "<group>"; };
96E0273A1E57C9BB004B8E66 /* vi */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = vi; path = vi.lproj/Localizable.strings; sourceTree = "<group>"; };
96E0273B1E57C9BC004B8E66 /* pt-BR */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "pt-BR"; path = "pt-BR.lproj/Localizable.strings"; sourceTree = "<group>"; };
+ CA9461A520884CCB0015EB12 /* MGLAnnotationTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationTests.m; path = test/MGLAnnotationTests.m; sourceTree = SOURCE_ROOT; };
DA00FC881D5EEAC3009AABC8 /* MGLAttributionInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLAttributionInfo.h; sourceTree = "<group>"; };
DA00FC891D5EEAC3009AABC8 /* MGLAttributionInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MGLAttributionInfo.mm; sourceTree = "<group>"; };
DA0CD58D1CF56F5800A5F5A5 /* MGLFeatureTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLFeatureTests.mm; path = ../../darwin/test/MGLFeatureTests.mm; sourceTree = "<group>"; };
@@ -1076,6 +1078,7 @@
4031AD001E9FD61000A3EA26 /* Test Helpers */,
4031ACFA1E9EB39A00A3EA26 /* Swift Integration */,
DA8F257D1D51C5F40010E6B5 /* Styling */,
+ CA9461A520884CCB0015EB12 /* MGLAnnotationTests.m */,
DAEDC4311D6033F1000224FF /* MGLAttributionInfoTests.m */,
DAEDC4361D606291000224FF /* MGLAttributionButtonTests.m */,
DA35A2C11CCA9F4A00E826B2 /* MGLClockDirectionFormatterTests.m */,
@@ -1601,6 +1604,7 @@
DAE6C3D31CC34C9900DB3429 /* MGLOfflinePackTests.m in Sources */,
DA87A9A51DCACC5000810D09 /* MGLLineStyleLayerTests.mm in Sources */,
DA87A9A31DCACC5000810D09 /* MGLRasterStyleLayerTests.mm in Sources */,
+ CA9461A620884CCB0015EB12 /* MGLAnnotationTests.m in Sources */,
DA87A9991DC9D88400810D09 /* MGLTileSetTests.mm in Sources */,
DA35A2A81CC9F41600E826B2 /* MGLCoordinateFormatterTests.m in Sources */,
DAE7DEC41E24549F007505A6 /* MGLNSStringAdditionsTests.m in Sources */,
diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm
index 9cab9a76da..9a9e19c363 100644
--- a/platform/macos/src/MGLMapView.mm
+++ b/platform/macos/src/MGLMapView.mm
@@ -2249,8 +2249,16 @@ public:
// The annotation's anchor will bounce to the current click.
NSRect positioningRect = [self positioningRectForCalloutForAnnotationWithTag:annotationTag];
+ // Check for invalid (zero) positioning rect
+ if (NSEqualRects(positioningRect, NSZeroRect)) {
+ CLLocationCoordinate2D origin = annotation.coordinate;
+ positioningRect.origin = [self convertCoordinate:origin toPointToView:self];
+ }
+
if (!moveOnscreen && NSIsEmptyRect(NSIntersectionRect(positioningRect, self.bounds))) {
- positioningRect = CGRectMake(gesturePoint.x, gesturePoint.y, positioningRect.size.width, positioningRect.size.height);
+ if (!NSEqualPoints(gesturePoint, NSZeroPoint)) {
+ positioningRect = CGRectMake(gesturePoint.x, gesturePoint.y, positioningRect.size.width, positioningRect.size.height);
+ }
}
self.selectedAnnotation = annotation;
@@ -2464,6 +2472,8 @@ public:
if (callout) {
NSRect rect = [self positioningRectForCalloutForAnnotationWithTag:_selectedAnnotationTag];
+ NSAssert(!NSEqualRects(rect, NSZeroRect), @"Positioning rect should be non-zero");
+
if (!NSIsEmptyRect(NSIntersectionRect(rect, self.bounds))) {
// It's possible that the current callout hasn't been presented (since the original
diff --git a/platform/macos/test/MGLAnnotationTests.m b/platform/macos/test/MGLAnnotationTests.m
new file mode 100644
index 0000000000..36a7aef9f0
--- /dev/null
+++ b/platform/macos/test/MGLAnnotationTests.m
@@ -0,0 +1,52 @@
+#import <Mapbox/Mapbox.h>
+#import <XCTest/XCTest.h>
+
+@interface MGLAnnotationTests : XCTestCase <MGLMapViewDelegate>
+@property (nonatomic) MGLMapView *mapView;
+@property (nonatomic) BOOL centerCoordinateDidChange;
+@end
+
+@implementation MGLAnnotationTests
+
+- (void)setUp
+{
+ [super setUp];
+ _mapView = [[MGLMapView alloc] initWithFrame:CGRectMake(0, 0, 64, 64)];
+ _mapView.delegate = self;
+}
+
+- (void)testSelectingOnscreenAnnotationThatHasNotBeenAdded {
+ // See https://github.com/mapbox/mapbox-gl-native/issues/11476
+
+ // This bug occurs under the following conditions:
+ //
+ // - There are content insets (e.g. navigation bar) for the compare against
+ // NSZeroRect
+
+ self.mapView.contentInsets = NSEdgeInsetsMake(10.0, 10.0, 10.0, 10.0);
+
+ // Create annotation
+ MGLPointFeature *point = [[MGLPointFeature alloc] init];
+ point.title = NSStringFromSelector(_cmd);
+ point.coordinate = CLLocationCoordinate2DMake(0.0, 0.0);
+
+ MGLCoordinateBounds coordinateBounds = [self.mapView convertRect:self.mapView.bounds toCoordinateBoundsFromView:self.mapView];
+ XCTAssert(MGLCoordinateInCoordinateBounds(point.coordinate, coordinateBounds), @"The test point should be within the visible map view");
+
+ [self.mapView addObserver:self forKeyPath:@"centerCoordinate" options:NSKeyValueObservingOptionNew context:_cmd];
+ XCTAssertFalse(self.centerCoordinateDidChange, @"Center coordinate should not have changed at this point");
+
+ // Select on screen annotation (DO NOT ADD FIRST).
+ [self.mapView selectAnnotation:point];
+
+ XCTAssertFalse(self.centerCoordinateDidChange, @"Center coordinate should not have changed after selecting a visible annotation");
+}
+
+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context {
+ if ((context == @selector(testSelectingOnscreenAnnotationThatHasNotBeenAdded)) &&
+ (object == self.mapView)) {
+ self.centerCoordinateDidChange = YES;
+ }
+}
+
+@end