BastiaanJansen / toast-swift

Customizable Swift Toast view built with UIKit. 🍞
MIT License
467 stars 77 forks source link

Timer invalidation breaking pan gesture #18

Closed julianpomper closed 10 months ago

julianpomper commented 1 year ago

With the current version the closeTimer?.invalidate() in the pan gesture handler is breaking the gesture with the following error: [SystemGestureGate] <0x143508050> Gesture: System gesture gate timed out.. The gesture handler is then not called anymore and the toast is then frozen and is not disappearing anymore. Does someone else has a similar behavior? It tested it on Xcode 14.0.1, 14.1 - iOS 16 Simulator and device with SwiftUI and UIKit in a plain project.

If I remove the invalidation everything works well, but I cannot figure out why this happens.

Nikoloutsos commented 1 year ago

Hi @julianpomper,

While developing a PR I realised a similar behaviour with pan gesture that made me wonder.

class ViewController: UIViewController {
    override func viewDidLoad() {
        super.viewDidLoad()
        DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
            Toast.text("I am a text").show()  // pan gesture is enabled by default
        }
    }
}

When running the application with the above the pan gesture was stopped working or was not working all the times.

As bad as it sounds there is a hidden problem with the above code. Toast.text() returns a Banner object which is not referenced by any variable (Hence ARC is 0).

And because of that the object is deallocated from memory. Thus, the panGesture is not going to be called any more (as it is living in Toast class).

The gesture works well for me if I convert the above code to:

class ViewController: UIViewController {
    var toast: Toast! // Adds 1 to ARC so that the Toast is not deallocated.
    override func viewDidLoad() {
        super.viewDidLoad()
        self.toast = Toast.text("I am a text")
        DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
            self.toast.show()  // pan gesture is enabled by default
        }
    }
}

@BastiaanJansen I do have some ideas but I want to hear if you are thinking of any solution first 😄

BastiaanJansen commented 1 year ago

Hi @Nikoloutsos

Thanks for the explanation. Swift and iOS development is not my main stack, so any suggestions and PR's are appreciated!

Of course always having to define a toast variable isn't preferred. So I would like to hear your ideas.

Nikoloutsos commented 1 year ago

No worries, OOC may I know what's your main stack 😄?

My idea to tackle this problem is simple. I think the lifetime of the Toast must be the same with the ToastView. ToastView has a guaranteed reference from topViewController that is displaying it since it is its subview.

So a quick solution would be to refactor the ToastView to strongly reference the Toast instance. That way when toastView is removed from controller the Toast instance will be deallocated from memory and will have fulfil it's purpose.

But I want to think it more and see if I come up with a better solution than this

BastiaanJansen commented 1 year ago

My main stack is Java Spring and Angular, so I don't have to worry about object lifetimes as much because Spring IoC takes mostly care of it.

Your solution sounds fine to me. I'll look at the PR when it's ready!

efehelvaci commented 11 months ago

In AppleToastView, making the toast variable strongly referenced and overriding removeFromSuperview to prevent the reference cycle fixes the issue.

private var toast: Toast?

public override func removeFromSuperview() {
  super.removeFromSuperview()
  self.toast = nil
}