[iOS] Using presentingViewController to dismiss Modal#24959
Conversation
facebook-github-bot
added
the
CLA Signed
janicduplessis
left a comment
janicduplessis
left a comment
There was a problem hiding this comment.
Can we use [modalHostView reactViewController] instead? It would be more constant with the rest of the code.
🤔 Originally, I want to change all And seems we can't use |
|
@janicduplessis @zhongwuzw I fell that my idiomatic iOS skills are now rusty. :( I don't remember exactly how it should work but I have to admit fixed version looks more correct to me.
|
| _dismissalBlock([modalHostView reactViewController], viewController, animated, completionBlock); | ||
| } else { | ||
| [viewController dismissViewControllerAnimated:animated completion:completionBlock]; | ||
| [viewController.presentingViewController dismissViewControllerAnimated:animated completion:completionBlock]; |
There was a problem hiding this comment.
The documentation is clear and confirms my recollections:
dismissViewControllerAnimated:completion:
Dismisses the view controller that was presented modally by the view controller.
SDKs
So, the fix looks perfectly legit to me, the only Q that prevents me from landing it:
I don't understand how/why it worked previously. Did it? (Until we find answer to this question we cannot be sure that our conclusions are correct.)
There was a problem hiding this comment.
@shergin Could be why it works, but maybe it dismisses the wrong vc sometimes.
If you call this method on the presented view controller itself, UIKit asks the presenting view controller to handle the dismissal.
Thanks for clarifiying what reactViewController is. LGTM
There was a problem hiding this comment.
If we just have one Modal, it works whatever we passed presented VC or presenting VC, but if we have multiple Modals, bug may happen, for example, VC present Modal A, ModalA present Modal B, if we call [ModalA dismissVC], what this should perform? because ModalA is the presenting VC to present ModalB,but it also the presented VC which presenting VC is the closest VC when present ModalA, I think this is why bug happened. :)
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request was successfully merged by @zhongwuzw in 284c5f0. When will my fix make it into a release? | Upcoming Releases |
Summary: Fixes #16037. We need to keep the order of Modals, if we dismiss Modal randomly(before we use hash table), some Modals may not dismiss successfully. This PR should based on #24959. ## Changelog [iOS] [Fixed] - Keep the order of Modals that we can dismiss in sequence Pull Request resolved: #24961 Differential Revision: D15621858 Pulled By: cpojer fbshipit-source-id: 964729f8f4584995f4e1dd527af4b61534d369ba
Summary: Fixes facebook#23463. Using `presentingViewController` to dismiss Modal VC directly, don't let Modal VC to find `presentingViewController`, then dismiss. ## Changelog [iOS] [Fixed] - Using presentingViewController to dismiss Modal Pull Request resolved: facebook#24959 Differential Revision: D15575571 Pulled By: cpojer fbshipit-source-id: e275e7c7fef644c06cc8e64dba5b5a5af4129192
) Summary: Fixes facebook#16037. We need to keep the order of Modals, if we dismiss Modal randomly(before we use hash table), some Modals may not dismiss successfully. This PR should based on facebook#24959. ## Changelog [iOS] [Fixed] - Keep the order of Modals that we can dismiss in sequence Pull Request resolved: facebook#24961 Differential Revision: D15621858 Pulled By: cpojer fbshipit-source-id: 964729f8f4584995f4e1dd527af4b61534d369ba
6 participants
Summary
Fixes #23463.
Using
presentingViewControllerto dismiss Modal VC directly, don't let Modal VC to findpresentingViewController, then dismiss.Changelog
[iOS] [Fixed] - Using presentingViewController to dismiss Modal
Test Plan
Sample code please see #23463, it should works.