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.

Sign up for GitHub

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

Jump to bottom

iOS: Screen with { presentation: "modal" } navigated to from within another react-native's Modal doesn't open #2048

Closed
bbarthec opened this issue Feb 22, 2024 · 8 comments · Fixed by #2113
Closed

iOS: Screen with { presentation: "modal" } navigated to from within another react-native's Modal doesn't open #2048

bbarthec opened this issue Feb 22, 2024 · 8 comments · Fixed by #2113
Assignees
@kkafar
Labels
Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@bbarthec
Copy link
Member

Description

https://github.com/software-mansion/react-native-screens/releases/tag/3.29.0 and #1912 are causing breaking change in behaviour of '@react-navigation/native-stack''s Screen that is mounted with { presentation: "modal" }.

Hiding the react-native's Modal that is the most top component in the view hierarchy prevents Screen with { presentation: "modal" } from showing.
My scenario:

  1. show a BottomSheet that is based on react-native's Modal with an action button
  2. clicking the button should hide the BottomSheet and navigate to { presentation: "modal" } individual Screen
Before 3.29.0 (3.28.0 here) - desired behaviour 3.29.0 - breaking change 3.29.0 - without BottomSheet autohide
Screen.Recording.2024-02-22.at.16.17.37.mov
RPReplay_Final1708615160.mov
RPReplay_Final1708615180.MP4

Steps to reproduce

  1. Open https://snack.expo.dev/@bbarthec/react-native-screens-modal-in-modal-issues
  2. Open the snack in Expo GO (SDK 50 comes with version 3.29.0) on either physical iPhone or Simulator
  3. Open BottomSheet modal & open Screen modal - see Screen is not visible. It's probably mounted using dismissed BottomSheet modal as an anchor component.
  4. Commend out setVisible(false) from line 51 and repeat the process. See the modal Screen properly mounts.

Snack or a link to a repository

https://snack.expo.dev/@bbarthec/react-native-screens-modal-in-modal-issues

Screens version

3.29.0

React Native version

0.73

Platforms

iOS

JavaScript runtime

None

Workflow

None

Architecture

Paper (Old Architecture)

Build type

Debug mode

Device

None

Device model

No response

Acknowledgements

Yes
@github-actions github-actions bot added Repro provided A reproduction with a snack or repo is provided Platform: iOS This issue is specific to iOS labels Feb 22, 2024
@kkafar
Copy link
Member

kkafar commented Feb 24, 2024

yeah, I see why that's happening, thanks for reporting! We will include a fix in incoming release

@kimlesieur
Copy link

@kkafar Any news on an incoming fix ? I have a similar issue that I think could be related to this one :)

@hollanderbart
Copy link

The issue I described (#2085) is similar to this issue. Any news on the potential fix?

@hollanderbart
Copy link

@kkafar Can you please let us know when a possible fix is planned? Thanks!

@kkafar kkafar mentioned this issue Apr 26, 2024
Merged
2 tasks
@kkafar
Copy link
Member

kkafar commented Apr 26, 2024

Excuse me for late response, @hollanderbart, #2113 should resolve the issue.
If you have capacity, I would like to get feedback & confirmation that the patch works also in your case.

You can test out the patch by installing react-native-screens directly from GitHub, by putting following in your package.json:

"react-native-screens": "software-mansion/react-native-screens#@kkafar/fix-presenting-from-react-native-modal"

(watch for typos, as I'm writing from memory 😅)

@hollanderbart
Copy link

Thanks @kkafar I will test it out and come back to you on this.

@hollanderbart
Copy link

I've tested it and your fix works! 🎉

Thanks again @kkafar
I suppose you can merge this fix.

@kkafar
Copy link
Member

kkafar commented Apr 27, 2024

Great, thanks for checking it out!

kkafar added a commit that referenced this issue Apr 27, 2024
@kkafar
fix: issues with presenting owned modals from foreign ones (#2113)
c21de50 
## Description

Basically this is another edition of the issue #1829 (handled by #1912).
The issue comes down to the fact, that our `ScreenStack` is not aware of
all modal view controllers being in presentation,
but this time it is not aware of third-party modal view controllers
(I've named them "foreign" modals in opposite to "owned" modals).

This PR is not a comprehensive solution but rather just a patch aiming
at fixing one particular interaction reported in #2048.

I've left verbose code comments explaining the issue and suggesting
solution in the source code, including:

```
  // TODO: Find general way to manage owned and foreign modal view controllers and refactor this code. Consider building
  // model first (data structue, attempting to be aware of all modals in presentation and some text-like algorithm for
  // computing required operations).
```

Closes #2048
Closes #2085

## Changes

Trigger dissmisal of foreign modal if it is presented above `changeRoot`
modal (last modal that is to stay on stack after the updates).

## Test code and steps to reproduce

`Test2048` in `TestsExample` & `FabricTestExample`.

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
@kkafar kkafar self-assigned this Apr 27, 2024
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
@kkafar @ja1ns
fix: issues with presenting owned modals from foreign ones (software-…
e3cef0b 
…mansion#2113)

## Description

Basically this is another edition of the issue software-mansion#1829 (handled by software-mansion#1912).
The issue comes down to the fact, that our `ScreenStack` is not aware of
all modal view controllers being in presentation,
but this time it is not aware of third-party modal view controllers
(I've named them "foreign" modals in opposite to "owned" modals).

This PR is not a comprehensive solution but rather just a patch aiming
at fixing one particular interaction reported in software-mansion#2048.

I've left verbose code comments explaining the issue and suggesting
solution in the source code, including:

```
  // TODO: Find general way to manage owned and foreign modal view controllers and refactor this code. Consider building
  // model first (data structue, attempting to be aware of all modals in presentation and some text-like algorithm for
  // computing required operations).
```

Closes software-mansion#2048
Closes software-mansion#2085

## Changes

Trigger dissmisal of foreign modal if it is presented above `changeRoot`
modal (last modal that is to stay on stack after the updates).

## Test code and steps to reproduce

`Test2048` in `TestsExample` & `FabricTestExample`.

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kkafar kkafar

Labels
Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: issues with presenting owned modals from foreign ones software-mansion/react-native-screens
4 participants
@hollanderbart @bbarthec @kimlesieur @kkafar