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

Add support for disabling animations when presenting/dismissing modals on iOS #337

Closed
wants to merge 1 commit into from

Conversation

Jarred-Sumner
Copy link

@Jarred-Sumner Jarred-Sumner commented Feb 18, 2020

Currently, there's no way to disable the system animation for presenting modal view controllers.

if you're using the native stack navigator on iOS and you have a modal with a custom animation, it presents the new controller with the system animation, and then your modal with the custom animation will trigger – two animations, when you only meant for one to happen.

To address this, I made it so that, when presenting the next view controller, it checks if the RNSScreenView has stackAnimation set to RNSScreenStackAnimationNone, and if so,animated will be NO. It also does the reverse check when dismissing, but relying on the top screen.

  • I tested this manually in my app, though I also noticed a possible memory leak on modals that have been dismissed. I don't think the memory leak is related to this PR though.
  • I'm not totally sure about using stackAnimation for this, since stack and modal are not the same thing....but it seems like the easiest way to do this while introducing the fewest number of changes.
  • This probably should be considered a breaking change, since it might change behavior for some existing apps using the 2.0.0 beta. If merged, I would suggest bumping the version to 2.1.0 beta.

@WoLewicki
Copy link
Member

I think this behavior was added in #502. Can you confirm it?

@WoLewicki
Copy link
Member

I am closing this PR because it looks outdated. Feel free to comment if I am wrong.

@WoLewicki WoLewicki closed this Jul 29, 2020
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.

2 participants