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

Conversation

fabian-guerra
Copy link
Contributor

Fixes #167

@fabian-guerra fabian-guerra added the bug Something isn't working label Sep 4, 2020
@fabian-guerra fabian-guerra self-assigned this Sep 4, 2020
@fabian-guerra fabian-guerra marked this pull request as ready for review September 9, 2020 21:47
@fabian-guerra fabian-guerra requested review from a team and julianrex September 9, 2020 21:47
Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

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

Just some confusion with naming here on my part. Any chance of consolidating here?

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

- (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.

Comment on lines +947 to +953
- (NSLayoutYAxisAnchor *)safeTopAnchor {
if ([self hasAutomaticallyAdjustContentInset]) {
return self.mgl_safeTopAnchor;
} else {
return self.topAnchor;
}
}
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.

@fabian-guerra fabian-guerra merged commit b9a1d76 into master Sep 10, 2020
@fabian-guerra fabian-guerra deleted the fabian-content-insets branch September 10, 2020 16:55
katydecorah pushed a commit that referenced this pull request Sep 10, 2020
* master:
  Fix content insets when automaticallyAdjustsScrollViewInsets is set to NO (#420)
  Adds missing method for location test. (#422)
  Autodetect Xcode 12 to apply xcconfig patch (#419)
katydecorah pushed a commit that referenced this pull request Sep 10, 2020
* master:
  Fix content insets when automaticallyAdjustsScrollViewInsets is set to NO (#420)
katydecorah pushed a commit that referenced this pull request Sep 11, 2020
* master:
  Approximate Location "Puck" logic updates (#427)
  Fix content insets when automaticallyAdjustsScrollViewInsets is set to NO (#420)
@julianrex julianrex mentioned this pull request Sep 14, 2020
6 tasks
fabian-guerra added a commit that referenced this pull request Sep 14, 2020
…o NO (#420)

* Fix an issue when automaticallyAdjustContentInset is set to NO

* Update tests to account for safe areas on iOS 11

* Update changelog

* Update method documentation
@fabian-guerra fabian-guerra mentioned this pull request Sep 14, 2020
6 tasks
fabian-guerra added a commit that referenced this pull request Sep 14, 2020
* changing the default size for approx ring border when zoomed out (#429)

* Approximate Location "Puck" logic updates (#427)

* updates to approximate puck drawing logic

* removing unused header

* Allow for transitive dependencies (#431)

* Fix content insets when automaticallyAdjustsScrollViewInsets is set to NO (#420)

* Fix an issue when automaticallyAdjustContentInset is set to NO

* Update tests to account for safe areas on iOS 11

* Update changelog

* Update method documentation

* Update to gl-native 5.0.0 (#438)

* Update to application state management and associated view setup and rendering (#432)

Co-authored-by: Neel Mistry <Neel.mistry@mapbox.com>
Co-authored-by: Fredrik Karlsson <bjorn.fredrik.karlsson@gmail.com>
Co-authored-by: Julian Rex <julian.rex@mapbox.com>
mappy-mobile pushed a commit to Mappy/mapbox-gl-native-ios that referenced this pull request Sep 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

testContentInsetCenter & testContentInsetOrnaments fail on iPhone X
2 participants