react-native icon indicating copy to clipboard operation
react-native copied to clipboard

fix(iOS): allow to interactively swipe down the modal

Open okwasniewski opened this issue 8 months ago • 5 comments

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

okwasniewski avatar May 20 '25 20:05 okwasniewski

@rshest has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar May 21 '25 08:05 facebook-github-bot

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 avatar May 21 '25 09:05 javache

@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

okwasniewski avatar May 21 '25 16:05 okwasniewski

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 avatar May 22 '25 15:05 javache

@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.

okwasniewski avatar May 25 '25 12:05 okwasniewski

Hey @javache @cipolleschi is there anything else needed to get this PR merged?

okwasniewski avatar Jun 11 '25 12:06 okwasniewski

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Jun 13 '25 13:06 facebook-github-bot

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?

javache avatar Jun 13 '25 15:06 javache

Hey @javache thanks for your suggestion, I've added this as a prop. Can you re-review?

okwasniewski avatar Jun 16 '25 14:06 okwasniewski

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Jun 16 '25 14:06 facebook-github-bot

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 avatar Jun 17 '25 10:06 javache

@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

okwasniewski avatar Jun 18 '25 11:06 okwasniewski

@javache merged this pull request in facebook/react-native@28986a7599952a77b8b8e433f72ca837afde310e.

facebook-github-bot avatar Jun 18 '25 14:06 facebook-github-bot

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?

react-native-bot avatar Jun 18 '25 14:06 react-native-bot