feat(ios): add support for ios 16

oofaish commented 2 years ago

Platforms affected

Apple devices running iOS 16

Motivation and Context

resolves #100

Currently orientation is broken on iOS 16 with Setting UIDevice.orientation is not supported. Please use UIWindowScene.requestGeometryUpdate(_:)

Description

deprecate on whether iOS 16 is available - either use the old method or the new one.

Testing

confirmed on iPhone 11 running iOS 15 and iPhone 13 Pro running iOS 16. I have not confirmed all features work, but simply setting orientations works - the old pre iOS 16 should be unaffected.

Checklist

[x] I've run the tests to see all new and existing tests pass
[ ] I added automated test coverage as appropriate for this change
[x] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
[x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
[x] I've updated the documentation if necessary

Theblood commented 2 years ago

Please Confirm this PR!!

jindrichkuba commented 2 years ago

can u please release this change?

webfletch commented 2 years ago

What is holding up the release of this change?

fernandaj222 commented 2 years ago

Hey, do you have any news about the merge of this PR? or something about the fix to screen orientation on IOS?

thmclellan commented 2 years ago

Thanks @oofaish for sharing your fix! I've been testing on iOS 16.1 on a physical iPhone 13 Pro Max (Capacitor 4.4, Ionic 5, Angular 12, XCode 14.1, Ventura 13.0) and noticed two issues:

1 - Starting from primary portrait without my screen locked, calling window.screen.orientation.lock("landscape") causes a strange double rotation (see attached video). Calling window.screen.orientation.lock("landscape-primary") works as expected.

From debugging, it seems L68 (UIInterfaceOrientationMaskLandscapeLeft) is getting called in the general landscape scenario, while L74 UIInterfaceOrientationMaskLandscapeRight is getting called on landscape-primary.

2 - Calling window.screen.orientation.unlock() is causing a requestGeometryUpdate error in Xcode:

2022-11-08 11:44:59.124078-0800 App[3407:1282732] [Orientation] BUG IN CLIENT OF UIKIT: Setting UIDevice.orientation is not supported. Please use UIWindowScene.requestGeometryUpdate(_:)

It seems L93 is being called on iOS 16: [[UIDevice currentDevice] setValue:[NSNumber numberWithInt:_lastOrientation] forKey:@"orientation"];

XCode complains but still executes L96 (setNeedsUpdateOfSupportedInterfaceOrientations) and presumably that does the unlock.

This is minor stuff but wanted to share what I found while testing. Thanks again

https://user-images.githubusercontent.com/9682900/200661364-19f4e0f9-6cd0-4bd1-b8d7-b18dd46d0ef0.mov

oofaish commented 2 years ago

thanks @thmclellan I can have a look in the next couple of days

oofaish commented 2 years ago

@thmclellan the latest commit in the branch fixes the double rotation issue - the code is definitely a bit messy now - but seems to be working fine on the cases I have tried.

thmclellan commented 2 years ago

Thanks @oofaish for these changes. Tested on iOS 16 physical iPhone 13 Pro Max and the "landscape" lock is working as expected now. The "unlock" call still results in the below xcode warning but it's working fine from a user standpoint:

[Orientation] BUG IN CLIENT OF UIKIT: Setting UIDevice.orientation is not supported. Please use UIWindowScene.requestGeometryUpdate(_:))

undimon commented 2 years ago

