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

Add compass image as a sublayer #501

Merged
merged 6 commits into from
Oct 12, 2020
Merged

Add compass image as a sublayer #501

merged 6 commits into from
Oct 12, 2020

Conversation

julianrex
Copy link
Contributor

@julianrex julianrex commented Oct 9, 2020

Fixes #500

Changing the transform on the compass button triggers -[UIView layoutSubviews] on iOS 14. This PR add a sublayer for the compass image, keeping the original UIView's transform intact.

The original issue in #500 is not fully resolved however, as the scale bar causes a similar issue (to a lesser extent).

@julianrex julianrex marked this pull request as ready for review October 9, 2020 04:26
@julianrex julianrex requested review from a team and captainbarbosa and removed request for a team October 9, 2020 04:26
@julianrex
Copy link
Contributor Author

cc @MaximAlien

@julianrex julianrex self-assigned this Oct 9, 2020
@knov knov requested a review from lloydsheng October 9, 2020 13:59
@lloydsheng
Copy link
Contributor

Since we have added an imageLayer, does MGLCompassButton still need to inherit UIImageView ?
I think there may be a better solution to this issue. In order to avoid -[UIView layoutSubviews] being called, Do we have to add an extra layer to the subview?

@julianrex
Copy link
Contributor Author

julianrex commented Oct 9, 2020

Since we have added an imageLayer, does MGLCompassButton still need to inherit UIImageView ?

Good catch; because it's called "Button" I had assumed it was a UIButton. I'll change to a plain UIView. Let's keep it as the UIImageView for the time-being, as it could be argued changing it would be a SEMVER major change.

I think there may be a better solution to this issue. In order to avoid -[UIView layoutSubviews] being called, Do we have to add an extra layer to the subview?

Something other than the compass view needs to have its transform changed. This works for both a subview and a sublayer. I chose a sublayer since it won't play a role in auto-layout.

Copy link

@MaximAlien MaximAlien left a comment

Choose a reason for hiding this comment

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

Would it be possible to back-port current fix to version 6.2 as well?

Update transform immediately.
@knov knov added this to the release-a milestone Oct 9, 2020
Copy link
Contributor

@captainbarbosa captainbarbosa left a comment

Choose a reason for hiding this comment

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

Look good, just one question - does this change any accessibility-related functionality?

@@ -57,7 +57,7 @@ workflows:
- ios-build:
matrix:
parameters:
xcode: ["11.3.1", "11.5.0", "11.6.0", "12.0.0"]
xcode: ["11.1.0", "11.3.1", "12.2.0"]
Copy link
Contributor

Choose a reason for hiding this comment

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

For next time, I'd maybe consider splitting this into a separate PR.

@julianrex
Copy link
Contributor Author

does this change any accessibility-related functionality?

No, this should be a visual change only.

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

Successfully merging this pull request may close these issues.

Call to setCamera on MGLMapView is causing too many layoutSubviews updates on iOS 14.
5 participants