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

Check supported interface orientations into account when rotating device/view layout #13900

Merged
merged 4 commits into from Feb 12, 2019

Conversation

julianrex
Copy link
Contributor

@julianrex julianrex commented Feb 8, 2019

See #13895

Since we don't provide a view controller or hooks for -[UIViewController viewWillTransitionToSize:withTransitionCoordinator:] we call [[UIDevice currentDevice] beginGeneratingDeviceOrientationNotifications] to detect device rotation.

However, we do not currently take into account the application's (and view controller's) supported interface orientations. And as such, layoutSubviews is called on each device rotation (even if the UI doesn't change).

This experimental branch/PR adds those checks:

From the Apple documentation for UIViewController.supportedInterfaceOrientations:

When the user changes the device orientation, the system calls this method on the root view controller or the topmost presented view controller that fills the window. If the view controller supports the new orientation, the window and view controller are rotated to the new orientation. This method is only called if the view controller's shouldAutorotate method returns YES.

This PR checks the top-most view controller's shouldAutorotate...

Override this method to report all of the orientations that the view controller supports. [...] The system intersects the view controller's supported orientations with the app's supported orientations (as determined by the Info.plist file or the app delegate's application:supportedInterfaceOrientationsForWindow: method) and the device's supported orientations to determine whether to rotate.

... and also checks that view controller's supportedInterfaceOrientations intersected with the orientations provided by the application (as noted above).

Note: this PR checks the top-most view controller rather than the view controller that hosts the map view.

@julianrex julianrex added bug iOS Mapbox Maps SDK for iOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Feb 8, 2019
@julianrex julianrex requested a review from a team February 8, 2019 17:08
@julianrex
Copy link
Contributor Author

@friedbunny thoughts?

platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
@julianrex julianrex added needs changelog Indicates PR needs a changelog entry prior to merging. and removed ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Feb 11, 2019
Copy link
Contributor

@friedbunny friedbunny left a comment

Choose a reason for hiding this comment

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

LGTM, besides the changelog.

// at the owning view controller (in cases where the map view may be covered
// by another view.

// UIViewController *viewController = [self viewControllerForLayoutGuides];
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we get rid of this line?

@friedbunny friedbunny added this to the release-kombucha milestone Feb 12, 2019
@julianrex julianrex removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Feb 12, 2019
@julianrex julianrex force-pushed the jrex/layoutsubviews-device-rotation branch from 0396de6 to 9b9b38c Compare February 12, 2019 03:38
@julianrex julianrex merged commit 2deebc0 into master Feb 12, 2019
@julianrex julianrex deleted the jrex/layoutsubviews-device-rotation branch February 12, 2019 04:07
@julianrex
Copy link
Contributor Author

julianrex commented May 14, 2019

As a follow-up, we're investigating whether we even need deviceOrientationDidChange: anymore. See #14661

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.

None yet

2 participants