-
Notifications
You must be signed in to change notification settings - Fork 356
iOS: Update top view controller retrieval for iOS multi-window #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
iOS: Update top view controller retrieval for iOS multi-window #190
Conversation
- Refactored getTopPresentedViewController to use the active UIWindowScene for keyWindow selection, improving compatibility with iOS multi-window environments. - Using `[UIApplication sharedApplication].keyWindow` is deprecated since iOS 13
latest stable release of cordova-ios supports iOS 11+, if we want to drop support for iOS < 13 for the plugin we should wait until cordova-ios 8 is released and add engine tags in plugin.xml and package.json to require that version, otherwise we should keep the old code with version checks. |
Is somewhere documented, which cordova-ios supports which minimum iOS version? |
I see it's in the iOS Platform Guide |
You are correct, I forgot to support minimum iOS 11. I corrected this. |
src/ios/CDVNotification.m
Outdated
} | ||
} | ||
} else { | ||
// Fallback for iOS 11-12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plugin technically supports iOS 8+ (and dropping iOS 7 and older is what caused the still unreleased 3.x version bump), so better just say "fallback for older versions" instead of giving specifics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand, where the minimum iOS version is configured. I looked in package.json
, but found only this:
"cordovaDependencies": {
"3.0.0": {
"cordova": ">100"
}
What dos "cordova": ">100"
mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not specified anywhere at the moment, when the version was bumped to 3.0.0-dev and iOS 7 and older code was removed we should have added a cordova-ios >= 4 and the > 100 should have been moved to a new 4.0.0 entry.
The > 100 was supposed to be a protective measure to prevent installing a version when a major bump happens or something, that has only caused issues as we usually forget to update it and it prevents the installing on the version of the plugin.
you can read about the proposal here
https://lists.apache.org/thread/lfnxoss879yqw0rbljqycms7r0g6vgy7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cordova-ios 6 is the first one, which supports iOS 11. So we should support minimum this cordova-ios version. But for cordova-ios 8.0.0, we should raise the minimum iOS version to 15, because you can't test your app on older OSs. XCode doesn't let you install older simulators and according to ChatGPT, you can't install with the oldest supported XCode 16 your app on a device which has an iOS version older than 15. I started a discussion for this here: apache/cordova#566
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest release of the plugin supports all iOS versions, the unreleased 3.0.0 would support iOS 8+ as it removed old UIAlertView code, and the Cordova-iOS version that supports iOS 8+ is 4.0.0. We could require Cordova-iOS 6 if we wanted, but there is no technical reason to do so as the code you added will work on older versions too.
Not everybody submit apps to the Apple App Store, so in those cases there are no Xcode restrictions and people can have real devices to test with those older iOS versions.
About the min iOS version discussion, I think you should send a mail to the Apache dev list, GitHub discussions is more for user questions, since this is a formal discussion from a contributor it should go to the dev list, but some other contributors have been opposed to raising the deployment target, it has already been raised to 13 and was not easy to get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not everybody submit apps to the Apple App Store, so in those cases there are no Xcode restrictions and people can have real devices to test with those older iOS versions.
Do this really do people? Do know real examples, or this just a guess? I mean also with the old os versions.
Thanks for the hint with the dev list, I will try this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plugin technically supports iOS 8+ (and dropping iOS 7 and older is what caused the still unreleased 3.x version bump), so better just say "fallback for older versions" instead of giving specifics
I fixed it by not mention any version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not everybody submit apps to the Apple App Store, so in those cases there are no Xcode restrictions and people can have real devices to test with those older iOS versions.
Do this really do people? Do know real examples, or this just a guess? I mean also with the old os versions.
Thanks for the hint with the dev list, I will try this.
Yeah, since mac computers are expensive there are people not being able to buy them so often and use older versions with older Xcode versions since Apple also requires certain OS versions for using latest Xcode versions.
Apple allows "in house" distribution of apps for enterprise customers that since are hosted on the enterprise servers don't have Xcode version restrictions.
Also allows installing the apps from Xcode directly, even without a paid developer license nowadays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks for the information. So it's better to support the older ones, though they are not distributable to the app store.
…w usage on older iOS - This plugin currently supports OSs older than iOS 11
[UIApplication sharedApplication].keyWindow
is deprecated since iOS 13'keyWindow' is deprecated: first deprecated in iOS 13.0 - Should not be used for applications that support multiple scenes as it returns a key window across all connected scenes
Platforms affected
Motivation and Context
Description
Testing
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)