fix(animations): enable shadowElements to leave when their parent does #46459

dario-piotrowicz · 2022-06-22T20:09:02Z

when a component uses the shadowDom view encapsulation its children are
not rendered as normal HTML children of the element but they are
insterted in the element's shadowRoot, this causes the leave of the
element not to be normally propagated to the shadow child elements, fix
such issue

resolves #46450

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

If I have a component with ShadowDom view encapsulation and that component leaves the dom, its child elements won't receive a leave event (thus will not animate)

Issue Number: #46450

What is the new behavior?

The child elements of the component with ShadowDom view encapsulation receive the leave event as usual and can therefore animate their leave animations

Does this PR introduce a breaking change?

Yes
No

Other information

interestingly this does not make the minimal reproduction I provided in the issue work 100% correctly, it makes it better by propagating the leave event but the parent gets removed without waiting for the child animation to finish, I will look into that next 🙂
I've found a very similar issue, the query function does not seem to take the shadowDom into account, so if you query for some elements inside a shadowRoot (from outside the shadowRoot) the query call will fail, another thing that I'll look into 🙂 (let me know if it's worth opening an issue explaining it better)
I get the feeling there may be more issues related to not drilling inside shadowDoms 🤔

when a component uses the shadowDom view encapsulation its children are not rendered as normal HTML children of the element but they are insterted in the element's shadowRoot, this causes the leave of the element not to be normally propagated to the shadow child elements, fix such issue resolves angular#46450

dario-piotrowicz · 2022-06-22T20:14:25Z

packages/core/test/animation/animation_query_integration_spec.ts

@@ -2832,6 +2832,64 @@ describe('animation query tests', function() {
         ]);
       }));

