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

[ios] Fix content insets with custom edge padding. #14813

Merged
merged 5 commits into from Jun 3, 2019
Merged

[ios] Fix content insets with custom edge padding. #14813

merged 5 commits into from Jun 3, 2019

Conversation

fabian-guerra
Copy link
Contributor

Fixes #12818

@fabian-guerra fabian-guerra added bug iOS Mapbox Maps SDK for iOS needs changelog Indicates PR needs a changelog entry prior to merging. labels Jun 1, 2019
@fabian-guerra fabian-guerra added this to the release-oolong milestone Jun 1, 2019
@fabian-guerra fabian-guerra requested a review from a team June 1, 2019 01:56
@fabian-guerra fabian-guerra self-assigned this Jun 1, 2019
1ec5
1ec5 approved these changes Jun 1, 2019
View reviewed changes
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

I went ahead and consolidated the NSEdgeInsets/UIEdgeInsets→MGLEdgeInsets typedef, factored out a method that adds two edge insets together, and made the same change on macOS.

@1ec5 1ec5 added macOS Mapbox Maps SDK for macOS and removed needs changelog Indicates PR needs a changelog entry prior to merging. labels Jun 1, 2019
@1ec5
Copy link
Contributor

1ec5 commented Jun 1, 2019

Per #12818 (comment), we may want to clarify the new behavior in the documentation for this method on both platforms.

@1ec5 1ec5 requested a review from frederoni June 1, 2019 02:55
@1ec5 1ec5 mentioned this pull request Jun 1, 2019
Closed
@fabian-guerra fabian-guerra merged commit 4026451 into master Jun 3, 2019
@fabian-guerra fabian-guerra deleted the fabian-insets-12818 branch June 3, 2019 14:32
@friedbunny
Copy link
Contributor

friedbunny commented Jun 3, 2019 •
edited

Given how unreliable this behavior has been, this really feels like it should have tests... otherwise we’re going to keep repeating this cycle of breaking/fixing.

Ticketed this readily apparent need in #14827.

This was referenced Jun 11, 2019
Closed
Closed
@fabian-guerra fabian-guerra mentioned this pull request Jun 17, 2019
Closed
@1ec5 1ec5 mentioned this pull request Jul 19, 2019
Open
@1ec5 1ec5 mentioned this pull request Jul 26, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@frederoni frederoni frederoni approved these changes

@1ec5 1ec5 1ec5 approved these changes

@julianrex julianrex julianrex approved these changes

Assignees

@fabian-guerra fabian-guerra

Labels
bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Milestone
release-oolong
Development

Successfully merging this pull request may close these issues.

Content insets should be added when setting camera with custom edge padding
5 participants
@fabian-guerra @1ec5 @friedbunny @frederoni @julianrex