LeoNatan / LNPopupUI

A SwiftUI library for presenting views as popups, much like the Apple Music and Podcasts apps.
MIT License
320 stars 29 forks source link

Making isBarPresented = true, while its animating closed when set to false causes the popup to become bugged until open/closed again #31

Closed Cleover closed 8 months ago

Cleover commented 8 months ago

Describe the Bug When hiding the popup if the popup is unhidden during the hide animation, it will become bugged and will not show again until it is first shown then closed again, then shown again

To Reproduce I have created a extremely basic SwiftUI app to show this bug, https://pastie.io/ukxbjx.swift

  1. Open the popup by clicking on it, or by scrolling up on it
  2. Click the button in the popup to close it
  3. While its animating closed click the button on the main screen to show it again

Expected Behavior When clicking the button

while its animating closed, the animation would be stopped, and it would re-open

Screenshots Demo App Bug: https://github.com/LeoNatan/LNPopupUI/assets/40129637/03099756-1656-4b0f-a759-913bdeb9a6e8 Prod App Bug: https://github.com/LeoNatan/LNPopupUI/assets/40129637/0b23cd29-c8c4-4048-85b4-219a95f67e0d

Extra Bug: I have not been able to replicate it, or understand what causes it however, so I do not have a demo app version. Sometimes when closing it can get stuck in a loop of infinitely closing and opening the popup.

Prod App Extra Bug: https://github.com/LeoNatan/LNPopupUI/assets/40129637/9a8edf3b-ce99-4aac-8375-e618051e9832

LeoNatan commented 8 months ago

Hello, Can you please check which version of LNPopupController you have cached in Xcode? Please try updating to the latest version. Thanks

Cleover commented 8 months ago

2.18.0, which is latest if I'm not mistaken.

image
LeoNatan commented 8 months ago

Thanks, I’ll take a look. With 2.18, it’s supposed to gate each operation/animation. I guess it’s not as air-tight as I hoped.

LeoNatan commented 8 months ago

This is such a hard problem to solve. The SwiftUI/UIKit bridge manages to hide this issue by either disabling interactions during the transition (sheet) or breaking animations (showing/hiding navigation bar). I'm really not sure what to do. Not sure how to throttle or maintain state across animations.

Cleover commented 8 months ago

Would the animation break by snapping it back to shown when interrupting the animation to close? If that's the case, I'm not personally against it, but I figure you'd prefer it to be smoother for an official release. The way it is now kinda messes with the user experience in my opinion, getting stuck in this weird shown/hidden limbo.

LeoNatan commented 8 months ago

The issue with interruptible animations is that the underlying LNPopupController framework was never really designed to operate like this. It used to be a user error to change state while a transition was happening. One was supposed to listen to state events (didPresent, didOpen, etc.) and throttle UI on their own, considering the state. With SwiftUI, this is not really possible, and I did try to make it somewhat interruptible, but never got it to 100%. This queue mechanism I added recently was supposed to hide this, but I guess it doesn't work as you say. Apple is doing something similar too (try showing+hiding+showing+hiding+showing a sheet very fast—it will go the same queue mechanism), but their hacks are better behaved than mine. :)

LeoNatan commented 8 months ago

It's even worse. Even adding a hack at the UIKit level (just testing, not really for production), which blocks touch events while the animation is running, SwiftUI is so asynchronous that it manages to get several taps on the button before UIKit gets to run and disable interactions. 🤣 SwiftUI is just peaches.

LeoNatan commented 8 months ago

OK, I did something, and at least from a quick play, it seems to work correctly so far. I'll make a new LNPopupController release, so you can test to see if it solves your issues. Events are now being coalesced, so this might be enough to prevent crazy queues.

LeoNatan commented 8 months ago

Please try with LNPopupController 2.18.1.

Cleover commented 8 months ago

Massive props, works like a charm now. https://github.com/LeoNatan/LNPopupUI/assets/40129637/868a5727-0637-4666-b755-0ff3d4a3863f I did add a small check on my end to make sure whenever its switched to false it checks if it should be set to true, and switch it back this has helped to update it right as it can after the animation to keep it on track.

.onChange(of: showPopupBar) { popupBarOpen in
    if !popupBarOpen {
        if queueManager.queue.count > 0 {
            showPopupBar = true
        }
    }
}

Huge Thank You!

LeoNatan commented 8 months ago

Great, thanks for confirming! Happy coding!

LeoNatan commented 8 months ago

There is a new version of LNPopupUI too, which will no longer display the empty image on the bar if you don't set an image.