fix: emit afterClosed only after the native modal dismissal completes - #179
Merged
Conversation
afterClosed was emitted by a hardcoded 100ms fallback timer while the iOS dismissal animation (~400ms) was still running, so opening another dialog from afterClosed failed with "the modal view could not be presented". The 'closed' state is now driven by core's showModal closeCallback, which fires once the native dismissal has completed, plus a one-shot 'unloaded' listener scoped inside it so background-driven unloads can't masquerade as a close. The timer remains only as a 5s safety net. Both portal types now share the same show/dismiss path, which also makes natively-dismissed template dialogs emit afterClosed, and afterOpened is wired to shownModally (it never fired before).
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
What is the current behavior?
NativeDialogRef.afterClosed()is unreliable: it is usually emitted by a hardcoded 100ms fallback timer instead of the actual dismissal. On iOS the modal dismiss animation takes ~400–500ms, soafterClosedfires while the previous view controller is still being dismissed — opening another dialog fromafterClosedthen fails withFailed to open dialog: the modal view could not be presented, forcing apps to work around it with timeouts.Additionally:
afterOpened()never fires (nothing ever emitted the'opened'state).afterClosedat all.What is the new behavior?
afterClosedis now driven by core'sshowModalcloseCallback, which fires only once the native dismissal has completed (on iOS, from thedismissViewControllerAnimatedcompletion handler), followed by a one-shotunloadedlistener attached only at that point — so an unrelated unload (e.g. the app going to the background) can never masquerade as a close. The old timer remains only as a 5s safety net for dismissals that never report completion.afterClosednow works without workarounds.afterOpened()fires once the modal is fully presented (wired toshownModally).afterClosed, and_closeModalNavigationruns exactly once per close.native-dialogspecs (passing on iOS and Android) assert the behavior:beforeClosed→afterClosedordering and result value, no ancestor still presenting atafterClosedtime, an immediate no-timeout reopen, and native dismissal of both dialog types.Note:
afterClosednow fires at the real dismissal completion (~400–500ms on iOS) instead of ~100ms — intentionally later, since the earlier timing was a lie.