angular / angular

Deliver web apps with confidence πŸš€
https://angular.dev
MIT License
96.19k stars 25.46k forks source link

Enter and leave animations don't behave as expected with parent-child relationships #35825

Open BeejeeDS opened 4 years ago

BeejeeDS commented 4 years ago

🐞 Bug report

Affected Package

@angular/animation or @angular/core

Is this a regression?

I've made a Stackblitz with Angular 9, 8 and 7. All versions have this problem.

Description

There is a problem when you nest a component with an :enter and :leave animation inside a parent which is conditionally visible using *ngIf. When the parent is visible for the first time there is no issue. Once you trigger the parent to leave and enter again, the child will not be visible. It looks like the leave animation of the child gets triggered when the parent leaves but the enter animation isn't fired once the parent enters again.

πŸ”¬ Minimal Reproduction

I've made a simple example in Stackblitz. You can even make it less complex by removing the animations on the collapsible panel. Stackblitz with Angular 17 Stackblitz with Angular 9 Stackblitz with Angular 8 Stackblitz with Angular 7

🌍 Your Environment

Angular 9 but it's also reproducible in 8 and 7. Stackblitz contains my package versions.

VinceEeckhout commented 4 years ago

We also have this problem in our project. Is there any solution for this? When will this be addressed?

Thanks in advance!

BeejeeDS commented 4 years ago

Guys this bug makes animations unusable. My client keeps on logging bugs caused by this.

Almost 2 months have passed and nothing has happened to this issue. As a workaround I'm updating all my packages and remove all the animations in it... We just can’t have this behavior in production.

arnoabraham commented 4 years ago

Having the same issue for my project

BeejeeDS commented 3 years ago

@jelbourn For something with freq3: high, this bug really has taken its time. This was posted almost a year ago, I would really appreciate some kind of progress.

My clients just keep on bothering me with new bugs having this bug as the root cause.

jelbourn commented 3 years ago

