Skip to content

Commit 6597ac0

Browse files
thePunderWomancrisbeto
authored andcommitted
fix(core): fix support for space separated strings in leave animations (#62979)
Space separated strings, e.g. `class-1 class-2`, should work with both enter and leave animations. `animate.leave` lost that functionality in a refactor. Tests are now added to catch this. fixes: #62964 PR Close #62979
1 parent cbd300d commit 6597ac0

File tree

3 files changed

+137
-28
lines changed

3 files changed

+137
-28
lines changed

packages/core/src/animation.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ export class ElementRegistry {
9696

9797
/** Used when animate.leave is only applying classes */
9898
trackClasses(details: AnimationDetails, classes: string | string[]): void {
99-
const classList = typeof classes === 'string' ? [classes] : classes;
99+
const classList = getClassListFromValue(classes);
100+
if (!classList) return;
100101
for (let klass of classList) {
101102
details.classes?.add(klass);
102103
}
@@ -175,3 +176,15 @@ export class ElementRegistry {
175176
details.animateFn(remove);
176177
}
177178
}
179+
180+
export function getClassListFromValue(value: string | Function | string[]): string[] | null {
181+
const classes = typeof value === 'function' ? value() : value;
182+
let classList: string[] | null = Array.isArray(classes) ? classes : null;
183+
if (typeof classes === 'string') {
184+
classList = classes
185+
.trim()
186+
.split(/\s+/)
187+
.filter((k) => k);
188+
}
189+
return classList;
190+
}

packages/core/src/render3/instructions/animation.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
AnimationFunction,
1515
AnimationRemoveFunction,
1616
ANIMATIONS_DISABLED,
17+
getClassListFromValue,
1718
LongestAnimation,
1819
} from '../../animation';
1920
import {getLView, getCurrentTNode, getTView, getAnimationElementRemovalRegistry} from '../state';
@@ -27,7 +28,6 @@ import {NgZone} from '../../zone';
2728
import {assertDefined} from '../../util/assert';
2829

2930
const DEFAULT_ANIMATIONS_DISABLED = false;
30-
const WS_REGEXP = /\s+/;
3131
const areAnimationSupported =
3232
(typeof ngServerMode === 'undefined' || !ngServerMode) &&
3333
typeof document !== 'undefined' &&
@@ -352,18 +352,6 @@ function getLongestAnimation(
352352
return currentLongest;
353353
}
354354

355-
function getClassListFromValue(value: string | Function): string[] | null {
356-
const classes = typeof value === 'function' ? value() : value;
357-
let classList: string[] | null = classes instanceof Array ? classes : null;
358-
if (typeof classes === 'string') {
359-
classList = classes
360-
.trim()
361-
.split(WS_REGEXP)
362-
.filter((k) => k);
363-
}
364-
return classList;
365-
}
366-
367355
function setupAnimationCancel(event: Event, classList: string[] | null, renderer: Renderer) {
368356
if (!(event.target instanceof Element)) return;
369357
const nativeElement = event.target;

packages/core/test/acceptance/animation_spec.ts

Lines changed: 122 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ describe('Animation', () => {
2828
describe('animate.leave', () => {
2929
const styles = `
3030
.fade {
31-
animation: fade-out 1s;
31+
animation: fade-out 1ms;
3232
}
3333
@keyframes fade-out {
3434
from {
@@ -70,7 +70,7 @@ describe('Animation', () => {
7070
new AnimationEvent('animationend', {animationName: 'fade-out'}),
7171
);
7272
expect(fixture.nativeElement.outerHTML).not.toContain('class="fade"');
73-
}, 100_000);
73+
});
7474

7575
it('should remove right away when animations are disabled', () => {
7676
@Component({
@@ -96,10 +96,10 @@ describe('Animation', () => {
9696
it('should support string arrays', () => {
9797
const multiple = `
9898
.slide-out {
99-
animation: slide-out 2s;
99+
animation: slide-out 2ms;
100100
}
101101
.fade {
102-
animation: fade-out 1s;
102+
animation: fade-out 1ms;
103103
}
104104
@keyframes slide-out {
105105
from {
@@ -155,6 +155,67 @@ describe('Animation', () => {
155155
expect(fixture.nativeElement.outerHTML).not.toContain('class="slide-out fade"');
156156
});
157157

158+
it('should support multiple classes as a single string with spaces', () => {
159+
const multiple = `
160+
.slide-out {
161+
animation: slide-out 2ms;
162+
}
163+
.fade {
164+
animation: fade-out 1ms;
165+
}
166+
@keyframes slide-out {
167+
from {
168+
transform: translateX(0);
169+
}
170+
to {
171+
transform: translateX(10px);
172+
}
173+
}
174+
@keyframes fade-out {
175+
from {
176+
opacity: 1;
177+
}
178+
to {
179+
opacity: 0;
180+
}
181+
}
182+
`;
183+
@Component({
184+
selector: 'test-cmp',
185+
styles: multiple,
186+
template:
187+
'<div>@if (show()) {<p animate.leave="slide-out fade" #el>I should slide out</p>}</div>',
188+
encapsulation: ViewEncapsulation.None,
189+
})
190+
class TestComponent {
191+
show = signal(true);
192+
@ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
193+
}
194+
195+
TestBed.configureTestingModule({animationsEnabled: true});
196+
197+
const fixture = TestBed.createComponent(TestComponent);
198+
const cmp = fixture.componentInstance;
199+
fixture.detectChanges();
200+
const paragragh = fixture.debugElement.query(By.css('p'));
201+
202+
expect(fixture.nativeElement.outerHTML).not.toContain('class="slide-out fade"');
203+
cmp.show.set(false);
204+
fixture.detectChanges();
205+
expect(cmp.show()).toBeFalsy();
206+
fixture.detectChanges();
207+
paragragh.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
208+
expect(fixture.nativeElement.outerHTML).toContain('class="slide-out fade"');
209+
fixture.detectChanges();
210+
paragragh.nativeElement.dispatchEvent(
211+
new AnimationEvent('animationend', {animationName: 'fade-out'}),
212+
);
213+
paragragh.nativeElement.dispatchEvent(
214+
new AnimationEvent('animationend', {animationName: 'slide-out'}),
215+
);
216+
expect(fixture.nativeElement.outerHTML).not.toContain('class="slide-out fade"');
217+
});
218+
158219
it('should support function syntax', () => {
159220
@Component({
160221
selector: 'test-cmp',
@@ -224,10 +285,10 @@ describe('Animation', () => {
224285
it('should compose class list when host binding and regular binding', () => {
225286
const multiple = `
226287
.slide-out {
227-
animation: slide-out 2s;
288+
animation: slide-out 2ms;
228289
}
229290
.fade {
230-
animation: fade-out 1s;
291+
animation: fade-out 1ms;
231292
}
232293
@keyframes slide-out {
233294
from {
@@ -301,10 +362,10 @@ describe('Animation', () => {
301362
it('should compose class list when host binding on a directive and regular binding', () => {
302363
const multiple = `
303364
.slide-out {
304-
animation: slide-out 2s;
365+
animation: slide-out 2ms;
305366
}
306367
.fade {
307-
animation: fade-out 1s;
368+
animation: fade-out 1ms;
308369
}
309370
@keyframes slide-out {
310371
from {
@@ -376,10 +437,10 @@ describe('Animation', () => {
376437
it('should compose class list when host binding a string and regular class strings', () => {
377438
const multiple = `
378439
.slide-out {
379-
animation: slide-out 2s;
440+
animation: slide-out 2ms;
380441
}
381442
.fade {
382-
animation: fade-out 1s;
443+
animation: fade-out 1ms;
383444
}
384445
@keyframes slide-out {
385446
from {
@@ -448,10 +509,10 @@ describe('Animation', () => {
448509
describe('animate.enter', () => {
449510
const styles = `
450511
.slide-in {
451-
animation: slide-in 1s;
512+
animation: slide-in 1ms;
452513
}
453514
.fade-in {
454-
animation: fade-in 2s;
515+
animation: fade-in 2ms;
455516
}
456517
@keyframes slide-in {
457518
from {
@@ -574,10 +635,10 @@ describe('Animation', () => {
574635
it('should support string arrays', () => {
575636
const multiple = `
576637
.slide-in {
577-
animation: slide-in 1s;
638+
animation: slide-in 1ms;
578639
}
579640
.fade-in {
580-
animation: fade-in 2s;
641+
animation: fade-in 2ms;
581642
}
582643
@keyframes slide-in {
583644
from {
@@ -619,6 +680,53 @@ describe('Animation', () => {
619680
expect(cmp.el.nativeElement.outerHTML).toContain('class="slide-in fade-in"');
620681
});
621682

683+
it('should support multple classes as a single string separated by a space', () => {
684+
const multiple = `
685+
.slide-in {
686+
animation: slide-in 1ms;
687+
}
688+
.fade-in {
689+
animation: fade-in 2ms;
690+
}
691+
@keyframes slide-in {
692+
from {
693+
transform: translateX(-10px);
694+
}
695+
to {
696+
transform: translateX(0);
697+
}
698+
}
699+
@keyframes fade-in {
700+
from {
701+
opacity: 0;
702+
}
703+
to {
704+
opacity: 1;
705+
}
706+
}
707+
`;
708+
@Component({
709+
selector: 'test-cmp',
710+
styles: multiple,
711+
template:
712+
'<div>@if (show()) {<p animate.enter="slide-in fade-in" #el>I should slide in</p>}</div>',
713+
encapsulation: ViewEncapsulation.None,
714+
})
715+
class TestComponent {
716+
show = signal(false);
717+
@ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
718+
}
719+
TestBed.configureTestingModule({animationsEnabled: true});
720+
721+
const fixture = TestBed.createComponent(TestComponent);
722+
const cmp = fixture.componentInstance;
723+
fixture.detectChanges();
724+
cmp.show.set(true);
725+
fixture.detectChanges();
726+
expect(cmp.show()).toBeTruthy();
727+
expect(cmp.el.nativeElement.outerHTML).toContain('class="slide-in fade-in"');
728+
});
729+
622730
it('should remove right away when animations are disabled', fakeAsync(() => {
623731
@Component({
624732
selector: 'test-cmp',

0 commit comments

Comments
 (0)