fix(core): deferred blocks not removing content immediately when animations are enabled#51971
fix(core): deferred blocks not removing content immediately when animations are enabled#51971crisbeto wants to merge 1 commit intoangular:mainfrom
Conversation
5499c15 to
8bc3395
Compare
There was a problem hiding this comment.
Note that I was having a hard time capturing this behavior in a unit test, because the testing APIs would trigger change detection which would flush the animations.
There was a problem hiding this comment.
Thanks for the comment, I haven't seen it while leaving a review comment (#51971 (review)). May be we can have a test to make sure that we run event handlers inside NgZone?
There was a problem hiding this comment.
Hmm, I don't know if we can observe the internal event handlers without introducing another DI token. I ended up adding a dev mode assertion that it's in the NgZone which should give us the same result.
There was a problem hiding this comment.
I ended up adding a dev mode assertion that it's in the NgZone which should give us the same result.
That's a great idea!
There was a problem hiding this comment.
It looks like the assertion isn't super reliable since Zone.js doesn't seem to patch addEventListener in the same way in Node and in the browser so we were getting different test results. I reworked it to spy on addEventListener and check that it's called within the zone instead.
AndrewKushnir
left a comment
There was a problem hiding this comment.
@crisbeto thanks for the fix 👍
Just left a comment and I was also wondering if we can add a new test (or modify one of the existing one) to reproduce an original issue with animations?
8bc3395 to
58d6c27
Compare
…ations are enabled Fixes an issue where if animations are enabled, deferred blocks don't remove their placeholder blocks immediately from the DOM. The problem is that we register the event handlers in `afterRender` which runs outside the zone, but the logic that removes the DOM nodes during animations is tied to change detection. These changes resolve the issue by binding the listeners inside the zone. This was the intention from the beginning, I just forgot that `afterRender` runs outside the zone. Fixes angular#51970.
58d6c27 to
57634f6
Compare
| template: 'Primary block content.', | ||
| }) | ||
| class NestedCmp { | ||
| @Input() block!: string; |
There was a problem hiding this comment.
This is unrelated, I just noticed that it was logging a warning.
|
This PR was merged into the repository by commit 4f69d62. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…ations are enabled (angular#51971) Fixes an issue where if animations are enabled, deferred blocks don't remove their placeholder blocks immediately from the DOM. The problem is that we register the event handlers in `afterRender` which runs outside the zone, but the logic that removes the DOM nodes during animations is tied to change detection. These changes resolve the issue by binding the listeners inside the zone. This was the intention from the beginning, I just forgot that `afterRender` runs outside the zone. Fixes angular#51970. PR Close angular#51971
Fixes an issue where if animations are enabled, deferred blocks don't remove their placeholder blocks immediately from the DOM. The problem is that we register the event handlers in
afterRenderwhich runs outside the zone, but the logic that removes the DOM nodes during animations is tied to change detection.These changes resolve the issue by binding the listeners inside the zone. This was the intention from the beginning, I just forgot that
afterRenderruns outside the zone.Fixes #51970.