Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Fix content insets when automaticallyAdjustsScrollViewInsets is set to NO #420

Merged
merged 4 commits into from
Sep 10, 2020
Merged

Fix content insets when automaticallyAdjustsScrollViewInsets is set to NO #420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ebc32c3
Fix an issue when automaticallyAdjustContentInset is set to NO
fabian-guerra Sep 9, 2020
f37eaef
Update tests to account for safe areas on iOS 11
fabian-guerra Sep 9, 2020
e45a10d
Update changelog
fabian-guerra Sep 9, 2020
51ffbb7
Update method documentation
fabian-guerra Sep 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ You should not submit apps built with `6.2.0-beta.1` to the App Store. If you ne
* Added `MGLNetworkConfiguration.connected` property to enforce `MGLMapView` to use cached tiles. ([#416](https://github.com/mapbox/mapbox-gl-native-ios/pull/416))
* Enabled local glyph rasterization for all writing systems. The new feature uses real glyph metrics and improves rendering quality for mixed (Latin / CJKV) labels. (#561)
* Minimum and Maximum allowed values are now considered for style layer properties, as defined by the specification. If a style property is assigned with a value outside the allowed range, the property gets assigned with its default value instead. (#647)
* Fixed an issue that caused ornaments to consider safe areas when `MGLMapView.automaticallyAdjustsContentInset` is set to `NO`. ([#420](https://github.com/mapbox/mapbox-gl-native-ios/pull/420))

### 🐞 Bug fixes

Expand Down
89 changes: 68 additions & 21 deletions platform/ios/src/MGLMapView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,19 +886,10 @@ - (void)updateConstraintsForOrnament:(UIView *)view
NSMutableArray *updatedConstraints = [NSMutableArray array];
UIEdgeInsets inset = UIEdgeInsetsZero;

BOOL automaticallyAdjustContentInset;
if (_automaticallyAdjustContentInsetHolder) {
automaticallyAdjustContentInset = _automaticallyAdjustContentInsetHolder.boolValue;
} else {
UIViewController *viewController = [self rootViewController];
automaticallyAdjustContentInset = viewController.automaticallyAdjustsScrollViewInsets;
}


if (! automaticallyAdjustContentInset) {
inset = UIEdgeInsetsMake(self.contentInset.top - self.safeMapViewContentInsets.top,
self.contentInset.left - self.safeMapViewContentInsets.left,
self.contentInset.bottom - self.safeMapViewContentInsets.bottom,
self.contentInset.right - self.safeMapViewContentInsets.right);
if (! [self hasAutomaticallyAdjustContentInset]) {
inset = self.contentInset;

// makes sure the insets don't have negative values that could hide the ornaments
// thus violating our ToS
Expand All @@ -910,20 +901,20 @@ - (void)updateConstraintsForOrnament:(UIView *)view

switch (position) {
case MGLOrnamentPositionTopLeft:
[updatedConstraints addObject:[view.topAnchor constraintEqualToAnchor:self.mgl_safeTopAnchor constant:margins.y + inset.top]];
[updatedConstraints addObject:[view.leadingAnchor constraintEqualToAnchor:self.mgl_safeLeadingAnchor constant:margins.x + inset.left]];
[updatedConstraints addObject:[view.topAnchor constraintEqualToAnchor:self.safeTopAnchor constant:margins.y + inset.top]];
[updatedConstraints addObject:[view.leadingAnchor constraintEqualToAnchor:self.safeLeadingAnchor constant:margins.x + inset.left]];
break;
case MGLOrnamentPositionTopRight:
[updatedConstraints addObject:[view.topAnchor constraintEqualToAnchor:self.mgl_safeTopAnchor constant:margins.y + inset.top]];
[updatedConstraints addObject:[self.mgl_safeTrailingAnchor constraintEqualToAnchor:view.trailingAnchor constant:margins.x + inset.right]];
[updatedConstraints addObject:[view.topAnchor constraintEqualToAnchor:self.safeTopAnchor constant:margins.y + inset.top]];
[updatedConstraints addObject:[self.safeTrailingAnchor constraintEqualToAnchor:view.trailingAnchor constant:margins.x + inset.right]];
break;
case MGLOrnamentPositionBottomLeft:
[updatedConstraints addObject:[self.mgl_safeBottomAnchor constraintEqualToAnchor:view.bottomAnchor constant:margins.y + inset.bottom]];
[updatedConstraints addObject:[view.leadingAnchor constraintEqualToAnchor:self.mgl_safeLeadingAnchor constant:margins.x + inset.left]];
[updatedConstraints addObject:[self.safeBottomAnchor constraintEqualToAnchor:view.bottomAnchor constant:margins.y + inset.bottom]];
[updatedConstraints addObject:[view.leadingAnchor constraintEqualToAnchor:self.safeLeadingAnchor constant:margins.x + inset.left]];
break;
case MGLOrnamentPositionBottomRight:
[updatedConstraints addObject:[self.mgl_safeBottomAnchor constraintEqualToAnchor:view.bottomAnchor constant:margins.y + inset.bottom]];
[updatedConstraints addObject: [self.mgl_safeTrailingAnchor constraintEqualToAnchor:view.trailingAnchor constant:margins.x + inset.right]];
[updatedConstraints addObject:[self.safeBottomAnchor constraintEqualToAnchor:view.bottomAnchor constant:margins.y + inset.bottom]];
[updatedConstraints addObject: [self.safeTrailingAnchor constraintEqualToAnchor:view.trailingAnchor constant:margins.x + inset.right]];
break;
}

Expand All @@ -941,10 +932,66 @@ - (void)updateConstraintsForOrnament:(UIView *)view
[constraints addObjectsFromArray:updatedConstraints];
}

// Convenience method to check if a user has enabled automatically adjust content
// insets. Currently users can use MGLMapView.automaticallyAdjustContentInset or
// UIViewController.automaticallyAdjustsScrollViewInsets
// TODO: Remove when UIViewController.automaticallyAdjustsScrollViewInsets is removed
- (BOOL)hasAutomaticallyAdjustContentInset {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'm having trouble understanding the meaning of the method name.

Can we match the similar view controller method? automaticallyAdjustsContentInsets

depending on context?

EDIT: Oh, I see - (BOOL)automaticallyAdjustsContentInset - I'm more confused now 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasAutomaticallyAdjustContentInset is an internal convenience method to avoid code duplication. Since we have two ways to check if we disabled adjusting automatically content insets: using UIViewController.automaticallyAdjustsScrollViewInsets (deprecated) and MGLMapView.automaticallyAdjustContentInset I am using this method to check these options.

BOOL automaticallyAdjustContentInset;
if (_automaticallyAdjustContentInsetHolder) {
automaticallyAdjustContentInset = _automaticallyAdjustContentInsetHolder.boolValue;
} else {
UIViewController *viewController = [self rootViewController];
automaticallyAdjustContentInset = viewController.automaticallyAdjustsScrollViewInsets;
}

return automaticallyAdjustContentInset;
}

// Checks if it has enabled adjust content insets. When true it will calculate the anchor using
// safe area insets otherwise it will return the map's view anchor.
- (NSLayoutYAxisAnchor *)safeTopAnchor {
if ([self hasAutomaticallyAdjustContentInset]) {
return self.mgl_safeTopAnchor;
} else {
return self.topAnchor;
}
}
Comment on lines +953 to +959
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another confusing one for me - is there anyway to combine safeTapAnchor and mgl_safeTopAnchor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this method is doing is check if it has automatically adjust content insets. If it has then it uses an anchor that considers safe areas which is self.mgl_safeTopAnchor that is in a category, if it is not then uses the current view anchor without considering safe areas.


// Checks if it has enabled adjust content insets. When true it will calculate the anchor using
// safe area insets otherwise it will return the map's view anchor.
- (NSLayoutYAxisAnchor *)safeBottomAnchor {
if ([self hasAutomaticallyAdjustContentInset]) {
return self.mgl_safeBottomAnchor;
} else {
return self.bottomAnchor;
}
}

// Checks if it has enabled adjust content insets. When true it will calculate the anchor using
// safe area insets otherwise it will return the map's view anchor.
- (NSLayoutXAxisAnchor *)safeLeadingAnchor {
if ([self hasAutomaticallyAdjustContentInset]) {
return self.mgl_safeLeadingAnchor;
} else {
return self.leadingAnchor;
}
}

// Checks if it has enabled adjust content insets. When true it will calculate the anchor using
// safe area insets otherwise it will return the map's view anchor.
- (NSLayoutXAxisAnchor *)safeTrailingAnchor {
if ([self hasAutomaticallyAdjustContentInset]) {
return self.mgl_safeTrailingAnchor;
} else {
return self.trailingAnchor;
}
}

- (void)installConstraints
{
[self installCompassViewConstraints];
[self installScaleBarConstraints];
[self installCompassViewConstraints];
[self installLogoViewConstraints];
[self installAttributionButtonConstraints];
}
Expand Down
99 changes: 60 additions & 39 deletions platform/ios/test/MGLMapViewContentInsetTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ - (void)tearDown {

- (void)testContentInsetCenter {
CLLocationCoordinate2D center = CLLocationCoordinate2DMake(1.0, 5.0);
self.mapView.centerCoordinate = center;
[self.mapView setCenterCoordinate:center zoomLevel:10 animated:NO];
self.mapView.contentInset = UIEdgeInsetsZero;

XCTAssertEqualWithAccuracy(self.mapView.centerCoordinate.latitude, center.latitude, 0.01);
XCTAssertEqualWithAccuracy(self.mapView.centerCoordinate.longitude, center.longitude, 0.01);

Expand All @@ -77,98 +79,117 @@ - (void)testContentInsetCenter {
}

- (void)testContentInsetOrnaments {
CGFloat margin = 8;
UIEdgeInsets margin = UIEdgeInsetsMake(8, 8, 8, 8);

if (@available(iOS 11.0, *)) {
UIEdgeInsets safeInsets = self.mapView.safeAreaInsets;
margin.top += safeInsets.top;
margin.bottom += safeInsets.bottom;
margin.left += safeInsets.left;
margin.right += safeInsets.right;
}

self.mapView.contentInset = UIEdgeInsetsZero;
UIView *scaleBar = self.mapView.scaleBar;
CGPoint expectedScaleBarOrigin = CGPointMake(margin, margin);
CGPoint expectedScaleBarOrigin = CGPointMake(margin.left, margin.top);
XCTAssertTrue(CGPointEqualToPoint(scaleBar.frame.origin, expectedScaleBarOrigin));

UIView *compassView = self.mapView.compassView;
CGFloat x = self.screenBounds.size.width - compassView.bounds.size.width - margin;
CGPoint expectedCompassOrigin = CGPointMake(x, margin);
CGFloat x = self.screenBounds.size.width - compassView.bounds.size.width - margin.right;
CGPoint expectedCompassOrigin = CGPointMake(x, margin.top);
XCTAssertTrue(CGPointEqualToPoint(compassView.frame.origin, expectedCompassOrigin));

UIView *logoView = self.mapView.logoView;
CGFloat y = self.screenBounds.size.height - logoView.bounds.size.height - margin;
CGPoint expectedLogoOrigin = CGPointMake(margin, y);
CGFloat y = self.screenBounds.size.height - logoView.bounds.size.height - margin.bottom;
CGPoint expectedLogoOrigin = CGPointMake(margin.left, y);
XCTAssertTrue(CGPointEqualToPoint(logoView.frame.origin, expectedLogoOrigin));

UIView *attributionView = self.mapView.attributionButton;
x = self.screenBounds.size.width - attributionView.bounds.size.width - margin;
y = self.screenBounds.size.height - attributionView.bounds.size.height - margin;
x = self.screenBounds.size.width - attributionView.bounds.size.width - margin.right;
y = self.screenBounds.size.height - attributionView.bounds.size.height - margin.bottom;
CGPoint expectedAttributionOrigin = CGPointMake(x, y);
XCTAssertTrue(CGPointEqualToPoint(attributionView.frame.origin, expectedAttributionOrigin));

UIEdgeInsets insets = UIEdgeInsetsMake(15, 10, 20, 5);
self.viewController.automaticallyAdjustsScrollViewInsets = NO;
self.mapView.contentInset = insets;

[self.mapView setNeedsLayout];
[self.mapView layoutIfNeeded];

expectedScaleBarOrigin = CGPointMake(insets.left + self.mapView.scaleBarMargins.x, insets.top + self.mapView.scaleBarMargins.y);
XCTAssertTrue(CGPointEqualToPoint(scaleBar.frame.origin, expectedScaleBarOrigin));

x = self.screenBounds.size.width - compassView.bounds.size.width - insets.right - self.mapView.compassViewMargins.x;
expectedCompassOrigin = CGPointMake(x, insets.top + self.mapView.compassViewMargins.y);
XCTAssertTrue(CGPointEqualToPoint(compassView.frame.origin, expectedCompassOrigin));

y = self.screenBounds.size.height - logoView.bounds.size.height - insets.bottom - self.mapView.logoViewMargins.y;
expectedLogoOrigin = CGPointMake(insets.left + self.mapView.logoViewMargins.x, y);
XCTAssertTrue(CGPointEqualToPoint(logoView.frame.origin, expectedLogoOrigin));

x = self.screenBounds.size.width - attributionView.bounds.size.width - insets.right - self.mapView.attributionButtonMargins.x;
y = self.screenBounds.size.height - attributionView.bounds.size.height - insets.bottom - self.mapView.attributionButtonMargins.y;
expectedAttributionOrigin = CGPointMake(x, y);
XCTAssertTrue(CGPointEqualToPoint(attributionView.frame.origin, expectedAttributionOrigin));

// tests that passing negative values result in a 0 inset value
insets = UIEdgeInsetsMake(-100, -100, -100, -100);
self.mapView.contentInset = insets;

[self.mapView setNeedsLayout];
[self.mapView layoutIfNeeded];

expectedScaleBarOrigin = CGPointMake(margin, margin);
margin = UIEdgeInsetsMake(8, 8, 8, 8);

expectedScaleBarOrigin = CGPointMake(margin.left, margin.top);
XCTAssertTrue(CGPointEqualToPoint(scaleBar.frame.origin, expectedScaleBarOrigin));
x = self.screenBounds.size.width - compassView.bounds.size.width - margin;
expectedCompassOrigin = CGPointMake(x, margin);

x = self.screenBounds.size.width - compassView.bounds.size.width - margin.right;
expectedCompassOrigin = CGPointMake(x, margin.top);
XCTAssertTrue(CGPointEqualToPoint(compassView.frame.origin, expectedCompassOrigin));
y = self.screenBounds.size.height - logoView.bounds.size.height - margin;
expectedLogoOrigin = CGPointMake(margin, y);

y = self.screenBounds.size.height - logoView.bounds.size.height - margin.bottom;
expectedLogoOrigin = CGPointMake(margin.left, y);
XCTAssertTrue(CGPointEqualToPoint(logoView.frame.origin, expectedLogoOrigin));
x = self.screenBounds.size.width - attributionView.bounds.size.width - margin;
y = self.screenBounds.size.height - attributionView.bounds.size.height - margin;

x = self.screenBounds.size.width - attributionView.bounds.size.width - margin.right;
y = self.screenBounds.size.height - attributionView.bounds.size.height - margin.bottom;
expectedAttributionOrigin = CGPointMake(x, y);
XCTAssertTrue(CGPointEqualToPoint(attributionView.frame.origin, expectedAttributionOrigin));

self.mapView.automaticallyAdjustsContentInset = YES;
insets = UIEdgeInsetsMake(100, 100, 100, 100);
self.mapView.contentInset = insets;
XCTAssertTrue(UIEdgeInsetsEqualToEdgeInsets(self.mapView.contentInset, insets));

[self.mapView setNeedsLayout];
[self.mapView layoutIfNeeded];

if (@available(iOS 11.0, *)) {
UIEdgeInsets safeInsets = self.mapView.safeAreaInsets;
margin.top += safeInsets.top;
margin.bottom += safeInsets.bottom;
margin.left += safeInsets.left;
margin.right += safeInsets.right;
}

// when automaticallyAdjustsContentInset = YES the content insets should be overwriten
XCTAssertFalse(UIEdgeInsetsEqualToEdgeInsets(self.mapView.contentInset, insets));
expectedScaleBarOrigin = CGPointMake(margin, margin);

expectedScaleBarOrigin = CGPointMake(margin.left, margin.top);
XCTAssertTrue(CGPointEqualToPoint(scaleBar.frame.origin, expectedScaleBarOrigin));
x = self.screenBounds.size.width - compassView.bounds.size.width - margin;
expectedCompassOrigin = CGPointMake(x, margin);

x = self.screenBounds.size.width - compassView.bounds.size.width - margin.right;
expectedCompassOrigin = CGPointMake(x, margin.top);
XCTAssertTrue(CGPointEqualToPoint(compassView.frame.origin, expectedCompassOrigin));
y = self.screenBounds.size.height - logoView.bounds.size.height - margin;
expectedLogoOrigin = CGPointMake(margin, y);

y = self.screenBounds.size.height - logoView.bounds.size.height - margin.bottom;
expectedLogoOrigin = CGPointMake(margin.left, y);
XCTAssertTrue(CGPointEqualToPoint(logoView.frame.origin, expectedLogoOrigin));
x = self.screenBounds.size.width - attributionView.bounds.size.width - margin;
y = self.screenBounds.size.height - attributionView.bounds.size.height - margin;

x = self.screenBounds.size.width - attributionView.bounds.size.width - margin.right;
y = self.screenBounds.size.height - attributionView.bounds.size.height - margin.bottom;
expectedAttributionOrigin = CGPointMake(x, y);
XCTAssertTrue(CGPointEqualToPoint(attributionView.frame.origin, expectedAttributionOrigin));

Expand Down