Note: this issue covers general situations of parent-child enter/leave animations not behaving as expected (e.g. #40799, #35825)

ilyakonrad commented 3 years ago

When will this be solved? Having parent-child enter/leave animations problems seems to be critical for UX.

KyleStank commented 3 years ago

@BeejeeDS I managed to get your Angular 9 Stackblitz working with a really stupid solution. Shouldn't be required at all, but if you switch from using :enter and :leave to using true or false, it will work properly.

message.component.html

<div class="message" [@closeAnimation]="isClosed" *ngIf="!isClosed">
  <button class="message-close-button" type="button" (click)="close()" tabindex="-1">
       X
  </button>

  <ng-content></ng-content>
</div>

The important part is this change: [@closeAnimation]="isClosed"

Now you need to modify your actual animation trigger.

message.component.ts

...
trigger("closeAnimation", [
  transition("false => true", [
    useAnimation(MyAnimations.expand, { params: { opacity: 0 } })
  ]),
  transition("true => false", [
    useAnimation(MyAnimations.collapse, { params: { opacity: 0 } })
  ])
])
...

Specifically, the part that is breaking your animation is the :leave state. I tried doing :leave, true => false but it broke. This has something to do with *ngIf or <ng-content>, but I couldn't tell you what. That's why these animation issues need fixed.

EDIT: This solution basically means to stay away from ngIf in animations when you can. It is so incredibly frustrating, but I haven't found another solution until the Angular team finally fixes the many issues that exist with animations and ngIf.


Regarding Angular animations in general, there are many issues with animations I have encountered. If you use an Angular Material table with the multiTemplateDataRows and matSort directive, and make another inner table that has matSort, whenever you sort the inner table (assuming there's an animation somewhere combined with an ngIf), something will 100% be duplicated in the DOM. On top of that, the entire animation will actually break (including callbacks not working) until the ngIf's value is reset to true or false. But the DOM still has duplicated content that is VISIBLE.

Anyways, didn't mean for a mini rant. Just another problem with Angular's animation system. I might open a GitHub issue for what I mentioned above.

Let me know if this helps. I've wasted so many hours at work due to stupid Angular animation issues. I understand your pain.

JoostK commented 3 years ago

This looks like a similar scenario as #24041, but in this case it seems like there's a Chrome bug involved (unlike #24041). The resulting DOM structure after reopening the panel is identical to the initial DOM tree and it looks like the styles have been cleared (the style attributes are all empty). More importantly, the Stackblitz reproduction does not inhibit the problem in Safari!

BeejeeDS commented 3 years ago

Any update on this issue?

dario-piotrowicz commented 2 years ago

Hi @BeejeeDS, I've looked into this an I am pretty sure I understand why it happens (may be tricky to fix....)

Please have a quick look at this fork of your ng9 stackblitz, in which I simply added animation callbacks and console logs

please open the console and refresh the application, you will see that the :enter animation is logged right away, this may be surprising but it is related to content projection, basically the content you are projecting in the ng-content is getting initialized right away thus the enter animation gets immediately triggered

after that if you close the panel, you will see that the :leave animation is logged too, so far so good right? :slightly_smiling_face:

the "surprise" comes if you know open again the panel, you will not see any logging in the console, that is why the content is not being shown, it is because it got the leave styles and is not being re-entered anymore

the issue here is regarding content projection and the asynchronous way :enter/:leave are handled, basically whenever you insert a new element angular will trigger its enter animation right away (which is what happens here), but the parent's entering is not what triggers the children entering if you know what I mean

leaving is different though, and the parent actually queries it's children to see if any of those with an animation trigger are present, if so it triggers their leave animation ( this is the relevant code: https://github.com/angular/angular/blob/155742e305cad4b367267c03250d465ce9339587/packages/animations/browser/src/render/transition_animation_engine.ts#L318-L319)

so to summarise, when you project your content initially it creates a node and animates it's enter animation, then when you close the panel the panel itself will look for children and animate their leave animation, but the projected content is actually not being removed, thus it's enter animation will not be triggered when you open the panel back up (content projection can be a bit tricky, what I am about it is based on this nice ngconf talk: https://www.youtube.com/watch?v=PTwKhxLZ3jI)

so, to fix your current issues I see two alternatives, either animate the entering of child elements, but that's tricky and has been discussed to be probably a bad idea (see: https://github.com/angular/angular/pull/44243), or we could try to make sure that leave animations, either don't query for children's animations or if they do make them ignore children which are being projected (I am not too sure if that is actually feasible, as I don't know that much about content projection...) :thinking:


Similarly to what @KyleStank suggested I think there may be various work arounds, like for example making the ngIf of the projected component follow the panel being open or not: https://stackblitz.com/edit/angular-9-animations-bug-2dvysx?file=src%2Fapp%2Fcomponents%2Fmessage.component.ts (so that it kicks off it's own enter/leave animations)

BeejeeDS commented 2 years ago

@dario-piotrowicz & @KyleStank I've given it another try to fix the enter and leave animations again to my library.

The suggestion of Kyle prevents the issue but creates another one, the 'leave animation' doesn't get triggered because the *ngIf removes the element before the animation could take place.

Dario's example does work but isn't really possible to do in a library full of components that have animations and can be nested inside one another.

I just can't wrap my mind around the fact that 2 years have passed now and no actions have been taken to fix this 'high priority' issue.

weilinzung commented 1 year ago

Not too sure if related to my usage, I got unknown HTMLButtonElement.animate error for my Angular web elements.

The error is from here return element['animate'](this._convertKeyframesToObject(keyframes), options).

I added a console log before the above line to see if animate is existing, but it is true.

console.log('animate in element', 'animate' in element ); 

Here is my usage:

    trigger('fadeInout', [
      state('true', style({ opacity: 1, height: '100%' })),
      state('void', style({ opacity: 0, height: '0' })),
      transition(':enter', animate('{{timingEnter}}ms {{delay}}ms ease'), {
        params: { timingEnter: 350, delay: 0 }
      }),
      transition(':leave', animate('{{timingLeave}}ms {{delay}}ms ease'), {
        params: { timingLeave: 100, delay: 0 }
      })
    ])
    <button
     *ngIf="!hideTextUsButton || showConvoDialog"
      [@fadeInout]="{
        value: !showConvoDialog,
        params: {
          timingEnter: hideTextUsButton ? 0 : 350,
          timingLeave: hideTextUsButton ? 0 : 100,
          delay: hideTextUsButton ? 0 : 500
        }
      }"
      type="button"
      (click)="clickToAaction()"
    >Button</button>
Tlepel commented 1 year ago

We ran into this as well, and it would be nice if there was a proper solution to this. The documentation doesn't say anything about the weird behavior of the :leave animation that @dario-piotrowicz describes. Actually, reading the documentation would lead me to believe it shouldn't trigger the children's :leave animation:

NOTE: Entering/leaving behaviors can sometime be confusing. As a rule of thumb consider that any element being added to the DOM by Angular passes via the :enter transition. Only elements being directly removed from the DOM by Angular pass via the :leave transition. For example, an element's view is removed from the DOM because its parent is being removed from the DOM.

Or am I reading this wrong, and does that last sentence mean 'an elemen's :leave animation is triggered because its parent is being removed from the DOM'?

I suppose it would make sense for Angular to include a check, because it might happen more often

BeejeeDS commented 11 months ago

I can confirm that this problem is still present in Angular 17 (10 versions later and still no fix). But I've managed to find a workaround. For each animation I've specified a query for the animated children and applied the animation which results in the default visible state. In my example this is the expanded state.

 animations: [
    trigger('collapseAnimation', [
      transition(':enter', [
        useAnimation(MyAnimations.expand),
        query(
          '@*',
          useAnimation(MyAnimations.expand, { params: { duration: '0s' } })
        ),
      ]),
      transition(':leave', [
        useAnimation(MyAnimations.collapse),
        query(
          '@*',
          useAnimation(MyAnimations.expand, { params: { duration: '0s' } })
        ),
      ]),
    ]),
 ]
Tolia1 commented 10 months ago

i had the same problem. Solution: works fine in my case if i wrap child element into div and apply animation to the div

Dejavu333 commented 10 months ago

animations don't work properly on components only if I wrap the component into a div and put the animation onto the div.