Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
thetaPC
left a comment
thetaPC
left a comment
There was a problem hiding this comment.
LGTM, one question though
There was a problem hiding this comment.
The ticket mentions that it's hard to create reliable test. Would it be possible to create a controller-based modal, open, then verify that we can still interact with the Angular test app? If it can interact, then it passes?
There was a problem hiding this comment.
Well, that's kind of the thing. This test already "exists", there's e2e tests already doing this exact thing and they didn't break before. That's because the scenario itself is a bit unique and I'm not sure what actually makes it that way.
However, I think this might be a suitable alternative: da1911
In this commit I've created a test that ensures that controller-created modals do not have a mutation observer created for them, which should generally cover the bases here. Note: I had to use this PR as the "issue" because the actual issue is internal.
There was a problem hiding this comment.
I wonder if we can replicate it under the Angular test app instead of the core e2e tests? Regardless, I like your alternative! I'm happy with it. I only made one small suggestion.
thetaPC
left a comment
thetaPC
left a comment
There was a problem hiding this comment.
LGTM, one minor request though
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
3 participants
Issue number: resolves internal
What is the current behavior?
When using ModalController to present a modal in Angular applications, the browser becomes non-responsive and hangs in some circumstances. This regression was introduced in #30544 with the addition of a MutationObserver that watches document.body with subtree: true to detect when a modal's parent element is removed from the DOM. For controller-based modals, this observer fires on every DOM mutation in the document, causing severe performance issues during Angular's change detection cycles.
What is the new behavior?
The MutationObserver for parent removal detection is now skipped for controller-based modals and when the cached parent is the app root (document.body or ion-app). These parents are never removed from the DOM, so observing them is unnecessary. This prevents the performance issues while still maintaining the parent removal detection behavior for inline modals with meaningful parent elements.
Does this introduce a breaking change?
Other information
Current dev build: