From 33eca1f149076637eb738a7932943f08ced39113 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:51:18 -0500 Subject: [PATCH] refactor(core): distinguish animations that share a name animate.leave waits for the longest-running animation before removing an element. When multiple animations use the same keyframe name, their animationend events have the same animationName, so a shorter animation can be mistaken for the longest one and remove the element too early. Track the Animation returned by getAnimations() and compare it with event.animation when available. Keep the existing name/property checks as a fallback for older browsers and computed-style detection. --- packages/core/src/animation/interfaces.ts | 1 + .../core/src/animation/longest_animation.ts | 2 +- packages/core/src/animation/utils.ts | 34 +++++++--- .../core/test/acceptance/animation_spec.ts | 64 ++++++++++++++++++- .../test/animation/longest_animation_spec.ts | 24 ++++++- 5 files changed, 111 insertions(+), 14 deletions(-) diff --git a/packages/core/src/animation/interfaces.ts b/packages/core/src/animation/interfaces.ts index 77877e9dd107..b53e470e3efe 100644 --- a/packages/core/src/animation/interfaces.ts +++ b/packages/core/src/animation/interfaces.ts @@ -59,6 +59,7 @@ export type RunEnterAnimationFn = VoidFunction; export type RunLeaveAnimationFn = () => {promise: Promise; resolve: VoidFunction}; export interface LongestAnimation { + animation?: Animation; animationName: string | undefined; propertyName: string | undefined; duration: number; diff --git a/packages/core/src/animation/longest_animation.ts b/packages/core/src/animation/longest_animation.ts index d2f4c727a13e..b2812a298ded 100644 --- a/packages/core/src/animation/longest_animation.ts +++ b/packages/core/src/animation/longest_animation.ts @@ -148,7 +148,7 @@ function determineLongestAnimationFromElementAnimations( } if (duration >= longest.duration) { - longest = {animationName, propertyName, duration}; + longest = {animation, animationName, propertyName, duration}; } } if (isShorterThanExistingAnimation(animationsMap.get(el), longest)) return; diff --git a/packages/core/src/animation/utils.ts b/packages/core/src/animation/utils.ts index 050e3a7791ef..0653b7e14348 100644 --- a/packages/core/src/animation/utils.ts +++ b/packages/core/src/animation/utils.ts @@ -332,14 +332,32 @@ export function isLongestAnimation( // If we don't have any record of a longest animation, then we shouldn't // block the animationend/transitionend event from doing its work. if (longestAnimation === undefined) return true; - return ( - nativeElement === getEventTarget(event) && - ((longestAnimation.animationName !== undefined && - (event as AnimationEvent).animationName === longestAnimation.animationName) || - (longestAnimation.propertyName !== undefined && - (longestAnimation.propertyName === 'all' || - (event as TransitionEvent).propertyName === longestAnimation.propertyName))) - ); + + if (nativeElement !== getEventTarget(event)) return false; + + // Distinct CSS animations can share a name. Chrome 151 stable exposes their exact instance: + // https://developer.chrome.com/release-notes/151#animation_accessor_on_animation_and_transition_events + const eventAnimation = ( + event as (AnimationEvent | TransitionEvent) & {readonly animation?: Animation | null} + ).animation; + + if (eventAnimation && longestAnimation.animation) { + return eventAnimation === longestAnimation.animation; + } + + // Fall back to strings for older browsers and animations determined from computed styles. + if (longestAnimation.animationName !== undefined) { + return (event as AnimationEvent).animationName === longestAnimation.animationName; + } + + if (longestAnimation.propertyName !== undefined) { + return ( + longestAnimation.propertyName === 'all' || + (event as TransitionEvent).propertyName === longestAnimation.propertyName + ); + } + + return false; } /** diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts index 7751d32c4290..dd1d7932f733 100644 --- a/packages/core/test/acceptance/animation_spec.ts +++ b/packages/core/test/acceptance/animation_spec.ts @@ -864,6 +864,67 @@ describe('Animation', () => { expect(fixture.debugElement.query(By.css('div'))).toBeNull(); })); + it('should wait for the exact longest animation when animation names are duplicated', fakeAsync(() => { + const multiple = ` + .duplicate-animation-name { + animation: + duplicate-name 10s linear, + duplicate-name 20s linear; + } + @keyframes duplicate-name { + from { + opacity: 1; + } + to { + opacity: 0; + } + } + `; + @Component({ + changeDetection: ChangeDetectionStrategy.Eager, + selector: 'test-cmp', + styles: multiple, + template: + '@if (show()) {

Element with text

}', + encapsulation: ViewEncapsulation.None, + }) + class TestComponent { + show = signal(true); + } + + TestBed.configureTestingModule({animationsEnabled: true}); + + const fixture = TestBed.createComponent(TestComponent); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + const paragraph = fixture.debugElement.query(By.css('p')); + + expect(paragraph.nativeElement.className).not.toContain('duplicate-animation-name'); + cmp.show.set(false); + fixture.detectChanges(); + tickAnimationFrames(1); + expect(cmp.show()).toBeFalse(); + fixture.detectChanges(); + expect(paragraph.nativeElement.className).toContain('duplicate-animation-name'); + + const [shortAnimation, longAnimation] = paragraph.nativeElement.getAnimations(); + const dispatchAnimationEnd = (animation: Animation) => { + const event = new AnimationEvent('animationend', {animationName: 'duplicate-name'}); + Object.defineProperty(event, 'animation', {value: animation}); + paragraph.nativeElement.dispatchEvent(event); + }; + + dispatchAnimationEnd(shortAnimation); + // Flush leave completion so this fails if the shorter event removes the element. + tick(); + expect(fixture.nativeElement.outerHTML).toContain('duplicate-animation-name'); + + dispatchAnimationEnd(longAnimation); + tick(); + expect(fixture.nativeElement.outerHTML).not.toContain('duplicate-animation-name'); + expect(fixture.debugElement.query(By.css('p'))).toBeNull(); + })); + describe('legacy animations compatibility', () => { beforeAll(() => { TestBed.resetTestEnvironment(); @@ -2678,8 +2739,7 @@ describe('Animation', () => { const panels = () => Array.from(fixture.nativeElement.querySelectorAll('.panel')); const panelByText = (text: string) => panels().find((el) => (el as HTMLElement).textContent?.includes(text)) as - | HTMLElement - | undefined; + HTMLElement | undefined; expect(panels().length).toBe(1); expect(panelByText('Panel A')).toBeTruthy(); diff --git a/packages/core/test/animation/longest_animation_spec.ts b/packages/core/test/animation/longest_animation_spec.ts index 76fae451e144..966194f60416 100644 --- a/packages/core/test/animation/longest_animation_spec.ts +++ b/packages/core/test/animation/longest_animation_spec.ts @@ -58,7 +58,12 @@ describe('determineLongestAnimation', () => { determineLongestAnimation(el, animationsMap, true); const longest = animationsMap.get(el); - expect(longest).toEqual({animationName: 'anim-2', propertyName: undefined, duration: 1000}); + expect(longest).toEqual({ + animation: el.getAnimations()[1], + animationName: 'anim-2', + propertyName: undefined, + duration: 1000, + }); }); it('should correctly identify CSSTransitions vs CSSAnimations', () => { @@ -77,7 +82,12 @@ describe('determineLongestAnimation', () => { determineLongestAnimation(el, animationsMap, true); const longest = animationsMap.get(el); - expect(longest).toEqual({animationName: undefined, propertyName: 'opacity', duration: 800}); + expect(longest).toEqual({ + animation: el.getAnimations()[0], + animationName: undefined, + propertyName: 'opacity', + duration: 800, + }); }); it('should handle "auto" or undefined duration gracefully', () => { @@ -97,6 +107,7 @@ describe('determineLongestAnimation', () => { determineLongestAnimation(el, animationsMap, true); const longest = animationsMap.get(el); expect(longest).toEqual({ + animation: el.getAnimations()[0], animationName: 'bad-duration', propertyName: undefined, duration: 200, @@ -127,6 +138,7 @@ describe('determineLongestAnimation', () => { determineLongestAnimation(el, animationsMap, true); const longest = animationsMap.get(el); expect(longest).toEqual({ + animation: el.getAnimations()[1], animationName: 'finite-anim', propertyName: undefined, duration: 500, @@ -197,7 +209,12 @@ describe('determineLongestAnimation', () => { determineLongestAnimation(el, animationsMap, true); const longest = animationsMap.get(el); - expect(longest).toEqual({animationName: 'mock-anim', propertyName: undefined, duration: 500}); + expect(longest).toEqual({ + animation: el.getAnimations()[0], + animationName: 'mock-anim', + propertyName: undefined, + duration: 500, + }); }); it('should handle negative playback rates by taking the absolute value', () => { @@ -218,6 +235,7 @@ describe('determineLongestAnimation', () => { const longest = animationsMap.get(el); expect(longest).toEqual({ + animation: el.getAnimations()[0], animationName: 'mock-anim', propertyName: undefined, duration: 1200,