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

Fix problems with map view UI layout constraints #6776

Merged
merged 3 commits into from
Nov 15, 2016

Conversation

friedbunny
Copy link
Contributor

@friedbunny friedbunny commented Oct 20, 2016

This addresses two problems with the map view’s UI elements (compass, attribution button, logo):

Fixes ambiguous constraints

Fixes #5549.

By declaring that the vertical constraint relationships were NSLayoutRelationGreaterThanOrEqual, this meant that there were many positions that could potentially satisfy these constraints. This is not what we want (and not what was being shown) — the vertical placement of these views should be NSLayoutRelationEqual to the exact values we calculate.

Fixes iOS 10 giving us incorrect topLayoutGuide values

Fixes #6755, where the vertical offset of the compass would be incorrect after device rotation.

Compared to iOS 9, iOS 10 is giving incorrect values for topLayoutGuide when the device is rotated. For reasons I don’t understand, re-adding a direct constraint against topLayoutGuide fixes the issue. This walks back a small part of #5671, where our accounting for layout guides moved entirely to -adjustContentInset.

This fix isn’t great, but it’s backwards compatible and it works.

Notes

/cc @frederoni @1ec5

@friedbunny friedbunny added bug iOS Mapbox Maps SDK for iOS labels Oct 20, 2016
@friedbunny friedbunny added this to the ios-v3.4.0 milestone Oct 20, 2016
@friedbunny friedbunny self-assigned this Oct 20, 2016
@mention-bot
Copy link

@friedbunny, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1ec5, @frederoni and @incanus to be potential reviewers.

@frederoni
Copy link
Contributor

Assumed the Swift equivalent would be something like po UIApplication.shared.delegate?.window._autotraceLayout but apparently not. 😕

@friedbunny
Copy link
Contributor Author

As a private method, I think it’s not exposed in Swift (which is what lldb expects when you’re in a Swift app). Looks like expr -l objc++ -O -- [[UIWindow keyWindow] _autolayoutTrace] is how you’d do it. 😉

@1ec5
Copy link
Contributor

1ec5 commented Oct 21, 2016

The inequality constraints were originally added in #1328. Can you verify that these changes don’t regress #1327?

@friedbunny
Copy link
Contributor Author

The bottom elements continue to move with the map view, but the compass has gone MIA:

img_4568 2

friedbunny and others added 3 commits November 15, 2016 12:57
NSLayoutRelationGreaterThanOrEqual meant that it either greater-than or
equal could satisify the constraint, when really all we need is equal.
In iOS 10, the top layout guide is not always returning the expected
value unless there is a constraint applied directly against it.
@frederoni
Copy link
Contributor

If you're using a map view that is not full size, you should set automaticallyAdjustsScrollViewInsets to false. I rebased this PR and added a forced re-layout after the device has been rotated which takes care of the layout bug in iOS 10.

The following cases has been tested in iOS 8, 9 and 10:

  • ✅ Full size map view, w/ navbar and tabbar w/ automaticallyAdjustsScrollViewInsets=YES.
  • ✅ Half size map view w/ automaticallyAdjustsScrollViewInsets=NO.

11 22 33 44 55

@ketz
Copy link

ketz commented Nov 15, 2016

Sorry if I'm being thick, but I'm having the same problem when resizing the map view to show an info panel with a POI's details.

Where exactly should I set automaticallyAdjustsScrollViewInsets:NO? I'm not using auto layout, and all my views are created programatically.

thanks

@1ec5
Copy link
Contributor

1ec5 commented Nov 15, 2016

Where exactly should I set automaticallyAdjustsScrollViewInsets:NO?

automaticallyAdjustsScrollViewInsets is a property on the view controller that contains MGLMapView.

@1ec5 1ec5 deleted the fb-ui-constraints branch November 15, 2016 20:52
@1ec5
Copy link
Contributor

1ec5 commented Nov 15, 2016

If you're using a map view that is not full size, you should set automaticallyAdjustsScrollViewInsets to false.

@frederoni, just to be clear, though, we haven’t regressed #1327, have we? The compass should never float outside of the map view, even if you have a non-full-screen map view and automaticallyAdjustsScrollViewInsets set to YES.

@1ec5
Copy link
Contributor

1ec5 commented Nov 15, 2016

I reopened #6954 to track the regression exacerbated by this PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants