-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[url_launcher] Update for UIScene compatibility #10549
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,15 +13,32 @@ private func urlParsingIsStrict() -> Bool { | |
| return URL(string: "b a d U R L") == nil | ||
| } | ||
|
|
||
| final class URLLauncherTests: XCTestCase { | ||
| final class TestViewPresenter: ViewPresenter { | ||
| public var presentedController: UIViewController? | ||
|
|
||
| private func createPlugin() -> URLLauncherPlugin { | ||
| let launcher = FakeLauncher() | ||
| return URLLauncherPlugin(launcher: launcher) | ||
| func present( | ||
| _ viewControllerToPresent: UIViewController, animated: Bool, completion: (() -> Void)? = nil | ||
| ) { | ||
| presentedController = viewControllerToPresent | ||
| } | ||
| } | ||
|
|
||
| final class StubViewPresenterProvider: ViewPresenterProvider { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we use the same prefix "Test-" above and "Stub-" here?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They don't do the same things; StubViewPresenterProvider is a stub, in that it just returns a fixed value, while TestViewPresenter tracks calls and updates its state (more like a mock). |
||
| var viewPresenter: ViewPresenter? | ||
|
|
||
| init(viewPresenter: ViewPresenter?) { | ||
| self.viewPresenter = viewPresenter | ||
| } | ||
| } | ||
|
|
||
| final class URLLauncherTests: XCTestCase { | ||
|
|
||
| private func createPlugin(launcher: FakeLauncher) -> URLLauncherPlugin { | ||
| return URLLauncherPlugin(launcher: launcher) | ||
| private func createPlugin( | ||
| launcher: FakeLauncher = FakeLauncher(), viewPresenter: ViewPresenter? = TestViewPresenter() | ||
| ) -> URLLauncherPlugin { | ||
| return URLLauncherPlugin( | ||
| launcher: launcher, | ||
| viewPresenterProvider: StubViewPresenterProvider(viewPresenter: viewPresenter)) | ||
| } | ||
|
|
||
| func testCanLaunchSuccess() { | ||
|
|
@@ -133,7 +150,8 @@ final class URLLauncherTests: XCTestCase { | |
|
|
||
| func testLaunchSafariViewControllerWithClose() { | ||
| let launcher = FakeLauncher() | ||
| let plugin = createPlugin(launcher: launcher) | ||
| let viewPresenter = TestViewPresenter() | ||
| let plugin = createPlugin(launcher: launcher, viewPresenter: viewPresenter) | ||
|
|
||
| let expectation = XCTestExpectation(description: "completion called") | ||
| plugin.openUrlInSafariViewController(url: "https://flutter.dev") { result in | ||
|
|
@@ -147,6 +165,23 @@ final class URLLauncherTests: XCTestCase { | |
| } | ||
| plugin.closeSafariViewController() | ||
| wait(for: [expectation], timeout: 30) | ||
| XCTAssertNotNil(viewPresenter.presentedController) | ||
| } | ||
|
|
||
| func testLaunchSafariViewControllerFailureWithNoViewPresenter() { | ||
| let expectation = XCTestExpectation(description: "completion called") | ||
| createPlugin(viewPresenter: nil).openUrlInSafariViewController(url: "https://flutter.dev") { | ||
| result in | ||
| switch result { | ||
| case .success(let details): | ||
| XCTAssertEqual(details, .noUI) | ||
| case .failure(let error): | ||
| XCTFail("Unexpected error: \(error)") | ||
| } | ||
| expectation.fulfill() | ||
| } | ||
|
|
||
| wait(for: [expectation], timeout: 1) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,22 +8,20 @@ import UIKit | |
| public final class URLLauncherPlugin: NSObject, FlutterPlugin, UrlLauncherApi { | ||
|
|
||
| public static func register(with registrar: FlutterPluginRegistrar) { | ||
| let plugin = URLLauncherPlugin() | ||
| let plugin = URLLauncherPlugin( | ||
| viewPresenterProvider: DefaultViewPresenterProvider(registrar: registrar)) | ||
| UrlLauncherApiSetup.setUp(binaryMessenger: registrar.messenger(), api: plugin) | ||
| registrar.publish(plugin) | ||
| } | ||
|
|
||
| private var currentSession: URLLaunchSession? | ||
| private let launcher: Launcher | ||
| /// The view presenter provider, for showing a Safari view controller. | ||
| private let viewPresenterProvider: ViewPresenterProvider | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Should we call this a
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to "The view presenter provider". Because it's just a testability abstraction around a view controller it was hard not to default to calling it a view controller. |
||
|
|
||
| private var topViewController: UIViewController? { | ||
| // TODO(stuartmorgan) Provide a non-deprecated codepath. See | ||
| // https://github.com/flutter/flutter/issues/104117 | ||
| UIApplication.shared.keyWindow?.rootViewController?.topViewController | ||
| } | ||
|
|
||
| init(launcher: Launcher = DefaultLauncher()) { | ||
| init(launcher: Launcher = DefaultLauncher(), viewPresenterProvider: ViewPresenterProvider) { | ||
| self.launcher = launcher | ||
| self.viewPresenterProvider = viewPresenterProvider | ||
| } | ||
|
|
||
| func canLaunchUrl(url: String) -> LaunchResult { | ||
|
|
@@ -58,43 +56,21 @@ public final class URLLauncherPlugin: NSObject, FlutterPlugin, UrlLauncherApi { | |
| return | ||
| } | ||
|
|
||
| guard let presenter = viewPresenterProvider.viewPresenter else { | ||
| completion(.success(.noUI)) | ||
| return | ||
| } | ||
|
|
||
| let session = URLLaunchSession(url: url, completion: completion) | ||
| currentSession = session | ||
|
|
||
| session.didFinish = { [weak self] in | ||
| self?.currentSession = nil | ||
| } | ||
| topViewController?.present(session.safariViewController, animated: true, completion: nil) | ||
| presenter.present(session.safariViewController, animated: true, completion: nil) | ||
| } | ||
|
|
||
| func closeSafariViewController() { | ||
| currentSession?.close() | ||
| } | ||
| } | ||
|
|
||
| /// This method recursively iterates through the view hierarchy | ||
| /// to return the top-most view controller. | ||
| /// | ||
| /// It supports the following scenarios: | ||
| /// | ||
| /// - The view controller is presenting another view. | ||
| /// - The view controller is a UINavigationController. | ||
| /// - The view controller is a UITabBarController. | ||
| /// | ||
| /// @return The top most view controller. | ||
| extension UIViewController { | ||
| var topViewController: UIViewController { | ||
| if let navigationController = self as? UINavigationController { | ||
| return navigationController.viewControllers.last?.topViewController | ||
| ?? navigationController | ||
| .visibleViewController ?? navigationController | ||
| } | ||
| if let tabBarController = self as? UITabBarController { | ||
| return tabBarController.selectedViewController?.topViewController ?? tabBarController | ||
| } | ||
| if let presented = presentedViewController { | ||
| return presented.topViewController | ||
| } | ||
| return self | ||
| } | ||
| } | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is copied directly from the recent |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import Flutter | ||
| import UIKit | ||
|
|
||
| /// Protocol for UIViewController methods relating to presenting a controller. | ||
| /// | ||
| /// This protocol exists to allow injecting an alternate implementation for testing. | ||
| protocol ViewPresenter { | ||
| /// Presents a view controller modally. | ||
| func present( | ||
| _ viewControllerToPresent: UIViewController, | ||
| animated flag: Bool, | ||
| completion: (() -> Void)? | ||
| ) | ||
| } | ||
|
|
||
| /// ViewPresenter is intentionally a direct passthroguh to UIViewController. | ||
| extension UIViewController: ViewPresenter {} | ||
|
|
||
| /// Protocol for FlutterPluginRegistrar method for accessing the view controller. | ||
| /// | ||
| /// This is necessary because Swift doesn't allow for only partially implementing a protocol, so | ||
| /// a stub implementation of FlutterPluginRegistrar for tests would break any time something was | ||
| /// added to that protocol. | ||
| protocol ViewPresenterProvider { | ||
| /// Returns the view presenter associated with the Flutter content. | ||
| var viewPresenter: ViewPresenter? { get } | ||
| } | ||
|
|
||
| /// Non-test implementation of ViewPresenterProvider that forwards to the plugin registrar. | ||
| final class DefaultViewPresenterProvider: ViewPresenterProvider { | ||
| private let registrar: FlutterPluginRegistrar | ||
|
|
||
| init(registrar: FlutterPluginRegistrar) { | ||
| self.registrar = registrar | ||
| } | ||
|
|
||
| var viewPresenter: ViewPresenter? { | ||
| registrar.viewController | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,2 @@ | ||
| # TODO(louisehsu): Remove deprecation check when StoreKit 2 is adopted. https://github.com/flutter/flutter/issues/116383 | ||
| - in_app_purchase_storekit | ||
| # TODO(stuartmorgan): Remove once there's a non-deprecated codepath for getting | ||
| # the view controller available on stable. | ||
| # See https://github.com/flutter/flutter/issues/174415 | ||
| - url_launcher_ios |
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.
This refers to the removal of
UIApplication.shared.keyWindow?.rootViewController?.topViewControllerright?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.
Correct, that's the substantive change here; the rest is testability.