woocommerce / woocommerce-ios

WooCommerce iOS app
https://www.woocommerce.com/mobile
GNU General Public License v2.0
312 stars 113 forks source link

Pull-to-dismiss gesture not working on UIViewControllerRepresentable implementations #7353

Open iamgabrielma opened 2 years ago

iamgabrielma commented 2 years ago

Reported at p5T066-3qr-p2#comment-13080

"Pull-to-dismiss" doesn't seem to work in Views where we are using SafariView or SafariSheetView via an UIViewControllerRepresentable. This happens in different places where we use this implementation, for example:

Buy card reader announcement Open WebViews on Inbox
_Simulator 2022-07-27 at 11 10 02 safariview not pull-to-dismiss _Simulator 2022-07-27 at 11 20 42 safariview not pull-to-dismiss

When we make the UiViewController, as per Apple docs, this launches a modal so is expected that we can use pull-to-dismiss.

peril-woocommerce[bot] commented 2 years ago
Fails
:no_entry_sign: Please add a feature label to this issue. e.g. 'feature: stats'

Generated by :no_entry_sign: dangerJS

iamgabrielma commented 2 years ago

After some testing and taking the CardReaderManualRowView as an example, it would seem that the UIViewControllerRepresentable takes all the available space (the entire sheet area).

UIViewControllerRepresentable Only UIViewControllerRepresentable within a VStack
Screenshot 2022-07-27 at 15 09 53 Screenshot 2022-07-27 at 15 11 51

Solution 1:

If we add another component to act as a handle the pull-to-dismiss behavior works fine:

content: {
  VStack {
    RoundedRectangle(cornerRadius: 8).fill(Color.gray)
    .frame(width: 60, height: 8)
    .padding(.top, 8)
    SafariSheetView(url: URL(string: manual.urlString)!)
}})

_added handler Simulator Screen Recording - iPhone 11 Pro Max - 2022-07-27 at 14 33 52

I'm not sure if this is the style we're looking for, and it would also require modifying them every time we use the SafariView, so we would want to add it to the class itself if possible. A

Solution 2:

Another possible workaround could be to embed the controller into navigation controller and dismiss by dragging the navigation bar, but we can't edit the size of the navigation bar, and by making it hidden reverts to the pull-to-refresh not working:

func makeUIViewController(context: Context) -> UIViewController {
    let nav = UINavigationController(rootViewController: SFSafariViewController(url: URL))
    return nav
}

Solution 3: SFSafariViewControllerDelegate through a Coordinator

We create a custom instance that we use to communicate changes from our view controller to other parts of the SwiftUI interface. This demo currently works, but we still have to figure out how to access pull-to-dismiss which cannot be used from SFSafariViewControllerDelegate, so we could iterate the Coordinator to use UIAdaptativePresentationControllerDelegate instead.

The problem is that UIAdaptativePresentationControllerDelegate methods won't be called because we need to pass a UIPresentationController into the coordinator, which our SafariSheetView is not, we can't subclass it as is already a subclass of UIViewControllerRepresentable in order to make work the SwiftUI view, and we can't make it conform either as is not a protocol.

struct SafariSheetView: UIViewControllerRepresentable {
    let url: URL

    func makeUIViewController(context: Context) -> UIViewController {
        let safariSheetViewController = SFSafariViewController(url: url)
        safariSheetViewController.delegate = context.coordinator
        return safariSheetViewController
    }

    func updateUIViewController(_ uiViewController: UIViewController, context: Context) {}

    func makeCoordinator() -> Coordinator {
        return Coordinator(self)
    }

    class Coordinator: NSObject, SFSafariViewControllerDelegate, UIAdaptivePresentationControllerDelegate {
        let parent: SafariSheetView

        init(_ safariSheetRepresentable: SafariSheetView) {
            self.parent = safariSheetRepresentable
        }

        // Doesn't Work for UIPresentationController
        func presentationControllerShouldDismiss(_ presentationController: UIPresentationController) -> Bool {
            true
        }

        // Doesn't Work for UIPresentationController
        func presentationControllerDidDismiss(_ presentationController: UIPresentationController) {
            print("Testing: DidDismiss")
        }

        // Works for SFSafariViewController
        func safariViewControllerDidFinish(_ controller: SFSafariViewController) {
            print("Testing: DidFinish dismiss")
        }

    }
}
joshheald commented 2 years ago

Severity: medium, bug with a workaround in low-priority area Impact: medium, estimated 6-25% of users affected

Priority: medium