danielsaidi / SwiftUIKit

SwiftUIKit is a Swift SDK that adds extra functionality to Swift & SwiftUI.
MIT License
1.42k stars 56 forks source link

Setting subsequent Toasts to a context doesn't reset their timing #3

Closed daniloc closed 2 years ago

daniloc commented 3 years ago

Hello,

Lovely collection here.

Title says it all. Because of lingering asyncAfter calls, timing gets all gunked up if you present multiple toasts in sequence.

I'd be happy to contribute a fix on this myself if you had any suggestions on how to approach it within your existing design.

Thanks so much for the code, very helpful stuff.

danielsaidi commented 3 years ago

Hi @daniloc - thank you!

Yeah, I noticed it too. Thanks for creating an issue for it! 👍

I can fix it in a few days. As long as your fix doesn't change the external api, I'd be happy to look at any pr as well.

danielsaidi commented 3 years ago

Hi again,

I'm looking at the View+Toast extension and am not sure how to best do this. One thought was to keep track of all toasts and create a queue. Another to not put this responsibility on the extension, but rather just abort any current async if toast is called again before the old toast is dismissed.

However, I'm not really sure how to best manage state without changing the external api:s. The toast context would be the perfect place to store a "current toast" or async tag, but the context-based extension is just a shorthand to a function where this would not be applicable.

Any thoughts and ideas are most welcome.

lordzsolt commented 3 years ago

While it involves changing external APIs, one solution would be to immediately dismiss the old toast before presenting a new one.

The steps would be something like:

  1. Move out the dismissal from the extension method and move it into the ToastContext
  2. The duration would be moved into the ToastContext.
  3. The ToastContext.present would change to something along the lines of:

    public func present<Toast: View>(_ toast: @autoclosure @escaping () -> Toast) {
        if isActive {
            dismiss()
            dismissCancellable = nil
        }

        presentContext(toast().any())

        dismissCancellable = Just(())
            .delay(for: 2, scheduler: RunLoop.main)
            .sink { _ in
                self.dismiss()
            }
    }
danielsaidi commented 3 years ago

Hi! I have fixed this in another library of mine, using Combine, and will do so here as well. I will share the fix here when I have done so.

Thanks for reminding me 👍

danielsaidi commented 2 years ago

I have deprecated the toast part of this library, so I will not fix this.