[ios] simplified constraints and fixed regression 6954 #7084
Conversation
|
@frederoni, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1ec5, @incanus and @boundsj to be potential reviewers. |
| [self.compassViewConstraints removeAllObjects]; | ||
|
|
||
| [self.compassViewConstraints addObject: | ||
| [NSLayoutConstraint constraintWithItem:compassContainer | ||
| [NSLayoutConstraint constraintWithItem:self.compassView |
There was a problem hiding this comment.
So the compass container view isn’t constrained to anything anymore? Do we still need it?
@incanus, do you remember why we stuck the compass view inside a container view in the first place?
There was a problem hiding this comment.
@incanus, do you remember why we stuck the compass view inside a container view in the first place?
Indeed! When you don't have it in a container, but constrain it to the corner using margins, it jiggles as the constraints attempt to keep up with the actual margins as they change. You'd think this would not be the case for a round image, but it is. So constraining the container keeps it solidly in place, changing only for device rotation and superview frame changes, while letting the round part rotate about its center, which is firmly constrained to the (from its perspective) unchanging container.
So, if you get rid of the container, make sure the compass doesn't jiggle. It may have been a major iOS version-specific bug, so we should check all supported versions.
mapbox/DEPRECATED-mapbox-ios-sdk@f7fdc9d#diff-cc99abd5af5db8f48b362bcac3e8c307R514
There was a problem hiding this comment.
When you don't have it in a container, but constrain it to the corner using margins, it jiggles as the constraints attempt to keep up with the actual margins as they change.
Indeed that might've been the case in the early retina days. I've seen the same problems when using half pixels (compass is 34.5 w/h).
I removed the container view and the w/h constraints because UIImageView has intrinsic content size when initialized with an image or sizeToFit has been called and I can't see any regression.
There was a problem hiding this comment.
This simplification looks great, but before merging, can we get testing on iOS 8 (and ideally iOS 7, if possible, since it's still supported)?
|
The original compass jiggle issue came about on iOS 7.
|
|
Unable to test on iOS7 so far but 8, 9 and 10 looks 👍
|
Fixes regression #6954 (comment)
There's no need to attach the constraints to the superview as mentioned in the comment. This fixes the compass being out of bounds when not in full screen.
@1ec5 👀