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

fix($animateCss): don't unregister events when empty #13514

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions src/ngAnimate/animateCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,8 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
options.onDone();
}

// Remove the transitionend / animationend listener(s)
if (events) {
if (events && events.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: events will always be truthy, since it is initialized to [].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a nitpick, good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually possible that events is undefined, because if no node is detected, the animation is closed before a bunch of variables are even declared.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I missed that 😞

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

Expand Down Expand Up @@ -960,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 Down
33 changes: 33 additions & 0 deletions test/ngAnimate/animateCssSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,39 @@ describe("ngAnimate $animateCss", function() {
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();
});
});

});
});

Expand Down