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

detachPreviousScreen is not set to false when using "presentation: 'modal'" option #12016

Closed
3 of 12 tasks
IgorVanian opened this issue Jun 12, 2024 · 7 comments
Closed
3 of 12 tasks

Comments

@IgorVanian
Copy link

IgorVanian commented Jun 12, 2024

Current behavior

Hi!

The docs say that detachPreviousScreen is set to false if presentation is set to transparentModal or modal.

It seems that it is not the case:

detachPreviousScreen = options.presentation === 'transparentModal'
? false

In my app, it's true by default on iOS and the previous screen is visible but on Android the previous screen is not visible.

Is it a mistake in the docs or in the code? Should I make a PR with detachPreviousScreen = options.presentation === 'transparentModal' || options.presentation === 'modal' ?

Expected behavior

If the docs are correct, detachPreviousScreen should be set to false when using presentation: 'modal'

Reproduction

None.

Platform

  • Android
  • iOS
  • Web
  • Windows
  • MacOS

Packages

  • @react-navigation/bottom-tabs
  • @react-navigation/drawer
  • @react-navigation/material-top-tabs
  • @react-navigation/stack
  • @react-navigation/native-stack
  • react-native-tab-view

Environment

  • I've removed the packages that I don't use
package version
@react-navigation/native 6.1.6
@react-navigation/bottom-tabs 6.3.1
@react-navigation/stack 6.3.16
react-native-screens 3.29.0
react-native-gesture-handler 2.15.0
react-native-reanimated 3.7.1
react-native-tab-view 3.1.1
react-native-pager-view 5.4.11
react-native 0.73.6
node 18.16.1
npm or yarn 1.22.17
@IgorVanian IgorVanian added the bug label Jun 12, 2024
Copy link

Hey @IgorVanian! Thanks for opening the issue. It seems that the issue doesn't contain a link to a repro.

The best way to get attention to your issue is to provide an easy way for a developer to reproduce the issue.

You can provide a repro using any of the following:

A snack link is preferred since it's the easiest way to both create and share a repro. If it's not possible to create a repro using a snack, link to a GitHub repo under your username is a good alternative. Don't link to a branch or specific file etc. as it won't be detected.

Try to keep the repro as small as possible by narrowing down the minimal amount of code needed to reproduce the issue. Don't link to your entire project or a project containing code unrelated to the issue. See "How to create a Minimal, Reproducible Example" for more information.

You can edit your original issue to include a link to the repro, or leave it as a comment. The issue will be closed automatically after a while if you don't provide a repro.

Copy link

Couldn't find version numbers for the following packages in the issue:

  • @react-navigation/native
  • @react-navigation/stack
  • @react-navigation/bottom-tabs
  • @react-navigation/drawer
  • @react-navigation/material-top-tabs
  • react-native-tab-view

Can you update the issue to include version numbers for those packages? The version numbers must match the format 1.2.3.

Copy link

The versions mentioned in the issue for the following packages differ from the latest versions on npm:

  • @react-navigation/native (found: 6.1.6, latest: 6.1.17)
  • @react-navigation/bottom-tabs (found: 6.3.1, latest: 6.5.20)
  • @react-navigation/stack (found: 6.3.16, latest: 6.3.29)
  • react-native-tab-view (found: 3.1.1, latest: 3.5.2)

Can you verify that the issue still exists after upgrading to the latest versions of these packages?

@satya164
Copy link
Member

The docs say that detachPreviousScreen is set to false

It doesn't say that it's set to false. It says that it's automatically adjusted (to the necessary value - which would be false on iOS). Unless the current behaviour is causing a bug then this doesn't seem like an issue.

Copy link

Hey! This issue is closed and isn't watched by the core team. You are welcome to discuss the issue with others in this thread, but if you think this issue is still valid and needs to be tracked, please open a new issue with a repro.

@IgorVanian
Copy link
Author

IgorVanian commented Jun 12, 2024

@satya164 thank you for the quick reply.

The docs say that detachPreviousScreen is set to false

It doesn't say that it's set to false. It says that it's automatically adjusted (to the necessary value - which would be false on iOS). Unless the current behaviour is causing a bug then this doesn't seem like an issue.

This is automatically adjusted when using presentation as transparentModal or modal to keep the required screens visible.

This clearly states that detachPreviousScreen is automatically adjusted when using presentation as transparentModal or modal to keep the required screens visible.

From my understanding, "to keep the required screens visible" it should adjust detachPreviousScreen to false. Why should it detach previous screens for a modal? I believe it should not be detached, as for transparentModal.

@satya164
Copy link
Member

to keep the required screens visible

It keeps the screen that's required to stay visible like it says.

Why should it detach previous screens for a modal

Why exactly should the screen stay visible on Android for modals?

On iOS the previous screen is visible underneath when using modal presentation style, and same for transparent modals on both platforms. What's the point of keeping it on Android when it's not even visible underneath?

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

No branches or pull requests

2 participants