Allows interactive dismissal of modals on iOS

[ChrisSchofieldCheckatrade]

Summary:

This pull request allows the user to dismiss a modal using either pageSheet or formSheet presentation style whilst using slide animation type on iOS with an interactive gesture by explicitly opting in by setting interactiveDismissal property. Supports both old and new architectures.

Changelog:

[GENERAL] [ADDED] - Added new property interactiveDismissal to Modal props
[ANDROID] [ADDED] - Stubs a noop android method
[IOS] [ADDED] - Allows setting of modalInPresentation value in Modal UIKit files
[IOS] [ADDED] - Adds conformance to UIAdaptivePresentationControllerDelegate for the Fabric RCTModalHostViewComponentView.
[INTERNAL] [ADDED] - Adds an example to the RNTester package

Test Plan:

Clone, run the RNTester package on iOS and Android, test the interactiveDismissal prop (for which there are instructions on the modal example screen)

iOS

Screen.Recording.2024-01-16.at.14.56.16.mov

[facebook-github-bot]

Hi @ChrisSchofieldCheckatrade!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	16,951,561	+507
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	20,334,992	+147
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 26e33a5
Branch: main

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[cipolleschi]

Hi @ChrisSchofieldCheckatrade, thanks for your contribution.
As you might have seen, yesterday, some time after you opened your PR, another PR landed that changes the structure of the Modal codebase, to fix some bugs on the onDismiss event and to make this event available for Android as well.
I do think that your contribution is a valid use case for iOS. Do you think it would be possible to adapt your changes on top of that update?

[ChrisSchofieldCheckatrade]

Hi @ChrisSchofieldCheckatrade, thanks for your contribution. As you might have seen, yesterday, some time after you opened your PR, another PR landed that changes the structure of the Modal codebase, to fix some bugs on the onDismiss event and to make this event available for Android as well. I do think that your contribution is a valid use case for iOS. Do you think it would be possible to adapt your changes on top of that update?

Absolutely. I'll refactor it.

[ChrisSchofieldCheckatrade]

@cipolleschi all done and CI passing

[cipolleschi]

Hi @ChrisSchofieldCheckatrade, thank you again to work on the PR.
The code looks good to me but, If I understand correctly, it is only working in the Old Architecture.

Have you tried with the New Architecture also? If not, could you perhaps try to implement the same behavior there as well? 🙏
Thank you so much!

[ChrisSchofieldCheckatrade]

Hi @ChrisSchofieldCheckatrade, thank you again to work on the PR.

The code looks good to me but, If I understand correctly, it is only working in the Old Architecture.

Have you tried with the New Architecture also? If not, could you perhaps try to implement the same behavior there as well? 🙏

Thank you so much!

I'll check that out and update here once done.

[ChrisSchofieldCheckatrade]

@cipolleschi All done, CI was passing I've just had to write a new commit to revert to not using src dir for specs as per:
this commit: 5e8ce6c
and subsequent revert: c92f7e5

edit:
and subsequent reintroduction: 26e33a5 😅

[Gregoirevda]

enableGesture like RNSScreen seems like a better name?

[ChrisSchofieldCheckatrade]

interactiveDismissal matches the terminology Apple uses for this feature, so I'd argue no, but happy to revise to match consensus. (Reference)

[cipolleschi]

Hi @ChrisSchofieldCheckatrade, just an update not to keep you hanging.
Last week we landed the change on modal for which I asked you to rebase and update this PR. Than, over the weekend, we had to revert the commit because it was causing issues internally.
Now, I'm about to reland a lighter version of the previous change.

The root problem is that, for internal reasons, we can't land code that changes both the Native and the JS side at the side time. We need these changes to be split in Native and JS, to make sure they will not cause any issue.

To set expectations, I can take care of that after the other changes lands, but it will take some weeks. I hope this is ok with your timelines.

[ChrisSchofieldCheckatrade]

@cipolleschi our situation is that we need this feature for something in our application, and so long as you guys intend to merge this, we can continue to simply patch react-native in our own code base.

I'm happy to continue to maintain this PR for the coming weeks if it'll help you! Just give me an update when you're happy with the changes on your end. 😀

[cipolleschi]

@ChrisSchofieldCheckatrade many thanks for your understanding, I really appreciate it.
I'll keep you in the loop to make this happen!

[ChrisSchofieldCheckatrade]

@cipolleschi has there been any movement? 😄

[cipolleschi]

Hi @ChrisSchofieldCheckatrade thanks for the contribution. We are good to proceed now.
For how things work internally, we will have to split the RP in two parts (native and JS) and land them in two different steps.

[facebook-github-bot]

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

[ChrisSchofieldCheckatrade]

Grand, anything needed on my end?

[cipolleschi]

Not for now, I think I can split it pretty easily, thanks!
One of the two commit will still be attributed to you as contributor. If you want to have both the commit attributed to you, You could split the PR in Native and JS and I'll take care of make them depend on each other internally.

@ChrisSchofieldCheckatrade

[ChrisSchofieldCheckatrade]

Not for now, I think I can split it pretty easily, thanks! One of the two commit will still be attributed to you as contributor. If you want to have both the commit attributed to you, You could split the PR in Native and JS and I'll take care of make them depend on each other internally.

@ChrisSchofieldCheckatrade

I don't mind, whichever is easier. 😄

[react-native-bot]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[cipolleschi]

@ChrisSchofieldCheckatrade Sorry for making you wait so long on this, but we can try to land it now. Would you mind to rebase on top of main, first?

[ChrisSchofieldCheckatrade]

@ChrisSchofieldCheckatrade Sorry for making you wait so long on this, but we can try to land it now. Would you mind to rebase on top of main, first?

Sure, I'll try to get to it over the next few days!

[huynamboz]

I need this

[cipolleschi]

the code is very outdated now ad we can't import this pr. if @ChrisSchofieldCheckatrade as the original author would like to work on this, I can support landing it.