[scene requestGeometryUpdateWithPreferences:(UIWindowSceneGeometryPreferencesIOS*)value16 errorHandler:^(NSError * _Nonnull error) { NSLog(@"Failed to change orientation %@ %@", error, [error userInfo]); // do nothing }];

I get this error: None of the requested orientations are supported by the view controller

marco-hd commented 2 years ago

iPhone XR / iOS 16.1.1 / XCode 14.1

Hi guys,

I can confirm:

rotation is working
double rotation is fixed

However, the unlocking doesn't happen anymore.

I have no experience with Obj-C (please bear with me), but I tried to make it work this way:

...
if (_lastOrientation != UIInterfaceOrientationUnknown) {                                        
    ((void (*)(CDVViewController*, SEL, NSMutableArray*))objc_msgSend)(vc,selector,result);
    if (@available(iOS 16.0, *)) {
        [self.viewController setNeedsUpdateOfSupportedInterfaceOrientations];
        value16 = [[UIWindowSceneGeometryPreferencesIOS alloc] initWithInterfaceOrientations:_lastOrientation];
    }
    else {
        [[UIDevice currentDevice] setValue:[NSNumber numberWithInt:_lastOrientation] forKey:@"orientation"];                      
        [UINavigationController attemptRotationToDeviceOrientation];
    }
}
...

and the _lastOrientation initialisation:

...
if (!_isLocked) {
      if (@available(iOS 16.0, *)) {
          UIWindowScene *scene = (UIWindowScene*)[[UIApplication.sharedApplication connectedScenes] anyObject];

          switch(scene.interfaceOrientation) {
              case UIInterfaceOrientationUnknown:
                  _lastOrientation = UIInterfaceOrientationMaskAll;
                  break;
              case UIInterfaceOrientationPortrait:
                  _lastOrientation = UIInterfaceOrientationMaskPortrait;
                  break;
              case UIInterfaceOrientationPortraitUpsideDown:
                  _lastOrientation = UIInterfaceOrientationMaskPortraitUpsideDown;
                  break;
              case UIInterfaceOrientationLandscapeLeft:
                  _lastOrientation = UIInterfaceOrientationMaskLandscapeLeft;
                  break;
              case UIInterfaceOrientationLandscapeRight:
                  _lastOrientation = UIInterfaceOrientationMaskLandscapeRight;
                  break;
          }

      } else {
          _lastOrientation = [UIApplication sharedApplication].statusBarOrientation;        
      }      
}
...

So, it kind of works, but the locked screen can be unlocked by pure rotation of the device. Any idea how to keep it locked until it is explicitly unlocked from the JS WebView world?

VasanthCBH commented 1 year ago

Any update about the merge of this PR or a fix for this screen orientation issue on IOS?

oofaish commented 1 year ago

Ooh thanks for the review @erisu. I can take a look. the changes that you say should not be there were due to another commit that was not in master it seems. Weird. I ll sort it out.

oofaish commented 1 year ago

@erisu here is an updated PR:

go back and fix the issue with locking not working (had to call ((void (*)(CDVViewController*, SEL, NSMutableArray*))objc_msgSend)(vc,selector,result); even in iOS16
only call setNeedsUpdateOfSupportedInterfaceOrientations at the end so we dont end up double rotating - seems to work fine
split the code into post ios16 and pre ios16 as requested.

confirmed locking/unlocking works on iOS15 and 16.

oofaish commented 1 year ago

Have a look at latest PR see what you think @erisu - appreciate all the comments.

oofaish commented 1 year ago

Hmm, looks like the tests failed again? 😡

oofaish commented 1 year ago

should I be using

#if __IPHONE_OS_VERSION_MAX_ALLOWED >= 160000

instead of

#if __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5

confirmed if I use >= 170000 on xCode vs >= 160000 it will switch branches in xCode14, but not with __IPHONE_15_5 vs __IPHONE_16_5, not the perfect test, but there it is

erisu commented 1 year ago

should I be using
#if __IPHONE_OS_VERSION_MAX_ALLOWED >= 160000
instead of
#if __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5
confirmed if I use >= 170000 on xCode vs >= 160000 it will switch branches in xCode14, but not with __IPHONE_15_5 vs __IPHONE_16_5, not the perfect test, but there it is

Awesome, the update fixes the CI and everything is now green.

The changes look great, clean, and readable to me.

The only other small nitpick that I had was the variable name value16 could be renamed to value. Since the code was split out per iOS version range, there are no more overlapping variable name usages.

But, other than that, everything is great.

oofaish commented 1 year ago

thanks @erisu - changed the variable name too

oofaish commented 1 year ago

Can someone with write access merge this PR? 🤗

erisu commented 1 year ago

Can someone with write access merge this PR? 🤗

I will merge it in by tomorrow (JST). I am waiting for about 12hrs (since my last approval) to give other PMC members a chance to leave a review.

oofaish commented 1 year ago

🙏

matthew2564 commented 1 year ago

Will this fix be getting published via NPM? The latest version there is 3.0.2 - cordova-plugin-screen-orientation.

oofaish commented 1 year ago

@erisu do I need to create a new tag? can we do this manually or should I be putting up another PR?

breautek commented 1 year ago

Will this fix be getting published via NPM? The latest version there is 3.0.2 - cordova-plugin-screen-orientation.

It will be, but we cannot provide an ETA.

do I need to create a new tag? can we do this manually or should I be putting up another PR?

Nope, the tags are all managed by the release processed.

shruti-talekar commented 1 year ago

@oofaish - I tested this solution on iPhone 13 Pro Max v16.1.1. But the fix is not working. I am getting black(BLANK) screen when device is rotated to Landscape mode. What could be the issue? Can you please help

oofaish commented 1 year ago

@shruti-talekar please open a new issue and describe the problem, how you compiled the code, any error messages, etc - my tests have been working fine I am afraid.

livesemantics commented 1 year ago

Great job guys! How do I install this fix if it's not published to npm (I'm using Capacitor 4.6)? I tried to npm install directly from github but keep getting error messages (-4058). maybe I'm doing it wrong...

breautek commented 1 year ago

Great job guys! How do I install this fix if it's not published to npm (I'm using Capacitor 4.6)? I tried to npm install directly from github but keep getting error messages (-4058). maybe I'm doing it wrong...

I'm not familiar with Capacitor, but npm install is usually not enough, it simply installs the package into node_modules, but Cordova doesn't know about it and doesn't actually installs it into your platforms.

With the Cordova CLI, you'd use the git url with cordova plugin add like so:

cordova plugin remove cordova-plugin-screen-orientation
cordova plugin add https://github.com/apache/cordova-plugin-screen-orientation.git

The first command will remove the existing plugin install so that you can reinstall the newer version. Note that installing from git are unreleased versions not suitable for production.

I'm not sure how Capacitor exposes these CLI actions, so you may have to resort to their documentation.

livesemantics commented 1 year ago

Great job guys! How do I install this fix if it's not published to npm (I'm using Capacitor 4.6)? I tried to npm install directly from github but keep getting error messages (-4058). maybe I'm doing it wrong...

