Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($animateCss): remove animation end event listeners on close #13672

Closed
wants to merge 1 commit into from
Closed

fix($animateCss): remove animation end event listeners on close #13672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 35 additions & 27 deletions src/ngAnimate/animateCss.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,8 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
var maxDelayTime;
var maxDuration;
var maxDurationTime;
var startTime;
var events = [];

if (options.duration === 0 || (!$sniffer.animations && !$sniffer.transitions)) {
return closeAndReturnNoopAnimator();
Expand Down Expand Up @@ -747,6 +749,11 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
options.onDone();
}

if (events && events.length) {
// Remove the transitionend / animationend listener(s)
element.off(events.join(' '), onAnimationProgress);
}

// if the preparation function fails then the promise is not setup
if (runner) {
runner.complete(!rejected);
Expand Down Expand Up @@ -782,15 +789,37 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
};
}

function onAnimationProgress(event) {
event.stopPropagation();
var ev = event.originalEvent || event;
var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now();

/* Firefox (or possibly just Gecko) likes to not round values up
* when a ms measurement is used for the animation */
var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));

/* $manualTimeStamp is a mocked timeStamp value which is set
* within browserTrigger(). This is only here so that tests can
* mock animations properly. Real events fallback to event.timeStamp,
* or, if they don't, then a timeStamp is automatically created for them.
* We're checking to see if the timeStamp surpasses the expected delay,
* but we're using elapsedTime instead of the timeStamp on the 2nd
* pre-condition since animationPauseds sometimes close off early */
if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
// we set this flag to ensure that if the transition is paused then, when resumed,
// the animation will automatically close itself since transitions cannot be paused.
animationCompleted = true;
close();
}
}

function start() {
if (animationClosed) return;
if (!node.parentNode) {
close();
return;
}

var startTime, events = [];

// even though we only pause keyframe animations here the pause flag
// will still happen when transitions are used. Only the transition will
// not be paused since that is not possible. If the animation ends when
Expand Down Expand Up @@ -931,7 +960,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
element.data(ANIMATE_TIMER_KEY, animationsData);
}

element.on(events.join(' '), onAnimationProgress);
if (events.length) {
element.on(events.join(' '), onAnimationProgress);
}

if (options.to) {
if (options.cleanupStyles) {
registerRestorableStyles(restoreStyles, node, Object.keys(options.to));
Expand All @@ -953,30 +985,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
element.removeData(ANIMATE_TIMER_KEY);
}
}

function onAnimationProgress(event) {
event.stopPropagation();
var ev = event.originalEvent || event;
var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now();

/* Firefox (or possibly just Gecko) likes to not round values up
* when a ms measurement is used for the animation */
var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));

