Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(core): Fix animate.enter class removal when composing classes (#62981)

Browse source 
In the case when composing animation classes with `animate.enter` on the
element itself and also with host bindings, the removal would only
have context for one of the classes added: the last one added. This
allows for tracking of the classes added by `animate.enter` via a
WeakMap so we know the exact classes added and which to remove.

Also shores up the tests to make sure we are fully testing animate.enter.

PR Close #62981
This commit is contained in:
Jessica Janiuk 2025-08-04 12:36:56 +02:00 committed by Kristiyan Kostadinov
parent d00b3fed58
commit de3a0c5cf3
2 changed files with 123 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
packages/core/src/render3/instructions/animation.ts
View file

@ -36,6 +36,10 @@ const areAnimationSupported =
const noOpAnimationComplete = () => {};
// Tracks the list of classes added to a DOM node from `animate.enter` calls to ensure
// we remove all of the classes in the case of animation composition via host bindings.
const enterClassMap = new WeakMap<HTMLElement, string[]>();
/**
* Instruction to handle the `animate.enter` behavior for class bindings.
*
@ -76,7 +80,6 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
// This also allows us to setup cancellation of animations in progress if the
// gets removed early.
const handleAnimationStart = (event: AnimationEvent | TransitionEvent) => {
setupAnimationCancel(event, activeClasses, renderer);
const eventName = event instanceof AnimationEvent ? 'animationend' : 'transitionend';
ngZone.runOutsideAngular(() => {
cleanupFns.push(renderer.listen(nativeElement, eventName, handleInAnimationEnd));
@ -85,7 +88,7 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
// When the longest animation ends, we can remove all the classes
const handleInAnimationEnd = (event: AnimationEvent | TransitionEvent) => {
animationEnd(event, nativeElement, activeClasses, renderer, cleanupFns);
animationEnd(event, nativeElement, renderer, cleanupFns);
};
// We only need to add these event listeners if there are actual classes to apply
@ -95,6 +98,8 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
cleanupFns.push(renderer.listen(nativeElement, 'transitionstart', handleAnimationStart));
});
trackEnterClasses(nativeElement, activeClasses);
for (const klass of activeClasses) {
renderer.addClass(nativeElement as HTMLElement, klass);
}
@ -103,6 +108,23 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn
return ɵɵanimateEnter; // For chaining
}
/**
* trackEnterClasses is necessary in the case of composition where animate.enter
* is used on the same element in multiple places, like on the element and in a
* host binding. When removing classes, we need the entire list of animation classes
* added to properly remove them when the longest animation fires.
*/
function trackEnterClasses(el: HTMLElement, classes: string[]) {
const classlist = enterClassMap.get(el);
if (classlist) {
for (const klass of classes) {
classlist.push(klass);
}
} else {
enterClassMap.set(el, classes);
}
}
/**
* Instruction to handle the `(animate.enter)` behavior for event bindings, aka when
* a user wants to use a custom animation function rather than a class.
@ -390,10 +412,12 @@ function isLongestAnimation(
function animationEnd(
event: AnimationEvent | TransitionEvent,
nativeElement: HTMLElement,
classList: string[] | null,
renderer: Renderer,
cleanupFns: Function[],
) {
const classList = enterClassMap.get(nativeElement);
if (!classList) return;
setupAnimationCancel(event, classList, renderer);
const longestAnimation = getLongestAnimation(event);
if (isLongestAnimation(event, nativeElement, longestAnimation)) {
// Now that we've found the longest animation, there's no need
@ -401,11 +425,10 @@ function animationEnd(
// other elements further up. We don't want it to inadvertently
// affect any other animations on the page.
event.stopImmediatePropagation();
if (classList !== null) {
for (const klass of classList) {
renderer.removeClass(nativeElement, klass);
}
for (const klass of classList) {
renderer.removeClass(nativeElement, klass);
}
enterClassMap.delete(nativeElement);
for (const fn of cleanupFns) {
fn();
}

93
packages/core/test/acceptance/animation_spec.ts
View file

@ -629,7 +629,13 @@ describe('Animation', () => {
cmp.show.set(true);
fixture.detectChanges();
expect(cmp.show()).toBeTruthy();
const paragraph = fixture.debugElement.query(By.css('p'));
expect(cmp.el.nativeElement.outerHTML).toContain('class="slide-in"');
paragraph.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
paragraph.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'fade-in'}),
);
expect(cmp.el.nativeElement.outerHTML).not.toContain('class="slide-in fade-in"');
});
it('should support string arrays', () => {
@ -674,10 +680,71 @@ describe('Animation', () => {
const fixture = TestBed.createComponent(TestComponent);
const cmp = fixture.componentInstance;
fixture.detectChanges();
expect(cmp.show()).toBeFalsy();
cmp.show.set(true);
fixture.detectChanges();
const paragraph = fixture.debugElement.query(By.css('p'));
expect(cmp.show()).toBeTruthy();
expect(cmp.el.nativeElement.outerHTML).toContain('class="slide-in fade-in"');
paragraph.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
paragraph.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'fade-in'}),
);
expect(cmp.el.nativeElement.outerHTML).not.toContain('class="slide-in fade-in"');
});
it('should support multple classes as a single string separated by a space', () => {
const multiple = `
.slide-in {
animation: slide-in 1ms;
}
.fade-in {
animation: fade-in 2ms;
}
@keyframes slide-in {
from {
transform: translateX(-10px);
}
to {
transform: translateX(0);
}
}
@keyframes fade-in {
from {
opacity: 0;
}
to {
opacity: 1;
}
}
`;
@Component({
selector: 'test-cmp',
styles: multiple,
template:
'<div>@if (show()) {<p animate.enter="slide-in fade-in" #el>I should slide in</p>}</div>',
encapsulation: ViewEncapsulation.None,
})
class TestComponent {
show = signal(false);
@ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
}
TestBed.configureTestingModule({animationsEnabled: true});
const fixture = TestBed.createComponent(TestComponent);
const cmp = fixture.componentInstance;
fixture.detectChanges();
cmp.show.set(true);
fixture.detectChanges();
const paragraph = fixture.debugElement.query(By.css('p'));
expect(cmp.show()).toBeTruthy();
expect(cmp.el.nativeElement.outerHTML).toContain('class="slide-in fade-in"');
fixture.detectChanges();
paragraph.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
paragraph.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'fade-in'}),
);
expect(cmp.el.nativeElement.outerHTML).not.toContain('class="slide-in fade-in"');
});
it('should support multple classes as a single string separated by a space', () => {
@ -762,6 +829,12 @@ describe('Animation', () => {
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.outerHTML).toContain('class="slide-in"');
const paragraph = fixture.debugElement.query(By.css('p'));
paragraph.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
paragraph.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'slide-in'}),
);
expect(fixture.debugElement.nativeElement.outerHTML).toContain('class="slide-in"');
});
it('should compose class list when host binding and regular binding', () => {
@ -781,6 +854,7 @@ describe('Animation', () => {
styles: styles,
imports: [ChildComponent],
template: '<child-cmp [animate.enter]="fadeExp" />',
encapsulation: ViewEncapsulation.None,
})
class TestComponent {
fadeExp = 'fade-in';
@ -793,6 +867,15 @@ describe('Animation', () => {
expect(childCmp.nativeElement.className).toContain('slide-in');
expect(childCmp.nativeElement.className).toContain('fade-in');
childCmp.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
childCmp.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'fade-in'}),
);
childCmp.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'slide-in'}),
);
expect(childCmp.nativeElement.className).not.toContain('slide-in');
expect(childCmp.nativeElement.className).not.toContain('fade-in');
});
it('should compose class list when host binding a string and regular class strings', () => {
@ -810,6 +893,7 @@ describe('Animation', () => {
styles: styles,
imports: [ChildComponent],
template: '<child-cmp animate.enter="fade-in" />',
encapsulation: ViewEncapsulation.None,
})
class TestComponent {}
TestBed.configureTestingModule({animationsEnabled: true});
@ -819,6 +903,15 @@ describe('Animation', () => {
const childCmp = fixture.debugElement.query(By.css('child-cmp'));
expect(childCmp.nativeElement.className).toContain('slide-in fade-in');
childCmp.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
childCmp.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'fade-in'}),
);
childCmp.nativeElement.dispatchEvent(
new AnimationEvent('animationend', {animationName: 'slide-in'}),
);
fixture.detectChanges();
expect(childCmp.nativeElement.className).not.toContain('slide-in fade-in');
});
});
});