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: Make all screens after presentation: 'modal' as modal unless specified #11860

Conversation

groot007
Copy link
Member

Make all screens after presentation: 'modal' as modal unless specified

@codecov-commenter
Copy link

codecov-commenter commented Feb 29, 2024

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 77.02%. Comparing base (b146aed) to head (dcdf90d).

Files Patch % Lines
packages/stack/src/utils/getModalRoutesKeys.ts 77.77% 2 Missing ⚠️
...kages/native-stack/src/utils/getModalRoutesKeys.ts 88.88% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11860      +/-   ##
==========================================
+ Coverage   76.99%   77.02%   +0.02%     
==========================================
  Files         207      209       +2     
  Lines        6151     6171      +20     
  Branches     2420     2430      +10     
==========================================
+ Hits         4736     4753      +17     
- Misses       1361     1364       +3     
  Partials       54       54              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

netlify bot commented Feb 29, 2024

Deploy Preview for react-navigation-example ready!

Name Link
🔨 Latest commit c0b5b9c
🔍 Latest deploy log https://app.netlify.com/sites/react-navigation-example/deploys/65eb273da0b5b30008a2fe6c
😎 Deploy Preview https://deploy-preview-11860--react-navigation-example.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other thing to check is if we also need to consider transparentModal along with modal

@groot007
Copy link
Member Author

groot007 commented Mar 4, 2024

The other thing to check is if we also need to consider transparentModal along with modal

from my perspective automatically stack for 'modal' looks logical but 'transparentModal' can be a little mess with all these transparent screens stack

@satya164
Copy link
Member

satya164 commented Mar 4, 2024

from my perspective automatically stack for 'modal' looks logical but 'transparentModal' can be a little mess with all these transparent screens stack

We shouldn't mark all screens after transparentModal as transparentModal, but as regular modal. The idea is to avoid the issue of screens appearing behind the transparent modal even when pushed.

@satya164 satya164 merged commit f16216c into react-navigation:main Mar 8, 2024
8 checks passed
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.

None yet

3 participants