-
Notifications
You must be signed in to change notification settings - Fork 516
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
[StoreKit] Bind AppStore.requestReview. Fixes #21410. #21441
Conversation
The existing Objective-C class to request an App Store review (SKStoreReviewController) is deprecated in Xcode 16+, and it doesn't even work on the corresponding OS versions. The replacement API is Swift-only, but luckily it's a very simple API (just a static method), so it's possible to bind it manually. This required a few other changes/improvements: * Add support for Swift code in our runtime. * Just to keep the changes to a minimum, bump the min OS version for legacy code to match the .NET min OS versions. This is because our build logic uses the legacy min versions when compiling native code (a more involved fix would be to update all the build logic to build native code to use the .NET min OS versions, but that's not the point of this PR, so I took the easy route). I've tested the method locally, and it seems to work fine, but I've still marked it as experimental for now. There are no unit tests because calling the method will put up a dialog, which won't work correctly in unit tests. Fixes #21410.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
✅ API diff for current PR / commit.NET (No breaking changes)✅ API diff vs stable.NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
#if !os(macOS) | ||
import UIKit | ||
#endif | ||
|
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 know that this matters a lot, but you can simplify this a little bit with a type alias for the platform UI type:
#if os(macOS)
typealias PlatformScene = NSViewController
#elseif !os(tvOS)
typealias PlatformScene = UIWindowScene
#endif
@objc(XamarinSwiftFunctions)
public class XamarinSwiftFunctions : NSObject {
#if !os(tvOS)
@MainActor
@objc(requestReview:)
@available(macOS 13, iOS 16, macCatalyst 16, *)
public static func StoreKit_RequestReview(scene: PlatformScene)
{
AppStore.requestReview(in: scene)
}
#endif
}
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.
You can also use #if canImport(UIKit)
instead of a specific platform check around import
statements.
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 created a new PR for these changes: #21452.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 99 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
The existing Objective-C class to request an App Store review (SKStoreReviewController) is deprecated in Xcode 16+, and it doesn't even work on the corresponding OS versions. The replacement API is Swift-only, but luckily it's a very simple API (just a static method), so it's possible to bind it manually. This required a few other changes/improvements: * Add support for Swift code in our runtime. * Just to keep the changes to a minimum, bump the min OS version for legacy code to match the .NET min OS versions. This is because our build logic uses the legacy min versions when compiling native code (a more involved fix would be to update all the build logic to build native code to use the .NET min OS versions, but that's not the point of this PR, so I took the easy route). Fixes #10659. I've tested the method locally, and it seems to work fine, but I've still marked it as experimental for now. There are no unit tests because calling the method will put up a dialog, which won't work correctly in unit tests. Fixes #21410. Fixes #10659.
The existing Objective-C class to request an App Store review (SKStoreReviewController) is deprecated in Xcode 16+, and it doesn't even work on the corresponding OS versions.
The replacement API is Swift-only, but luckily it's a very simple API (just a static method), so it's possible to bind it manually.
This required a few other changes/improvements:
Add support for Swift code in our runtime.
Just to keep the changes to a minimum, bump the min OS version for legacy code to match the .NET min OS versions. This is because our build logic uses the legacy min versions when compiling native code (a more involved fix would be to update all the build logic to build native code to use the .NET min OS versions, but that's not the point of this PR, so I took the easy route). Fixes Use .NET-specific min OS versions to build native code for .NET #10659.
I've tested the method locally, and it seems to work fine, but I've still marked
it as experimental for now. There are no unit tests because calling the method will
put up a dialog, which won't work correctly in unit tests.
Fixes #21410.
Fixes #10659.