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

Not able to navigate to a nested modal navigator #2085

Closed
hollanderbart opened this issue Mar 27, 2024 · 5 comments · Fixed by #2113
Closed

Not able to navigate to a nested modal navigator #2085

hollanderbart opened this issue Mar 27, 2024 · 5 comments · Fixed by #2113
Assignees
Labels
Missing repro This issue need minimum repro scenario Platform: iOS This issue is specific to iOS

Comments

@hollanderbart
Copy link

hollanderbart commented Mar 27, 2024

Description

When trying to open a nested modal, the screen isn't opening. After some research this is due to the __block keyword.
This seems to be wrongly fixed in this PR: 471127e

Changing the following line will fix the issue:

  __block UIViewController *changeRootController = _controller;

into

UIViewController *changeRootController = _controller;

Steps to reproduce

  1. Use Test1829 and open a nested modal

Snack or a link to a repository

not-needed

Screens version

3.30.1

React Native version

0.72.x

Platforms

iOS

Acknowledgements

Yes

@github-actions github-actions bot added the Missing repro This issue need minimum repro scenario label Mar 27, 2024
Copy link

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@github-actions github-actions bot added the Platform: iOS This issue is specific to iOS label Mar 27, 2024
@hollanderbart
Copy link
Author

hollanderbart commented Mar 27, 2024

@kkafar

Do you know why you added the __block statement here?

__block UIViewController *changeRootController = _controller;

Can you please explain it? It seems to cause an issue with accessing the changeRootController. Thanks.

I'm trying to reproduce it using the example project, but have some trouble with running the example project. #2086

@hollanderbart
Copy link
Author

@kkafar When do you plan to create a new release?

@kkafar
Copy link
Member

kkafar commented May 14, 2024

Only thing holding me back is to allocate a time for library testing before releasing a version (and actually we have slight problem on main rn, but it should be fixed today).

I would say you can expect one early next week, but this is a rough estimate.

@hollanderbart
Copy link
Author

@kkafar how are you doing? I don't want to ask you again but please don't forget ;-)

ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
…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
Labels
Missing repro This issue need minimum repro scenario Platform: iOS This issue is specific to iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants