AndreaMiotto / PartialSheet

A SwiftUI Partial Sheet fully customizable with dynamic height
https://github.com/AndreaMiotto/PartialSheet/wiki
MIT License
1.73k stars 194 forks source link

[Bugfix] Cancel any dismissal closures that don't fire when the next presentation occurs #144

Closed maciesielka closed 1 year ago

maciesielka commented 2 years ago

What's the issue?

Using the enabled(_:) PSCoverStyle, it's possible for users to dismiss a presented PartialSheet by tapping the space behind the content. However, this makes it very easy to quickly dismiss the sheet just as it's presented, and then also present the sheet again just as it's dismissed.

When performing this set of actions in quick succession, it's possible to get the sheet to be stuck as dismissed because the isPresented binding fails to update.

This is referred to as the "rage-tapping" behavior that's partially solved by PSButton. While that helps to prevent presentations from happening too closely to one another, it doesn't help to solve the same issue for presenting too close to a dismissal.

Observe strange dismissal behavior below using the Basic Example that uses PSButton -- the content is removed after the view is presented or isn't presented at all:

https://user-images.githubusercontent.com/4276827/158268824-d61aa93c-d11f-41b0-8080-9297b1070feb.mp4

How do the changes here fix the problem?

My guess to the underlying reasoning here is that the onDismiss closure is always nil-ed out shortly after setting isPresented to false. If the user attempts to present the sheet again before the delay is reached, it'll set that closure to nil on a presented view, and when that view is dismissed it will not be able to update the isPresented binding properly.

By cancelling any incomplete action when a presentation occurs, I hope the rage-tapping behavior is more gracefully handled.

AndreaMiotto commented 2 years ago

thank you Mike, I'll give it a look asap and Ill come back to you

cao13646 commented 2 years ago
          if self?.isPresented  == false {
                self?.content = EmptyView().eraseToAnyView()
                self?.onDismiss = nil
                }
netspencer commented 2 years ago

any update on the status here? i would greatly benefit from this

AndreaMiotto commented 1 year ago

At the current state of the project I can't reproduce the problem, is it still a thing? If so I will reopen the pr