I'm not familiar with Capacitor, but npm install is usually not enough, it simply installs the package into node_modules, but Cordova doesn't know about it and doesn't actually installs it into your platforms.

With the Cordova CLI, you'd use the git url with cordova plugin add like so:
cordova plugin remove cordova-plugin-screen-orientation
cordova plugin add https://github.com/apache/cordova-plugin-screen-orientation.git
The first command will remove the existing plugin install so that you can reinstall the newer version. Note that installing from git are unreleased versions not suitable for production.

I'm not sure how Capacitor exposes these CLI actions, so you may have to resort to their documentation.

Capacitor is not that much different. You simply run npm install and then npx cap synch. The problem is that I get an error when I run npm install directly from github.

I've tried: npm install https://github.com/apache/cordova-plugin-screen-orientation.git (as well as other variations) and I keep getting an errors.

breautek commented 1 year ago

I'd advise seeing if you can reproduce the error in a pure cordova sample project, and if so, create a new issue noting you're testing the current development version.

The sample project can also be included in the issue.

livesemantics commented 1 year ago

project

Thanks @breautek but I fail to see how this issue has anything to do with either cordova or capacitor. It's just a simple npm install error.

If I run npm install cordova-plugin-screen-orientation then I get the old plugin (3.02) installed with no errors. If I run npm install https://github.com/apache/cordova-plugin-screen-orientation.git then I get the following error:

npm ERR! code ENOENT npm ERR! syscall spawn git npm ERR! path git npm ERR! errno -4058 npm ERR! enoent An unknown git error occurred npm ERR! enoent This is related to npm not being able to find a file. npm ERR! enoent

breautek commented 1 year ago

npm ERR! code ENOENT
npm ERR! syscall spawn git

is the error where NPM cannot find git, so it seems you don't have git installed, or at the very least the executable isn't found in your system PATH variable.

livesemantics commented 1 year ago

variable

Well, I'm using a cloud service to build so I don't have much control over system PATH etc... I just know that it fails to install. I'm also not so sure that it's "off topic" because plugins are typically published via npm and since you guys went through all the trouble of fixing but stopped short of publishing a new version ("we cannot provide an ETA") then for me at least, it looks like this fix doesn't really help...

apache / cordova-plugin-screen-orientation