Skip to content
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

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented Oct 15, 2024

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.

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.
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: bfb8635b7ce4a1d2f75b4331c088e7c58e9d5fd3 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: bfb8635b7ce4a1d2f75b4331c088e7c58e9d5fd3 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: bfb8635b7ce4a1d2f75b4331c088e7c58e9d5fd3 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: bfb8635b7ce4a1d2f75b4331c088e7c58e9d5fd3 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

.NET (No breaking changes)

✅ API diff vs stable

.NET (No breaking changes)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: bfb8635b7ce4a1d2f75b4331c088e7c58e9d5fd3 [PR build]

#if !os(macOS)
import UIKit
#endif

Copy link
Contributor

@stephen-hawley stephen-hawley Oct 15, 2024

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
}

Copy link
Contributor

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.

Copy link
Member Author

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.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [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
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 40 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 7 tests passed. [attempt 2] Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 7 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: bfb8635b7ce4a1d2f75b4331c088e7c58e9d5fd3 [PR build]

@rolfbjarne rolfbjarne merged commit 34d1fca into main Oct 16, 2024
17 checks passed
@rolfbjarne rolfbjarne deleted the dev/rolf/swift-in-runtime branch October 16, 2024 09:06
haritha-mohan pushed a commit that referenced this pull request Oct 19, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants