ionic-framework
ionic-framework copied to clipboard
fix(overlays): dismissed overlays will resolve in the next frame
Pull request checklist
Please check if your PR fulfills the following requirements:
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
- Some docs updates need to be made in the
ionic-docs
repo, in a separate PR. See the contributing guide for details.
- Some docs updates need to be made in the
- [x] Build (
npm run build
) was run locally and any changes were pushed - [x] Lint (
npm run lint
) has passed locally and any fixes were made for failures
Pull request type
Please check the type of change your PR introduces:
- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):
What is the current behavior?
With a very specific configuration of presenting and dismissing overlays, occasionally getTop()
will return the overlay that was just dismissed. When used with recursion (where you are trying to dismiss all overlays until there is no more), this creates problems/infinite loops.
Issue URL: #25447
What is the new behavior?
- The
dismiss()
API will resolve in the next frame, after the element has completed the transition and been removed from the DOM. - This improves the consistency of
getTop()
to return the active visible, top overlay.
Does this introduce a breaking change?
- [ ] Yes
- [x] No
Other information
You can verify this behavior in this reproduction app: https://github.com/sean-perkins/gh-25447-overlay-get-top it is a fork of the original reproduction, with safe guards to avoid an infinite loop locking up the browser tab.
The main
branch is on 6.2.0 and will highlight the problem in console (the infinite loop occurs, but bails with an error).
You can install the dev-build and see this behavior does not happen and the loading indicator properly dismisses.
Dev-build: 6.2.2-dev.11659644639.1b018cb5
Writing a test case around this problem was unsuccessful (without copying all the bolierplate from the reproduction app into our test app). I do not believe the value of covering this test condition is worth maintaining all the boilerplate.