+    it(`should emulate a leave animation on a child elements when a parent component using shadowDom leaves the DOM`,


seems like a too straightforward test but it does indeed not pass without the changes 🙂

jessicajaniuk

LGTM 🍪

As usual, thanks @dario-piotrowicz. This seems like a reasonable fix to me.

reviewed-for: fw-core, fw-animations

crisbeto · 2022-06-22T20:36:08Z

packages/animations/browser/src/render/transition_animation_engine.ts

@@ -314,11 +314,13 @@ export class AnimationTransitionNamespace {

  private _signalRemovalForInnerTriggers(rootElement: any, context: any) {
    const elements = this._engine.driver.query(rootElement, NG_TRIGGER_SELECTOR, true);
-
+    const shadowElements = rootElement.shadowRoot ?


A bit of an edge case, but this won't handle nested shadow roots.

ah yeah, you're totally right, as I mentioned in the PR's description I already saw that querying does not work with shadowDom, I'll look into improving the query function to also take shadowDom into account, so I think that should fix the case you're mentioning

So if you prefer I extend this PR with the query improvement or we can try to merge it now and look into the querying issue as a separate task, please let me know whatever works best for you 🙂

(I think I'd go with extending the query function as a separate PR, but I'll need to make extra sure that this piece of code is cleaned up)

I'm fine with merging it as it is, but I wanted to flag it as a potential issue.

awesome thanks 🙂 , I'll make sure to address it! 👍

dylhunn · 2022-06-23T20:49:15Z

This PR was merged into the repository by commit b417370.

#46459) when a component uses the shadowDom view encapsulation its children are not rendered as normal HTML children of the element but they are insterted in the element's shadowRoot, this causes the leave of the element not to be normally propagated to the shadow child elements, fix such issue resolves #46450 PR Close #46459

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fanimations/14.0.3/14.0.4) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcommon/14.0.3/14.0.4) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/14.0.3/14.0.4) | | [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/14.0.3/14.0.4) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcore/14.0.3/14.0.4) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fforms/14.0.3/14.0.4) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/14.0.3/14.0.4) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`14.0.3` -> `14.0.4`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/14.0.3/14.0.4) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v14.0.4`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1404-2022-06-29) [Compare Source](angular/angular@14.0.3...14.0.4) ##### animations | Commit | Type | Description | | -- | -- | -- | | [51be9bbe29](angular/angular@51be9bb) | fix | cleanup DOM elements when the root view is removed ([#45143](angular/angular#45143)) | | [999aca86c8](angular/angular@999aca8) | fix | enable shadowElements to leave when their parent does ([#46459](angular/angular#46459)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [42aed6b13e](angular/angular@42aed6b) | fix | handle CSS custom properties in NgStyle ([#46451](angular/angular#46451)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [1e7f22f00a](angular/angular@1e7f22f) | fix | trigger `ApplicationRef.destroy` when Platform is destroyed ([#46497](angular/angular#46497)) | | [8bde2dbc71](angular/angular@8bde2db) | fix | Update ngfor error code to be negative ([#46555](angular/angular#46555)) | | [57e8fc00eb](angular/angular@57e8fc0) | fix | Updates error to use RuntimeError code ([#46526](angular/angular#46526)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [74a26d870e](angular/angular@74a26d8) | fix | Convert existing reactive errors to use RuntimeErrorCode. ([#46560](angular/angular#46560)) | | [747872212d](angular/angular@7478722) | fix | Update a Forms validator error to use RuntimeError ([#46537](angular/angular#46537)) | ##### router | Commit | Type | Description | | -- | -- | -- | | [d6fac9e914](angular/angular@d6fac9e) | fix | Ensure that new `RouterOutlet` instances work after old ones are destroyed ([#46554](angular/angular#46554)) | #### Special Thanks Alan Agius, Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Bezael, Chad Ramos, Chellappan, Cédric Exbrayat, Dylan Hunn, George Kalpakas, Jeremy Meiss, Jessica Janiuk, Joey Perrott, KMathy, Kristiyan Kostadinov, Paul Gschwendtner, Pawel Kozlowski, Ramesh Thiruchelvam, Vaibhav Kumar, arturovt, dario-piotrowicz and renovate\[bot]  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1437 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

…rent does (angular#46459)" This reverts commit b417370. The change applied is no longer appropriate since the use of animations and shadow dom components is discouraged (as of angular#46738)

…rent does (#46459)" (#46739) This reverts commit b417370. The change applied is no longer appropriate since the use of animations and shadow dom components is discouraged (as of #46738) PR Close #46739

angular-automatic-lock-bot · 2022-07-24T16:06:31Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

pullapprove bot requested review from crisbeto and jessicajaniuk June 22, 2022 20:09

dario-piotrowicz commented Jun 22, 2022

View reviewed changes

jessicajaniuk approved these changes Jun 22, 2022

View reviewed changes

jessicajaniuk added the area: animations label Jun 22, 2022

ngbot bot added this to the Backlog milestone Jun 22, 2022

jessicajaniuk added target: patch This PR is targeted for the next patch release action: presubmit The PR is in need of a google3 presubmit labels Jun 22, 2022

crisbeto reviewed Jun 22, 2022

View reviewed changes

AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Jun 23, 2022

dylhunn closed this in b417370 Jun 23, 2022

dario-piotrowicz deleted the animations-shadow-bug branch June 23, 2022 21:13

dario-piotrowicz mentioned this pull request Jun 24, 2022

fix(animations): make sure query works with shadow dom elements #46488

Closed

14 tasks

This was referenced Jul 1, 2022

build(deps-dev): bump @angular/language-service from 12.2.4 to 14.0.4 numbersprotocol/capture-lite#1761

Closed

build(deps-dev): bump @angular/compiler from 12.2.4 to 14.0.4 numbersprotocol/capture-lite#1755

Closed

dario-piotrowicz mentioned this pull request Jul 7, 2022

Revert "fix(animations): enable shadowElements to leave when their parent does (#46459)" #46739

Closed

14 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(animations): enable shadowElements to leave when their parent does #46459

fix(animations): enable shadowElements to leave when their parent does #46459

dario-piotrowicz commented Jun 22, 2022

dario-piotrowicz Jun 22, 2022

jessicajaniuk left a comment

crisbeto Jun 22, 2022

dario-piotrowicz Jun 22, 2022

crisbeto Jun 22, 2022

dario-piotrowicz Jun 22, 2022

dylhunn commented Jun 23, 2022

angular-automatic-lock-bot bot commented Jul 24, 2022

fix(animations): enable shadowElements to leave when their parent does #46459

fix(animations): enable shadowElements to leave when their parent does #46459

Conversation

dario-piotrowicz commented Jun 22, 2022

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

dario-piotrowicz Jun 22, 2022

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

crisbeto Jun 22, 2022

Choose a reason for hiding this comment

dario-piotrowicz Jun 22, 2022

Choose a reason for hiding this comment

crisbeto Jun 22, 2022

Choose a reason for hiding this comment

dario-piotrowicz Jun 22, 2022

Choose a reason for hiding this comment

dylhunn commented Jun 23, 2022

angular-automatic-lock-bot bot commented Jul 24, 2022