fix(iOS): allow to interactively swipe down the modal
Summary:
This PR allows to interactively close the modal using the swipe down gesture.
It fixes 5 year old issue: #29319
In short it removes modalInPresentation which according to the documentation causes: "UIKit ignores events outside the view controller’s bounds and prevents the interactive dismissal of the view controller while it is onscreen.".
It also adds another delegate event to call onRequestClose whenever modal is closed by gesture.
https://github.com/user-attachments/assets/8849ecba-f762-47ec-a28b-b41c1991a882
Changelog:
[IOS] [BREAKING] - Allow to interactively swipe down the modal. Implementing onRequestClose is now required so that the modal doesn't end up in a corrupted state on iOS.
Test Plan:
Test if swiping down the modal calls onRequestClose
@rshest has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
This is only changing the new architecture component. Should we do the same for legacy arch for consistency?
This also needs a breaking changelog entry, and perhaps some more warnings. If you do not implement onRequestClose, the user can dismiss it and the application will get stuck.
@javache I've cloned the implementation for old architecture. Additionally I've added warnings and updated types. Also the PR is now marked as BREAKING
Let's update the changelog a bit to make it explicit what developers need to use if they use Modal on iOS. Otherwise looks good.
@javache Let me know if I should add something more to this description:
[IOS] [BREAKING] - Allow to interactively swipe down the modal. Implementing onRequestClose is now required so that the modal doesn't end up in a corrupted state on iOS.
Hey @javache @cipolleschi is there anything else needed to get this PR merged?
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Thanks for the ping here. I think we can safely make this a non-breaking change by making this controlled through an iOS-specific prop (allowSwipeDismissal) which we default to false, and we only log the new warning when that props is enabled. What do you think?
Hey @javache thanks for your suggestion, I've added this as a prop. Can you re-review?
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Can you also send a PR for the docs? https://reactnative.dev/docs/modal
The current docs seem incorrect with the description in the PR here:
On iOS, this callback is called when a Modal is being dismissed using a drag gesture when presentationStyle is pageSheet or formSheet
According to the docs, the previous callback would've also been delivered from presentationControllerDidAttemptToDismiss:
@javache I've added PR to the docs: https://github.com/facebook/react-native-website/pull/4658
I'm not sure If I understand your concern about current documentation state because before the onRequestClose would be also called on iOS but users had to update state in there to hide the modal.
With this new prop we allow users to fully swipe down the modal and afterwards we call onRequestClose so in both cases its called
@javache merged this pull request in facebook/react-native@28986a7599952a77b8b8e433f72ca837afde310e.
This pull request was successfully merged by @okwasniewski in 28986a7599952a77b8b8e433f72ca837afde310e
When will my fix make it into a release? | How to file a pick request?