diff --git a/src/ng/animate.js b/src/ng/animate.js index 96df569176c8..67c852eb0eae 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -326,6 +326,9 @@ var $AnimateProvider = ['$provide', function($provide) { * // remove all the animation event listeners listening for `enter` * $animate.off('enter'); * + * // remove listeners for all animation events from the container element + * $animate.off(container); + * * // remove all the animation event listeners listening for `enter` on the given element and its children * $animate.off('enter', container); * @@ -334,7 +337,9 @@ var $AnimateProvider = ['$provide', function($provide) { * $animate.off('enter', container, callback); * ``` * - * @param {string} event the animation event (e.g. enter, leave, move, addClass, removeClass, etc...) + * @param {string|DOMElement} event|container the animation event (e.g. enter, leave, move, + * addClass, removeClass, etc...), or the container element. If it is the element, all other + * arguments are ignored. * @param {DOMElement=} container the container element the event listener was placed on * @param {Function=} callback the callback function that was registered as the listener */ diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index 07fa81f3f9e6..5577d6f919f2 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -199,6 +199,23 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { return matches; } + function filterFromRegistry(list, matchContainer, matchCallback) { + var containerNode = extractElementNode(matchContainer); + return list.filter(function(entry) { + var isMatch = entry.node === containerNode && + (!matchCallback || entry.callback === matchCallback); + return !isMatch; + }); + } + + function cleanupEventListeners(phase, element) { + if (phase === 'close' && !element[0].parentNode) { + // If the element is not attached to a parentNode, it has been removed by + // the domOperation, and we can safely remove the event callbacks + $animate.off(element); + } + } + var $animate = { on: function(event, container, callback) { var node = extractElementNode(container); @@ -210,26 +227,33 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { // Remove the callback when the element is removed from the DOM jqLite(container).on('$destroy', function() { - $animate.off(event, container, callback); + var animationDetails = activeAnimationsLookup.get(node); + + if (!animationDetails) { + // If there's an animation ongoing, the callback calling code will remove + // the event listeners. If we'd remove here, the callbacks would be removed + // before the animation ends + $animate.off(event, container, callback); + } }); }, off: function(event, container, callback) { + if (arguments.length === 1 && !angular.isString(arguments[0])) { + container = arguments[0]; + for (var eventType in callbackRegistry) { + callbackRegistry[eventType] = filterFromRegistry(callbackRegistry[eventType], container); + } + + return; + } + var entries = callbackRegistry[event]; if (!entries) return; callbackRegistry[event] = arguments.length === 1 ? null : filterFromRegistry(entries, container, callback); - - function filterFromRegistry(list, matchContainer, matchCallback) { - var containerNode = extractElementNode(matchContainer); - return list.filter(function(entry) { - var isMatch = entry.node === containerNode && - (!matchCallback || entry.callback === matchCallback); - return !isMatch; - }); - } }, pin: function(element, parentElement) { @@ -543,7 +567,10 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { forEach(callbacks, function(callback) { callback(element, phase, data); }); + cleanupEventListeners(phase, element); }); + } else { + cleanupEventListeners(phase, element); } }); runner.progress(event, phase, data); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 0ae259a67d96..2d1d89b256d5 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -1896,6 +1896,68 @@ describe("animations", function() { expect(count).toBe(3); })); + it('should remove all event listeners for an element when $animate.off(element) is called', + inject(function($animate, $rootScope, $rootElement, $document, $$rAF) { + + element = jqLite('
'); + var otherElement = jqLite('
'); + $rootElement.append(otherElement); + + var count = 0; + var runner; + $animate.on('enter', element, counter); + $animate.on('leave', element, counter); + $animate.on('addClass', element, counter); + $animate.on('addClass', otherElement, counter); + + function counter(element, phase) { + count++; + } + + runner = $animate.enter(element, $rootElement); + $rootScope.$digest(); + $animate.flush(); + runner.end(); + + runner = $animate.addClass(element, 'blue'); + $rootScope.$digest(); + $animate.flush(); + + runner.end(); + $$rAF.flush(); + + expect(count).toBe(4); + + $animate.off(element); + + runner = $animate.enter(element, $rootElement); + $animate.flush(); + expect(capturedAnimation[1]).toBe('enter'); + runner.end(); + + runner = $animate.addClass(element, 'red'); + $animate.flush(); + expect(capturedAnimation[1]).toBe('addClass'); + runner.end(); + + runner = $animate.leave(element); + $animate.flush(); + expect(capturedAnimation[1]).toBe('leave'); + runner.end(); + + // Try to flush all remaining callbacks + expect(function() { + $$rAF.flush(); + }).toThrowError('No rAF callbacks present'); + + expect(count).toBe(4); + + // Check that other elements' event listeners are not affected + $animate.addClass(otherElement, 'green'); + $animate.flush(); + expect(count).toBe(5); + })); + it('should fire a `start` callback when the animation starts with the matching element', inject(function($animate, $rootScope, $rootElement, $document) { @@ -1937,63 +1999,96 @@ describe("animations", function() { expect(capturedElement).toBe(element); })); - it('should remove all event listeners when the element is removed', - inject(function($animate, $rootScope, $rootElement) { - element = jqLite('
'); + they('should remove all event listeners when the element is removed via $prop', + ['leave()', 'remove()'], function(method) { + inject(function($animate, $rootScope, $rootElement, $$rAF) { - var count = 0; - var runner; + element = jqLite('
'); - $animate.on('enter', element, counter); - $animate.on('addClass', element[0], counter); + var count = 0; + var enterSpy = jasmine.createSpy(); + var addClassSpy = jasmine.createSpy(); + var runner; - function counter(element, phase) { - if (phase === 'start') { - count++; + $animate.on('enter', element, enterSpy); + $animate.on('addClass', element[0], addClassSpy); + + function counter(element, phase) { + if (phase === 'start') { + count++; + } } - } - runner = $animate.enter(element, $rootElement); - $rootScope.$digest(); - expect(capturedAnimation).toBeTruthy(); + runner = $animate.enter(element, $rootElement); + $rootScope.$digest(); + expect(capturedAnimation).toBeTruthy(); - $animate.flush(); - runner.end(); // Otherwise the class animation won't run because enter is still in progress - expect(count).toBe(1); + $animate.flush(); + expect(enterSpy.calls.count()).toBe(1); + expect(enterSpy.calls.mostRecent().args[1]).toBe('start'); - capturedAnimation = null; + runner.end(); // Otherwise the class animation won't run because enter is still in progress + $$rAF.flush(); + expect(enterSpy.calls.count()).toBe(2); + expect(enterSpy.calls.mostRecent().args[1]).toBe('close'); - $animate.addClass(element, 'blue'); - $rootScope.$digest(); - expect(capturedAnimation).toBeTruthy(); + enterSpy.calls.reset(); + capturedAnimation = null; - $animate.flush(); - expect(count).toBe(2); + runner = $animate.addClass(element, 'blue'); + $rootScope.$digest(); + expect(capturedAnimation).toBeTruthy(); - capturedAnimation = null; + $animate.flush(); + expect(addClassSpy.calls.count()).toBe(1); + expect(addClassSpy.calls.mostRecent().args[1]).toBe('start'); - element.remove(); + runner.end(); + $$rAF.flush(); + expect(addClassSpy.calls.count()).toBe(2); + expect(addClassSpy.calls.mostRecent().args[1]).toBe('close'); - runner = $animate.enter(element, $rootElement); - $rootScope.$digest(); + addClassSpy.calls.reset(); + capturedAnimation = null; - expect(capturedAnimation).toBeTruthy(); + if (method === 'leave()') { + runner = $animate.leave(element); + $animate.flush(); + runner.end(); + } else if (method === 'remove()') { + element.remove(); + } - $animate.flush(); - runner.end(); // Otherwise the class animation won't run because enter is still in progress + runner = $animate.enter(element, $rootElement); + $rootScope.$digest(); - expect(count).toBe(2); + $animate.flush(); + expect(enterSpy.calls.count()).toBe(0); - capturedAnimation = null; + runner.end(); // Otherwise the class animation won't run because enter is still in progress + expect(function() { + $$rAF.flush(); + }).toThrowError('No rAF callbacks present'); // Try to flush any callbacks + expect(enterSpy.calls.count()).toBe(0); - $animate.addClass(element, 'red'); - $rootScope.$digest(); - expect(capturedAnimation).toBeTruthy(); + capturedAnimation = null; - $animate.flush(); - expect(count).toBe(2); - })); + $animate.addClass(element, 'red'); + $rootScope.$digest(); + expect(capturedAnimation).toBeTruthy(); + + $animate.flush(); + expect(addClassSpy.calls.count()).toBe(0); + + runner.end(); + expect(function() { + $$rAF.flush(); + }).toThrowError('No rAF callbacks present'); // Try to flush any callbacks + expect(addClassSpy.calls.count()).toBe(0); + expect(enterSpy.calls.count()).toBe(0); + }); + }); it('should always detect registered callbacks after one postDigest has fired', inject(function($animate, $rootScope, $rootElement) { @@ -2058,104 +2153,187 @@ describe("animations", function() { } })); - it('leave: should remove the element even if another animation is called after', - inject(function($animate, $rootScope, $rootElement) { + describe('for leave', function() { - var outerContainer = jqLite('
'); - element = jqLite('
'); - outerContainer.append(element); - $rootElement.append(outerContainer); + it('should remove the element even if another animation is called afterwards', + inject(function($animate, $rootScope, $rootElement) { - var runner = $animate.leave(element, $rootElement); - $animate.removeClass(element,'rclass'); - $rootScope.$digest(); - runner.end(); - $animate.flush(); + var outerContainer = jqLite('
'); + element = jqLite('
'); + outerContainer.append(element); + $rootElement.append(outerContainer); - var isElementRemoved = !outerContainer[0].contains(element[0]); - expect(isElementRemoved).toBe(true); - })); + var runner = $animate.leave(element, $rootElement); + $animate.removeClass(element,'rclass'); + $rootScope.$digest(); + runner.end(); + $animate.flush(); - it('leave : should trigger a callback for an leave animation', - inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { + var isElementRemoved = !outerContainer[0].contains(element[0]); + expect(isElementRemoved).toBe(true); + })); - var callbackTriggered = false; - $animate.on('leave', jqLite($document[0].body), function() { - callbackTriggered = true; - }); + they('should trigger callbacks when the listener is on the $prop element', ['same', 'parent'], + function(elementRelation) { + inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { + var listenerElement, callbackSpy = jasmine.createSpy(); - element = jqLite('
'); - $rootElement.append(element); - $animate.leave(element, $rootElement); - $rootScope.$digest(); + element = jqLite('
'); + listenerElement = elementRelation === 'same' ? element : jqLite($document[0].body); + $animate.on('leave', listenerElement, callbackSpy); + $rootElement.append(element); + var runner = $animate.leave(element, $rootElement); + $rootScope.$digest(); - $$rAF.flush(); + $$rAF.flush(); - expect(callbackTriggered).toBe(true); - })); + expect(callbackSpy.calls.count()).toBe(1); + expect(callbackSpy.calls.mostRecent().args[1]).toBe('start'); + callbackSpy.calls.reset(); - it('leave : should not fire a callback if the element is outside of the given container', - inject(function($animate, $rootScope, $$rAF, $rootElement) { + runner.end(); + $$rAF.flush(); - var callbackTriggered = false; - var innerContainer = jqLite('
'); - $rootElement.append(innerContainer); + expect(callbackSpy.calls.count()).toBe(1); + expect(callbackSpy.calls.mostRecent().args[1]).toBe('close'); + }); + } + ); - $animate.on('leave', innerContainer, - function(element, phase, data) { - callbackTriggered = true; - }); + it('should trigger callbacks for a leave animation', + inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { - element = jqLite('
'); - $rootElement.append(element); - $animate.leave(element, $rootElement); - $rootScope.$digest(); + var callbackSpy = jasmine.createSpy(); + $animate.on('leave', jqLite($document[0].body), callbackSpy); - expect(callbackTriggered).toBe(false); - })); + element = jqLite('
'); + $rootElement.append(element); + $animate.leave(element, $rootElement); + $rootScope.$digest(); - it('leave : should fire a `start` callback when the animation starts with the matching element', - inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { + $$rAF.flush(); - element = jqLite('
'); + expect(callbackSpy).toHaveBeenCalled(); + expect(callbackSpy.calls.count()).toBe(1); + })); - var capturedState; - var capturedElement; - $animate.on('leave', jqLite($document[0].body), function(element, phase) { - capturedState = phase; - capturedElement = element; - }); + it('should trigger a callback for an leave animation (same element)', + inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { - $rootElement.append(element); - $animate.leave(element, $rootElement); - $rootScope.$digest(); - $$rAF.flush(); + var callbackSpy = jasmine.createSpy(); - expect(capturedState).toBe('start'); - expect(capturedElement).toBe(element); - })); + element = jqLite('
'); + $animate.on('leave', element, callbackSpy); + $rootElement.append(element); + var runner = $animate.leave(element, $rootElement); + $rootScope.$digest(); - it('leave : should fire a `close` callback when the animation ends with the matching element', - inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { + $$rAF.flush(); - element = jqLite('
'); + expect(callbackSpy.calls.count()).toBe(1); + expect(callbackSpy.calls.mostRecent().args[1]).toBe('start'); + callbackSpy.calls.reset(); - var capturedState; - var capturedElement; - $animate.on('leave', jqLite($document[0].body), function(element, phase) { - capturedState = phase; - capturedElement = element; - }); + runner.end(); + $$rAF.flush(); - $rootElement.append(element); - var runner = $animate.leave(element, $rootElement); - $rootScope.$digest(); - runner.end(); - $$rAF.flush(); + expect(callbackSpy.calls.count()).toBe(1); + expect(callbackSpy.calls.mostRecent().args[1]).toBe('close'); + })); - expect(capturedState).toBe('close'); - expect(capturedElement).toBe(element); - })); + it('should not fire a callback if the element is outside of the given container', + inject(function($animate, $rootScope, $$rAF, $rootElement) { + + var callbackTriggered = false; + var innerContainer = jqLite('
'); + $rootElement.append(innerContainer); + + $animate.on('leave', innerContainer, + function(element, phase, data) { + callbackTriggered = true; + }); + + element = jqLite('
'); + $rootElement.append(element); + $animate.leave(element, $rootElement); + $rootScope.$digest(); + + expect(callbackTriggered).toBe(false); + })); + + it('should fire a `start` callback when the animation starts', + inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { + + element = jqLite('
'); + + var capturedState; + var capturedElement; + $animate.on('leave', jqLite($document[0].body), function(element, phase) { + capturedState = phase; + capturedElement = element; + }); + + $rootElement.append(element); + $animate.leave(element, $rootElement); + $rootScope.$digest(); + $$rAF.flush(); + + expect(capturedState).toBe('start'); + expect(capturedElement).toBe(element); + })); + + it('should fire a `close` callback when the animation ends', + inject(function($animate, $rootScope, $$rAF, $rootElement, $document) { + + element = jqLite('
'); + + var capturedState; + var capturedElement; + $animate.on('leave', jqLite($document[0].body), function(element, phase) { + capturedState = phase; + capturedElement = element; + }); + + $rootElement.append(element); + var runner = $animate.leave(element, $rootElement); + $rootScope.$digest(); + runner.end(); + $$rAF.flush(); + + expect(capturedState).toBe('close'); + expect(capturedElement).toBe(element); + })); + + it('should remove all event listeners after all callbacks for the "leave:close" phase have been called', + inject(function($animate, $rootScope, $rootElement, $$rAF) { + + var leaveSpy = jasmine.createSpy(); + var addClassSpy = jasmine.createSpy(); + + element = jqLite('
'); + $animate.on('leave', element, leaveSpy); + $animate.on('addClass', element, addClassSpy); + $rootElement.append(element); + var runner = $animate.leave(element, $rootElement); + $animate.flush(); + + runner.end(); + $$rAF.flush(); + + expect(leaveSpy.calls.mostRecent().args[1]).toBe('close'); + + $animate.addClass(element, 'blue'); + + $animate.flush(); + runner.end(); + expect(function() { + $$rAF.flush(); + }).toThrowError('No rAF callbacks present'); + + expect(addClassSpy.calls.count()).toBe(0); + })); + + }); they('should trigger a callback for a $prop animation if the listener is on the document', ['enter', 'leave'], function($event) {