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

[ios] fixes #3697 compass, logo and attribution now respects insets #5671

Merged
merged 3 commits into from
Jul 18, 2016

Conversation

frederoni
Copy link
Contributor

@frederoni frederoni commented Jul 13, 2016

Compass, logo and attribution button new respects the map view's content inset.

👀 @1ec5 @friedbunny @boundsj

@frederoni frederoni added iOS Mapbox Maps SDK for iOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Jul 13, 2016
@frederoni frederoni added this to the ios-v3.3.1 milestone Jul 13, 2016
@frederoni frederoni mentioned this pull request Jul 13, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jul 13, 2016

When the contentInset property changes, the constraints need to be updated immediately.

@frederoni
Copy link
Contributor Author

Note: automaticallyAdjustsScrollViewInsets must be set to NO on the VC for the contentInset to be persistent. If not, contentInset will be adjusted automatically and logo, attribution and compass will fallback to its original position next time -layoutSubviews is called. This is consistent with how UIScrollView works. @1ec5 👀

@frederoni frederoni force-pushed the 3697-compass-logo-attribution-insets branch from 5c16d8b to cb1a7bd Compare July 13, 2016 19:02
@@ -974,6 +975,9 @@ - (void)setContentInset:(UIEdgeInsets)contentInset animated:(BOOL)animated
{
[self didUpdateLocationWithUserTrackingAnimated:animated];
}

// Compass, logo and attribution button constraints needs to be updated.
[self setNeedsUpdateConstraints];
Copy link
Contributor

Choose a reason for hiding this comment

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

-layoutSubviews calls -adjustContentInset, which calls -setContentInset: and -setContentInset:animated:, which calls -setNeedsUpdateConstraints. I’m not quite sure, but are there any cases where that’ll cause a feedback loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I can find. -setNeedsUpdateConstraints will make sure a future call to -updateConstraintsIfNeeded calls -updateConstraints = no loop. If -adjustContentInset would get triggered in a loop, the loop is interrupted either because the previous contentInset is equal to the new or in case you're setting your own contentInset, it would get interrupted before that because of automaticallyAdjustsScrollViewInsets=NO

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@frederoni frederoni force-pushed the 3697-compass-logo-attribution-insets branch from f178476 to a755a32 Compare July 14, 2016 11:23
@frederoni frederoni removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jul 14, 2016
@postmechanical
Copy link

Will this make the 3.3.0 release?

@1ec5
Copy link
Contributor

1ec5 commented Jul 18, 2016

@postmechanical, no, v3.3.0 has already been released. 😃 But it will be in v3.3.1.

@frederoni frederoni force-pushed the 3697-compass-logo-attribution-insets branch from a755a32 to f32b857 Compare July 18, 2016 20:46
@frederoni frederoni merged commit ede1e71 into release-ios-v3.3.0 Jul 18, 2016
@frederoni frederoni deleted the 3697-compass-logo-attribution-insets branch July 18, 2016 21:04
1ec5 added a commit that referenced this pull request Jul 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants