From ce7f400011e1e2e1b9316f18ce87b87b79d878b4 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Fri, 4 Mar 2016 16:35:54 +0100 Subject: [PATCH] fix(ngAnimate.$animate): remove animation callbacks when the element is removed The test for this didn't actually test the listener removal. The addClass animation after the element removal didn't start because the enter animation was still in progress. --- src/ngAnimate/animateQueue.js | 9 +++++++- test/ngAnimate/animateSpec.js | 42 +++++++++++++++++++++++++++++------ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index 146e538a1..a81342d8a 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -194,7 +194,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { return matches; } - return { + var $animate = { on: function(event, container, callback) { var node = extractElementNode(container); callbackRegistry[event] = callbackRegistry[event] || []; @@ -202,6 +202,11 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { node: node, callback: callback }); + + // Remove the callback when the element is removed from the DOM + jqLite(container).on('$destroy', function() { + $animate.off(event, container, callback); + }); }, off: function(event, container, callback) { @@ -269,6 +274,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { } }; + return $animate; + function queueAnimation(element, event, initialOptions) { // we always make a copy of the options since // there should never be any side effects on diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index bed63f270..92e4c88e7 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -1912,14 +1912,16 @@ describe("animations", function() { expect(capturedElement).toBe(element); })); - it('should remove the event listener if the element is removed', + it('should remove all event listeners when the element is removed', inject(function($animate, $rootScope, $rootElement) { element = jqLite('
'); var count = 0; + var runner; + $animate.on('enter', element, counter); - $animate.on('addClass', element, counter); + $animate.on('addClass', element[0], counter); function counter(element, phase) { if (phase === 'start') { @@ -1927,18 +1929,45 @@ describe("animations", function() { } } - $animate.enter(element, $rootElement); + runner = $animate.enter(element, $rootElement); $rootScope.$digest(); - $animate.flush(); + expect(capturedAnimation).toBeTruthy(); + $animate.flush(); + runner.end(); // Otherwise the class animation won't run because enter is still in progress expect(count).toBe(1); + + capturedAnimation = null; + + $animate.addClass(element, 'blue'); + $rootScope.$digest(); + expect(capturedAnimation).toBeTruthy(); + + $animate.flush(); + expect(count).toBe(2); + + capturedAnimation = null; + element.remove(); - $animate.addClass(element, 'viljami'); + runner = $animate.enter(element, $rootElement); $rootScope.$digest(); + expect(capturedAnimation).toBeTruthy(); + $animate.flush(); - expect(count).toBe(1); + runner.end(); // Otherwise the class animation won't run because enter is still in progress + + expect(count).toBe(2); + + capturedAnimation = null; + + $animate.addClass(element, 'red'); + $rootScope.$digest(); + expect(capturedAnimation).toBeTruthy(); + + $animate.flush(); + expect(count).toBe(2); })); it('should always detect registered callbacks after one postDigest has fired', @@ -2116,7 +2145,6 @@ describe("animations", function() { var callbackTriggered = false; - $animate.on($event, $document[0], function() { callbackTriggered = true; });