/* $manualTimeStamp is a mocked timeStamp value which is set
* within browserTrigger(). This is only here so that tests can
* mock animations properly. Real events fallback to event.timeStamp,
* or, if they don't, then a timeStamp is automatically created for them.
* We're checking to see if the timeStamp surpasses the expected delay,
* but we're using elapsedTime instead of the timeStamp on the 2nd
* pre-condition since animations sometimes close off early */
if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
// we set this flag to ensure that if the transition is paused then, when resumed,
// the animation will automatically close itself since transitions cannot be paused.
animationCompleted = true;
close();
}
}
}
};
}];
Expand Down
6 changes: 5 additions & 1 deletion test/ngAnimate/.jshintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
"applyAnimationStyles": false,
"applyAnimationFromStyles": false,
"applyAnimationToStyles": false,
"applyAnimationClassesFactory": false
"applyAnimationClassesFactory": false,
"TRANSITIONEND_EVENT": false,
"TRANSITION_PROP": false,
"ANIMATION_PROP": false,
"ANIMATIONEND_EVENT": false
}
}
152 changes: 142 additions & 10 deletions test/ngAnimate/animateCssSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ describe("ngAnimate $animateCss", function() {
: expect(className).not.toMatch(regex);
}

function keyframeProgress(element, duration, delay) {
browserTrigger(element, 'animationend',
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
}

function transitionProgress(element, duration, delay) {
browserTrigger(element, 'transitionend',
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
}

var fakeStyle = {
color: 'blue'
};
Expand Down Expand Up @@ -355,16 +365,6 @@ describe("ngAnimate $animateCss", function() {
assert.toHaveClass('ng-enter-active');
}

function keyframeProgress(element, duration, delay) {
browserTrigger(element, 'animationend',
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
}

function transitionProgress(element, duration, delay) {
browserTrigger(element, 'transitionend',
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
}

beforeEach(inject(function($rootElement, $document) {
element = jqLite('<div></div>');
$rootElement.append(element);
Expand Down Expand Up @@ -1404,6 +1404,138 @@ describe("ngAnimate $animateCss", function() {
expect(count.stagger).toBe(2);
}));
});

describe('transitionend/animationend event listeners', function() {
var element, elementOnSpy, elementOffSpy, progress;

function setStyles(event) {
switch (event) {
case TRANSITIONEND_EVENT:
ss.addRule('.ng-enter', 'transition: 10s linear all;');
progress = transitionProgress;
break;
case ANIMATIONEND_EVENT:
ss.addRule('.ng-enter', '-webkit-animation: animation 10s;' +
'animation: animation 10s;');
progress = keyframeProgress;
break;
}
}

beforeEach(inject(function($rootElement, $document) {
element = jqLite('<div></div>');
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);

elementOnSpy = spyOn(element, 'on').andCallThrough();
elementOffSpy = spyOn(element, 'off').andCallThrough();
}));

they('should remove the $prop event listeners on cancel',
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
inject(function($animateCss) {

setStyles(event);

var animator = $animateCss(element, {
event: 'enter',
structural: true
});

var runner = animator.start();
triggerAnimationStartFrame();

expect(elementOnSpy).toHaveBeenCalledOnce();
expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);

runner.cancel();

expect(elementOffSpy).toHaveBeenCalledOnce();
expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
});
});

they("should remove the $prop event listener when the animation is closed",
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
inject(function($animateCss) {

setStyles(event);

var animator = $animateCss(element, {
event: 'enter',
structural: true
});

var runner = animator.start();
triggerAnimationStartFrame();

expect(elementOnSpy).toHaveBeenCalledOnce();
expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);

progress(element, 10);

expect(elementOffSpy).toHaveBeenCalledOnce();
expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
});
});

they("should remove the $prop event listener when the closing timeout occurs",
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
inject(function($animateCss, $timeout) {

setStyles(event);

var animator = $animateCss(element, {
event: 'enter',
structural: true
});

animator.start();
triggerAnimationStartFrame();

expect(elementOnSpy).toHaveBeenCalledOnce();
expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);

$timeout.flush(15000);

expect(elementOffSpy).toHaveBeenCalledOnce();
expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
});
});

they("should not add or remove $prop event listeners when no animation styles are detected",
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
inject(function($animateCss, $timeout) {

progress = event === TRANSITIONEND_EVENT ? transitionProgress : keyframeProgress;

// Make sure other event listeners are not affected
var otherEndSpy = jasmine.createSpy('otherEndSpy');
element.on(event, otherEndSpy);

expect(elementOnSpy).toHaveBeenCalledOnce();
elementOnSpy.reset();

var animator = $animateCss(element, {
event: 'enter',
structural: true
});

expect(animator.$$willAnimate).toBeFalsy();

// This will close the animation because no styles have been detected
var runner = animator.start();
triggerAnimationStartFrame();

expect(elementOnSpy).not.toHaveBeenCalled();
expect(elementOffSpy).not.toHaveBeenCalled();

progress(element, 10);
expect(otherEndSpy).toHaveBeenCalledOnce();
});
});

});
});

it('should avoid applying the same cache to an element a follow-up animation is run on the same element',
Expand Down