fix(ios): #100 - Resolve orientation issues on iOS 16+

jaydrogers commented 2 years ago

Platforms affected

iOS 16 or newer

Motivation and Context

It fixes #100

Description

Screen orientation is not working for iOS 16 devices. This fix is confirmed by many other users on the issue thread.

Testing

I forked this repo and tested it internally on one of our client apps. I was able to have the issue, implement this fix, and confirm the issue no longer existed: https://github.com/521dimensions/cordova-plugin-screen-orientation

Checklist

[ ] 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

breautek commented 2 years ago

Android tests passes :+1: The browser tests is failing, but I know it's not caused by this PR, so :+1:

I do think the iOS test that is failing needs to be looked at, as mentioned https://github.com/apache/cordova-plugin-screen-orientation/pull/102#discussion_r977062042

jaydrogers commented 2 years ago

To be straight with you, I am not a swift developer at all.

I just took what @msmtamburro suggested in this comment: https://github.com/apache/cordova-plugin-screen-orientation/issues/100#issuecomment-1212391656

Once I applied @msmtamburro's fix, it worked for me.

Before I applied this fix, the app was not detecting screen orientation at all. Once I put the fix in, it worked as expected.

If you find issues with this PR

By all means, do not merge it
I only attempted this PR to help the maintainers & community if this was the solution
If there is a better solution, I'm all for it!

Thanks for your work!

breautek commented 2 years ago

No worries, I'm not an really iOS developer either, I don't even personally own a mac. So unfortunately I cannot be much help when it comes to testing or experimenting with different solutions.

Last night I fixed the chrome test, so if you rebase the PR, the chrome test should start passing.

I think the solution does work if running XCode 14 (based on my reading of the Apple documentation). I understand that the code won't compile unless if you're running XCode 14, because iPhoneSDKs are tied to the xcode version. I believe #if __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5 // Xcode 14 and iOS 16, or greater is meant to address this, but it doesn't seem to be working properly (and I personally don't why or what is the alternative solution).

What we could do perhaps is to simply not use this compiler macro check and treat this as a breaking change PR. We will need to update our GitHub Actions to use XCode 14 or later first before we can accept this PR. Our current testing environment is outdated anyway, considering that Apple doesn't even allow XCode 11 to be used for publishing apps.

@erisu or @NiklasMerz Do you have any thoughts on this?

msmtamburro commented 2 years ago

Hi @breautek That __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5 check allows the code to compile on either Xcode 14, or (the still valid) Xcode 13.4.1. Worth noting, it will not compile on the earliest beta versions of Xcode 14 (which are not valid for app submission anyway).

Yeah, Xcode 12 (and earlier) is not valid for publishing apps to the Apple App Store anymore. 13 will probably be dropped by Apple in the Spring, per tradition.

jansgescheit commented 2 years ago

I have tested the PR changes once in the iPhone simulator. With an iPhone on iOS 16, it now works as expected. However, it is now broken for older iOS versions. Both was compiled with XCode14

laughsky commented 2 years ago

if use xcode 13，the following is better:

if (@available(iOS 16.0, *)) {
    #if __IPHONE_OS_VERSION_MAX_ALLOWED > __IPHONE_15_5
        SEL supportedInterfaceSelector = NSSelectorFromString(@"setNeedsUpdateOfSupportedInterfaceOrientations");
        [self.viewController performSelector:supportedInterfaceSelector];
    #endif
} else {
    [UINavigationController attemptRotationToDeviceOrientation];
}

anusha-kaparapu commented 1 year ago

Hi @oofaish . Glad that fix was ready, Can i check when can we expect the "fix(ios): #100 - Resolve orientation issues on iOS 16+" changes to be released?

Thank you in advance.

breautek commented 1 year ago

Superceded by https://github.com/apache/cordova-plugin-screen-orientation/pull/107

Thank you for your efforts, but I think it this PR is obsolete so I'll be closing it.

anusha-kaparapu commented 1 year ago

Hi @oofaish and @breautek is there any update for #107 PR? When can we expect this PR Changes.

Thank you in advance.

sujeet-matchps commented 1 year ago

@erisu @jaydrogers @breautek @msmtamburro @jansgescheit @laughsky Hey Team, Do you guys are planning any release of this hotfix, If not what is the alternative solution to which I can resolve this like patch work in a plugin or a different plugin?

breautek commented 1 year ago

Hey Team, Do you guys are planning any release of this hotfix, If not what is the alternative solution to which I can resolve this like patch work in a plugin or a different plugin?

I assume you're talking about https://github.com/apache/cordova-plugin-screen-orientation/pull/107 rather than this PR but there still a couple of items in the TODO list for patch 3.0.3.

In the meantime however, https://github.com/apache/cordova-plugin-screen-orientation/pull/107 has been merged in so the changes can be tested by installing from git:

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

You can also pin it to the specific commit if you want to guarantee that a newer version won't be installed automatically without your knowledge:

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

As a PMC member, I have to disclaim that installing Apache packages directly from git have not been voted on for release and thus aren't suitable for production use.

livia-vasconcelos commented 1 year ago

@breautek I was looking at the TODO list for the patch 3.0.3 and now it seems completed - 100%. In that case, do you have a due date for this release? https://github.com/apache/cordova-plugin-screen-orientation/milestone/1

Thank you!

sujeet-matchps commented 1 year ago

https://github.com/apache/cordova-plugin-screen-orientation/milestone/1

Thanks @breautek I have tested this locally, and It looks super dam cool, but I can't release it for production using GitHub link. Could help me with production release patch work?

NiklasMerz commented 1 year ago

I just started the release process and will probably prepare a release next week. Please follow the mailing list to keep updated https://lists.apache.org/list.html?dev@cordova.apache.org

sujeet-matchps commented 1 year ago

I just started the release process and will probably prepare a release next week. Please follow the mailing list to keep updated https://lists.apache.org/list.html?dev@cordova.apache.org

@NiklasMerz Thanks for the update...

MySelfMaddy commented 1 year ago

Hai @breautek / @NiklasMerz I have tested this locally, and It looks Good, but I can't release it for production using GitHub link. Could help when we expect latest patch release?

breautek commented 1 year ago

Hai @breautek / @NiklasMerz I have tested this locally, and It looks Good, but I can't release it for production using GitHub link. Could help when we expect latest patch release?

@NiklasMerz has started the release process which involves creating a Discuss thread to give an opportunity for people to raise potentially blocking concerns of the release, as per Apache policies.

Assuming no rejections, Nik will create a Vote thread on the mailing list as per Apache Policies which must stay active for a minimum of 72 hours to account for people across all geographical locations.

Non-PMC members are also encouraged to participate in discussions and vote threads, which you may choose to do so by subscribing to the Dev Mailing List.

Please keep in mind that Nik is volunteering his free time to do handle the release, so I can't give any concrete dates.

anusha-kaparapu commented 1 year ago

Hi @NiklasMerz I have tested this locally, Fix is working as expected, When can we expect the 3.0.3 release version?

breautek commented 1 year ago

Hi @NiklasMerz I have tested this locally, Fix is working as expected, When can we expect the 3.0.3 release version?

See https://github.com/apache/cordova-plugin-screen-orientation/pull/102#issuecomment-1438519675

Locking this issue as we are beginning on simply repeating ourselves now.

NiklasMerz commented 1 year ago

The release is out now https://cordova.apache.org/news/2023/02/24/screen-orientation-plugin-3.0.3.html

apache / cordova-plugin-screen-orientation