From e1cf6d1515e74e5ee4f9868aa1a4b19c44e77ce7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Wed, 22 Apr 2015 12:45:09 -0700 Subject: [PATCH 1/7] Refactored constraints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing Visual Format Language usage necessitated various ternaries and over-wide lines that made the code harder to grok. Now it’s just plain ol’ Objective-C, mostly column-80-compliant. This change also adds additional constraint relations so that the various subviews respect the `MGLView`’s frame when it’s smaller than the view controller’s root view. Fixes #1327. --- platform/ios/MGLMapView.mm | 158 ++++++++++++++++++++++++------------- 1 file changed, 105 insertions(+), 53 deletions(-) diff --git a/platform/ios/MGLMapView.mm b/platform/ios/MGLMapView.mm index 5f5e99b0eba..e57bc34141b 100644 --- a/platform/ios/MGLMapView.mm +++ b/platform/ios/MGLMapView.mm @@ -491,70 +491,122 @@ - (void)updateConstraints // views so they don't underlap navigation or tool bars. If we don't have a reference, apply // constraints against ourself to maintain (albeit less ideal) placement of the subviews. // - NSString *topGuideFormatString = (self.viewControllerForLayoutGuides ? @"[topLayoutGuide]" : @"|"); - NSString *bottomGuideFormatString = (self.viewControllerForLayoutGuides ? @"[bottomLayoutGuide]" : @"|"); - - id topGuideViewsObject = (self.viewControllerForLayoutGuides ? (id)self.viewControllerForLayoutGuides.topLayoutGuide : (id)@""); - id bottomGuideViewsObject = (self.viewControllerForLayoutGuides ? (id)self.viewControllerForLayoutGuides.bottomLayoutGuide : (id)@""); - - UIView *constraintParentView = (self.viewControllerForLayoutGuides.view ? self.viewControllerForLayoutGuides.view : self); + UIView *constraintParentView = (self.viewControllerForLayoutGuides.view ? + self.viewControllerForLayoutGuides.view : + self); // compass // UIView *compassContainer = self.compass.superview; - [constraintParentView addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:[NSString stringWithFormat:@"V:%@-topSpacing-[container]", topGuideFormatString] - options:0 - metrics:@{ @"topSpacing" : @(5) } - views:@{ @"topLayoutGuide" : topGuideViewsObject, - @"container" : compassContainer }]]; - - [constraintParentView addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:@"H:[container]-rightSpacing-|" - options:0 - metrics:@{ @"rightSpacing" : @(5) } - views:@{ @"container" : compassContainer }]]; - - [compassContainer addConstraint:[NSLayoutConstraint constraintWithItem:compassContainer - attribute:NSLayoutAttributeWidth - relatedBy:NSLayoutRelationEqual - toItem:nil - attribute:NSLayoutAttributeNotAnAttribute - multiplier:1 - constant:self.compass.image.size.width]]; - - [compassContainer addConstraint:[NSLayoutConstraint constraintWithItem:compassContainer - attribute:NSLayoutAttributeHeight - relatedBy:NSLayoutRelationEqual - toItem:nil - attribute:NSLayoutAttributeNotAnAttribute - multiplier:1 - constant:self.compass.image.size.height]]; + if (self.viewControllerForLayoutGuides) + { + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:compassContainer + attribute:NSLayoutAttributeTop + relatedBy:NSLayoutRelationGreaterThanOrEqual + toItem:self.viewControllerForLayoutGuides.topLayoutGuide + attribute:NSLayoutAttributeBottom + multiplier:1 + constant:5]]; + } + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:compassContainer + attribute:NSLayoutAttributeTop + relatedBy:NSLayoutRelationGreaterThanOrEqual + toItem:self + attribute:NSLayoutAttributeTop + multiplier:1 + constant:5]]; + + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self + attribute:NSLayoutAttributeRight + relatedBy:NSLayoutRelationEqual + toItem:compassContainer + attribute:NSLayoutAttributeRight + multiplier:1 + constant:5]]; + + [compassContainer addConstraint: + [NSLayoutConstraint constraintWithItem:compassContainer + attribute:NSLayoutAttributeWidth + relatedBy:NSLayoutRelationEqual + toItem:nil + attribute:NSLayoutAttributeNotAnAttribute + multiplier:1 + constant:self.compass.image.size.width]]; + + [compassContainer addConstraint: + [NSLayoutConstraint constraintWithItem:compassContainer + attribute:NSLayoutAttributeHeight + relatedBy:NSLayoutRelationEqual + toItem:nil + attribute:NSLayoutAttributeNotAnAttribute + multiplier:1 + constant:self.compass.image.size.height]]; // logo bug // - [constraintParentView addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:[NSString stringWithFormat:@"V:[logoBug]-bottomSpacing-%@", bottomGuideFormatString] - options:0 - metrics:@{ @"bottomSpacing" : @(4) } - views:@{ @"logoBug" : self.logoBug, - @"bottomLayoutGuide" : bottomGuideViewsObject }]]; - - [constraintParentView addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:@"H:|-leftSpacing-[logoBug]" - options:0 - metrics:@{ @"leftSpacing" : @(8) } - views:@{ @"logoBug" : self.logoBug }]]; + if (self.viewControllerForLayoutGuides) + { + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self.viewControllerForLayoutGuides.bottomLayoutGuide + attribute:NSLayoutAttributeTop + relatedBy:NSLayoutRelationGreaterThanOrEqual + toItem:self.logoBug + attribute:NSLayoutAttributeBaseline + multiplier:1 + constant:4]]; + } + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self + attribute:NSLayoutAttributeBottom + relatedBy:NSLayoutRelationGreaterThanOrEqual + toItem:self.logoBug + attribute:NSLayoutAttributeBaseline + multiplier:1 + constant:4]]; + + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self.logoBug + attribute:NSLayoutAttributeLeft + relatedBy:NSLayoutRelationEqual + toItem:self + attribute:NSLayoutAttributeLeft + multiplier:1 + constant:8]]; // attribution button // - [constraintParentView addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:[NSString stringWithFormat:@"V:[attributionButton]-bottomSpacing-%@", bottomGuideFormatString] - options:0 - metrics:@{ @"bottomSpacing" : @(8) } - views:@{ @"attributionButton" : self.attributionButton, - @"bottomLayoutGuide" : bottomGuideViewsObject }]]; - - [constraintParentView addConstraints:[NSLayoutConstraint constraintsWithVisualFormat:@"H:[attributionButton]-rightSpacing-|" - options:0 - metrics:@{ @"rightSpacing" : @(8) } - views:@{ @"attributionButton" : self.attributionButton }]]; + if (self.viewControllerForLayoutGuides) + { + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self.viewControllerForLayoutGuides.bottomLayoutGuide + attribute:NSLayoutAttributeTop + relatedBy:NSLayoutRelationGreaterThanOrEqual + toItem:self.attributionButton + attribute:NSLayoutAttributeBaseline + multiplier:1 + constant:8]]; + } + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self + attribute:NSLayoutAttributeBottom + relatedBy:NSLayoutRelationGreaterThanOrEqual + toItem:self.attributionButton + attribute:NSLayoutAttributeBaseline + multiplier:1 + constant:8]]; + + [constraintParentView addConstraint: + [NSLayoutConstraint constraintWithItem:self + attribute:NSLayoutAttributeRight + relatedBy:NSLayoutRelationEqual + toItem:self.attributionButton + attribute:NSLayoutAttributeRight + multiplier:1 + constant:8]]; [super updateConstraints]; } From 8ab22c11a26e58acb7467770eae5b1013c5dad8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Wed, 22 Apr 2015 12:48:15 -0700 Subject: [PATCH 2/7] Clip MGLMapView to its bounds Fixes #1300. --- platform/ios/MGLMapView.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/platform/ios/MGLMapView.mm b/platform/ios/MGLMapView.mm index e57bc34141b..0d2ad1edb64 100644 --- a/platform/ios/MGLMapView.mm +++ b/platform/ios/MGLMapView.mm @@ -242,7 +242,8 @@ - (BOOL)commonInit _glView.contentMode = UIViewContentModeCenter; - [self setBackgroundColor:[UIColor clearColor]]; + self.backgroundColor = [UIColor clearColor]; + self.clipsToBounds = YES; // load extensions // From 9b8b072d28c61a6cca2e4aea3631f8b5660bedad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Wed, 22 Apr 2015 15:33:56 -0700 Subject: [PATCH 3/7] Cleaned up subview layout No need to set the frames manually since we set up constraints as soon as the `MGLMapView` is added to the view hierarchy. Set proper edge insets on the logo bug rather than fudging constraint constants. --- platform/ios/MGLMapView.mm | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/platform/ios/MGLMapView.mm b/platform/ios/MGLMapView.mm index 0d2ad1edb64..405b643c0e8 100644 --- a/platform/ios/MGLMapView.mm +++ b/platform/ios/MGLMapView.mm @@ -294,9 +294,9 @@ - (BOOL)commonInit // setup logo bug // - _logoBug = [[UIImageView alloc] initWithImage:[MGLMapView resourceImageNamed:@"mapbox.png"]]; + UIImage *logo = [[MGLMapView resourceImageNamed:@"mapbox.png"] imageWithAlignmentRectInsets:UIEdgeInsetsMake(1.5, 4, 3.5, 2)]; + _logoBug = [[UIImageView alloc] initWithImage:logo]; _logoBug.accessibilityLabel = @"Mapbox logo"; - _logoBug.frame = CGRectMake(8, self.bounds.size.height - _logoBug.bounds.size.height - 4, _logoBug.bounds.size.width, _logoBug.bounds.size.height); _logoBug.translatesAutoresizingMaskIntoConstraints = NO; [self addSubview:_logoBug]; @@ -305,7 +305,6 @@ - (BOOL)commonInit _attributionButton = [UIButton buttonWithType:UIButtonTypeInfoLight]; _attributionButton.accessibilityLabel = @"Attribution info"; [_attributionButton addTarget:self action:@selector(showAttribution:) forControlEvents:UIControlEventTouchUpInside]; - _attributionButton.frame = CGRectMake(self.bounds.size.width - _attributionButton.bounds.size.width - 8, self.bounds.size.height - _attributionButton.bounds.size.height - 8, _attributionButton.bounds.size.width, _attributionButton.bounds.size.height); _attributionButton.translatesAutoresizingMaskIntoConstraints = NO; [self addSubview:_attributionButton]; @@ -316,7 +315,7 @@ - (BOOL)commonInit UIImage *compassImage = [MGLMapView resourceImageNamed:@"Compass.png"]; _compass.frame = CGRectMake(0, 0, compassImage.size.width, compassImage.size.height); _compass.alpha = 0; - UIView *container = [[UIView alloc] initWithFrame:CGRectMake(self.bounds.size.width - compassImage.size.width - 5, 5, compassImage.size.width, compassImage.size.height)]; + UIView *container = [[UIView alloc] initWithFrame:CGRectZero]; [container addSubview:_compass]; container.translatesAutoresizingMaskIntoConstraints = NO; [container addGestureRecognizer:[[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(handleCompassTapGesture:)]]; @@ -558,7 +557,7 @@ - (void)updateConstraints toItem:self.logoBug attribute:NSLayoutAttributeBaseline multiplier:1 - constant:4]]; + constant:8]]; } [constraintParentView addConstraint: [NSLayoutConstraint constraintWithItem:self @@ -567,7 +566,7 @@ - (void)updateConstraints toItem:self.logoBug attribute:NSLayoutAttributeBaseline multiplier:1 - constant:4]]; + constant:8]]; [constraintParentView addConstraint: [NSLayoutConstraint constraintWithItem:self.logoBug From fec1cf0e23a97b0f76f0aaabf0a71004e5a1dac2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Thu, 23 Apr 2015 15:02:06 -0700 Subject: [PATCH 4/7] RTL-friendly constraints Use leading/trailing instead of left/right for subview constraints for eventual right-to-left flippedness. --- platform/ios/MGLMapView.mm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/platform/ios/MGLMapView.mm b/platform/ios/MGLMapView.mm index 405b643c0e8..3094d2f8ce0 100644 --- a/platform/ios/MGLMapView.mm +++ b/platform/ios/MGLMapView.mm @@ -521,10 +521,10 @@ - (void)updateConstraints [constraintParentView addConstraint: [NSLayoutConstraint constraintWithItem:self - attribute:NSLayoutAttributeRight + attribute:NSLayoutAttributeTrailing relatedBy:NSLayoutRelationEqual toItem:compassContainer - attribute:NSLayoutAttributeRight + attribute:NSLayoutAttributeTrailing multiplier:1 constant:5]]; @@ -570,10 +570,10 @@ - (void)updateConstraints [constraintParentView addConstraint: [NSLayoutConstraint constraintWithItem:self.logoBug - attribute:NSLayoutAttributeLeft + attribute:NSLayoutAttributeLeading relatedBy:NSLayoutRelationEqual toItem:self - attribute:NSLayoutAttributeLeft + attribute:NSLayoutAttributeLeading multiplier:1 constant:8]]; @@ -601,10 +601,10 @@ - (void)updateConstraints [constraintParentView addConstraint: [NSLayoutConstraint constraintWithItem:self - attribute:NSLayoutAttributeRight + attribute:NSLayoutAttributeTrailing relatedBy:NSLayoutRelationEqual toItem:self.attributionButton - attribute:NSLayoutAttributeRight + attribute:NSLayoutAttributeTrailing multiplier:1 constant:8]]; From 2c436444d7ebf22ca48ef08e077a144ddea76ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Fri, 24 Apr 2015 13:06:54 -0700 Subject: [PATCH 5/7] Made user dot accessible Added an accessibility label to the user dot. --- platform/ios/MGLUserLocationAnnotationView.m | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/ios/MGLUserLocationAnnotationView.m b/platform/ios/MGLUserLocationAnnotationView.m index 779382b5e49..c130582d6f6 100644 --- a/platform/ios/MGLUserLocationAnnotationView.m +++ b/platform/ios/MGLUserLocationAnnotationView.m @@ -30,6 +30,7 @@ - (instancetype)initInMapView:(MGLMapView *)mapView self.annotation.mapView = mapView; _mapView = mapView; [self setupLayers]; + self.accessibilityLabel = @"User location"; } return self; } From 1dc2cbc280292c76eaca21fde9dddfbd30060fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Fri, 24 Apr 2015 13:30:50 -0700 Subject: [PATCH 6/7] Test inset MGLMapView MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verify that the ornaments hug the `MGLMapView` when it’s smaller than the view controller’s root view. --- test/ios/KIFTestActor+MapboxGL.h | 4 ++-- test/ios/KIFTestActor+MapboxGL.m | 4 ++-- test/ios/MGLTViewController.h | 2 ++ test/ios/MGLTViewController.m | 16 ++++++++++++---- test/ios/MapViewTests.m | 23 +++++++++++++++++++++++ 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/test/ios/KIFTestActor+MapboxGL.h b/test/ios/KIFTestActor+MapboxGL.h index df83b2377ae..d16e348486d 100644 --- a/test/ios/KIFTestActor+MapboxGL.h +++ b/test/ios/KIFTestActor+MapboxGL.h @@ -1,12 +1,12 @@ #import #import -@class MGLMapView; +@class MGLTViewController, MGLMapView; @interface KIFTestActor (MapboxGL) @property (nonatomic, readonly) UIWindow *window; -@property (nonatomic, readonly) UIViewController *viewController; +@property (nonatomic, readonly) MGLTViewController *viewController; @property (nonatomic, readonly) MGLMapView *mapView; @property (nonatomic, readonly) UIView *compass; diff --git a/test/ios/KIFTestActor+MapboxGL.m b/test/ios/KIFTestActor+MapboxGL.m index ef40c1bed1a..330adfdc28a 100644 --- a/test/ios/KIFTestActor+MapboxGL.m +++ b/test/ios/KIFTestActor+MapboxGL.m @@ -10,8 +10,8 @@ - (UIWindow *)window { return [[UIApplication sharedApplication] statusBarWindow]; } -- (UIViewController *)viewController { - return (UIViewController *)[[tester.mapView nextResponder] nextResponder]; +- (MGLTViewController *)viewController { + return (MGLTViewController *)[[tester.mapView nextResponder] nextResponder]; } - (MGLMapView *)mapView { diff --git a/test/ios/MGLTViewController.h b/test/ios/MGLTViewController.h index 39df59bb5ee..f8d56189938 100644 --- a/test/ios/MGLTViewController.h +++ b/test/ios/MGLTViewController.h @@ -2,4 +2,6 @@ @interface MGLTViewController : UIViewController +- (void)insetMapView; + @end diff --git a/test/ios/MGLTViewController.m b/test/ios/MGLTViewController.m index e839c45047b..2cb2b939c73 100644 --- a/test/ios/MGLTViewController.m +++ b/test/ios/MGLTViewController.m @@ -2,17 +2,25 @@ #import "MapboxGL.h" @implementation MGLTViewController +{ + MGLMapView *_mapView; +} - (void)viewDidLoad { [super viewDidLoad]; - MGLMapView *mapView = [[MGLMapView alloc] initWithFrame:self.view.bounds + _mapView = [[MGLMapView alloc] initWithFrame:self.view.bounds accessToken:@"pk.eyJ1IjoianVzdGluIiwiYSI6Ik9RX3RRQzAifQ.dmOg_BAp1ywuDZMM7YsXRg"]; - mapView.viewControllerForLayoutGuides = self; - mapView.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight; + _mapView.viewControllerForLayoutGuides = self; + _mapView.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight; - [self.view addSubview:mapView]; + [self.view addSubview:_mapView]; +} + +- (void)insetMapView +{ + _mapView.frame = CGRectInset(_mapView.frame, 50, 50); } @end diff --git a/test/ios/MapViewTests.m b/test/ios/MapViewTests.m index deed521b717..2d87b43c157 100644 --- a/test/ios/MapViewTests.m +++ b/test/ios/MapViewTests.m @@ -4,6 +4,7 @@ #import "KIFTestActor+MapboxGL.h" #import "MapboxGL.h" +#import "MGLTViewController.h" @interface MapViewTests : KIFTestCase @@ -297,6 +298,28 @@ - (void)testBottomLayoutGuide { @"rotated device should not have attribution button under toolbar"); } +- (void)testInsetMapView { + [tester.viewController insetMapView]; + [tester waitForAnimationsToFinish]; + + UIView *logoBug = (UIView *)[tester waitForViewWithAccessibilityLabel:@"Mapbox logo"]; + UIView *attributionButton = (UIView *)[tester waitForViewWithAccessibilityLabel:@"Attribution info"]; + + CGRect mapViewFrame = [tester.mapView.superview convertRect:tester.mapView.frame toView:nil]; + + CGRect logoBugFrame = [logoBug.superview convertRect:logoBug.frame toView:nil]; + XCTAssertTrue(CGRectIntersectsRect(logoBugFrame, mapViewFrame), + @"logo bug should lie inside shrunken map view"); + + CGRect attributionButtonFrame = [attributionButton.superview convertRect:attributionButton.frame toView:nil]; + XCTAssertTrue(CGRectIntersectsRect(attributionButtonFrame, mapViewFrame), + @"attribution button should lie inside shrunken map view"); + + CGRect compassFrame = [tester.compass.superview convertRect:tester.compass.frame toView:nil]; + XCTAssertTrue(CGRectIntersectsRect(compassFrame, mapViewFrame), + @"compass should lie inside shrunken map view"); +} + - (void)testDelegateRegionWillChange { __block NSUInteger unanimatedCount; __block NSUInteger animatedCount; From f9c544a9ea0f9a321616fbc7147f67b5c90b7832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Fri, 24 Apr 2015 14:27:25 -0700 Subject: [PATCH 7/7] Reset map view frame on every test To counteract the call to `-insetMapView` in `-testInsetMapView`. --- test/ios/MGLTViewController.h | 1 + test/ios/MGLTViewController.m | 5 +++++ test/ios/MapViewTests.m | 1 + 3 files changed, 7 insertions(+) diff --git a/test/ios/MGLTViewController.h b/test/ios/MGLTViewController.h index f8d56189938..0be0e1ff2c4 100644 --- a/test/ios/MGLTViewController.h +++ b/test/ios/MGLTViewController.h @@ -3,5 +3,6 @@ @interface MGLTViewController : UIViewController - (void)insetMapView; +- (void)resetMapView; @end diff --git a/test/ios/MGLTViewController.m b/test/ios/MGLTViewController.m index 2cb2b939c73..e8b500b430c 100644 --- a/test/ios/MGLTViewController.m +++ b/test/ios/MGLTViewController.m @@ -23,4 +23,9 @@ - (void)insetMapView _mapView.frame = CGRectInset(_mapView.frame, 50, 50); } +- (void)resetMapView +{ + _mapView.frame = self.view.bounds; +} + @end diff --git a/test/ios/MapViewTests.m b/test/ios/MapViewTests.m index 2d87b43c157..fff52adc0ee 100644 --- a/test/ios/MapViewTests.m +++ b/test/ios/MapViewTests.m @@ -14,6 +14,7 @@ @implementation MapViewTests - (void)beforeEach { [system simulateDeviceRotationToOrientation:UIDeviceOrientationPortrait]; + [tester.viewController resetMapView]; tester.mapView.centerCoordinate = CLLocationCoordinate2DMake(38.913175, -77.032458); tester.mapView.zoomLevel = 14;