Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: displaying the modal in a screen with presentation: modal on iOS #149

Merged

Conversation

fobos531
Copy link
Contributor

Fixes #125

This is actually a super simple solution, all that is needed is to just wrap the ModalStack in a FullWindowOverlay provided by react-native-screens.

This now works on iOS both with presentation: modal and regular screens.

The behavior is unchanged on Android since it is unaffected by this problem.

@CharlesMangwa I would very much appreciate if you could take a look at this and review it.

Also, there are a couple of things that could be done to modernise this library, like updating gesture handler to v2, as well as migrating to Reanimated. Would you be open to reviewing a PR doing the migration to reanimated 3, like #68?

@CharlesMangwa
Copy link
Member

hey @fobos531!

thanks a lot for the pr, that's a nice fix indeed 👍

i would be more than happy to welcome a pr that'd bring rngh v2 reanimated v3!

unfortunately, i do not have much time to work on it myself lately but if you're up for it, just let me know how i could be of any assistance.

@CharlesMangwa CharlesMangwa self-assigned this Jul 5, 2024
@CharlesMangwa CharlesMangwa merged commit 0704fa2 into colorfy-software:main Jul 5, 2024
@fobos531 fobos531 deleted the feat/fix-modal-presentation branch July 5, 2024 08:09
@fobos531
Copy link
Contributor Author

fobos531 commented Aug 6, 2024

@CharlesMangwa Would it be possible to get this published?

@fobos531
Copy link
Contributor Author

fobos531 commented Aug 12, 2024

Hello @CharlesMangwa, an important thing to note. Maybe it might be a good idea to revert this PR, because I found a use case where this implementation doesn't play well. For example, if you have a modal with a video inside (e.g. expo-video) and you use the "native controls" and want to go full screen, or change the playback speed, it will not work, because the FullWindowOverlay is placed at the root of the view hierarchy, so it seems like the native views are then not displayed properly. Therefore, it might be a good idea to revert this PR until a better solution is found.

@CharlesMangwa
Copy link
Member

hey @fobos531! thanks for looking into this and letting me know, much appreciated! reverting this pr back for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] open modal when screen has options presentation: "modal"
2 participants