Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

Add "interactively" parameter to partial sheet presentation delegate #167

Merged
merged 2 commits into from
Jul 30, 2024

Conversation

dwroth
Copy link
Contributor

@dwroth dwroth commented Jul 30, 2024

Adds an additional boolean argument, interactively, to the partialSheetPresentationControllerDidDismissSheet() method of PartialSheetPresentationControllerDelegate. This value is set to true when the dismissal of the modal is triggered by user interaction with the modal container (tapping the darkened background area or swiping down on the modal), rather than with the contents of the model itself.

Copy link
Collaborator

@scottasoutherland scottasoutherland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM. Minor Nit.

I would love to see a test for this, but i notice the existing tests are disabled with a TODO to fix them (over a year old). I'll follow up on this.

@@ -76,7 +80,9 @@ open class PartialSheetPresentation: NSObject, UIViewControllerTransitioningDele
return nil
}

return PercentDrivenInteractiveTransition(panGestureRecognizer: partialSheetPresentationController.panGestureRecognizer)
return PercentDrivenInteractiveTransition(panGestureRecognizer: partialSheetPresentationController.panGestureRecognizer) { didFinish in
partialSheetPresentationController.didCompleteInteractiveTransition(success: didFinish)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i find both didFinish and success to be a bit confusing for names here. Maybe didDismiss ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, the API is a little confusing. The dismiss is what triggers the interactive animation, so technically, it did "dismiss" even if the user doesn't swipe all the way and cancels the interaction. The question is, was the dismissal able to complete or not.

I could refactor this to not trigger the callback in the cancelled case. Maybe that would be clearer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. That makes sense then.

I'll leave it up to you if you want to change the api.

@dwroth dwroth merged commit 5689997 into main Jul 30, 2024
1 check passed
@dwroth dwroth deleted the droth/partial_sheet_interactive_dismissal branch July 30, 2024 23:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants