diff --git a/src/ng/animateCss.js b/src/ng/animateCss.js index 552e51095ebf..2e133167a6c3 100644 --- a/src/ng/animateCss.js +++ b/src/ng/animateCss.js @@ -15,10 +15,14 @@ var $CoreAnimateCssProvider = function() { this.$get = ['$$rAF', '$q', '$$AnimateRunner', function($$rAF, $q, $$AnimateRunner) { return function(element, initialOptions) { - // we always make a copy of the options since - // there should never be any side effects on - // the input data when running `$animateCss`. - var options = copy(initialOptions); + // all of the animation functions should create + // a copy of the options data, however, if a + // parent service has already created a copy then + // we should stick to using that + var options = initialOptions || {}; + if (!options.$$prepared) { + options = copy(options); + } // there is no point in applying the styles since // there is no animation that goes on at all in diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js index 278814b1f744..b9757f27777c 100644 --- a/src/ngAnimate/animateCss.js +++ b/src/ngAnimate/animateCss.js @@ -447,10 +447,14 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { } return function init(element, initialOptions) { - // we always make a copy of the options since - // there should never be any side effects on - // the input data when running `$animateCss`. - var options = copy(initialOptions); + // all of the animation functions should create + // a copy of the options data, however, if a + // parent service has already created a copy then + // we should stick to using that + var options = initialOptions || {}; + if (!options.$$prepared) { + options = prepareAnimationOptions(copy(options)); + } var restoreStyles = {}; var node = getDomNode(element); @@ -460,8 +464,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { return closeAndReturnNoopAnimator(); } - options = prepareAnimationOptions(options); - var temporaryStyles = []; var classes = element.attr('class'); var styles = packageStyles(options); diff --git a/test/ng/animateCssSpec.js b/test/ng/animateCssSpec.js index f0a027805cfa..dcc37b7fc9c9 100644 --- a/test/ng/animateCssSpec.js +++ b/test/ng/animateCssSpec.js @@ -31,6 +31,28 @@ describe("$animateCss", function() { expect(copiedOptions).toEqual(initialOptions); })); + it("should not create a copy of the provided options if they have already been prepared earlier", + inject(function($animateCss, $$rAF) { + + var options = { + from: { height: '50px' }, + to: { width: '50px' }, + addClass: 'one', + removeClass: 'two' + }; + + options.$$prepared = true; + var runner = $animateCss(element, options).start(); + runner.end(); + + $$rAF.flush(); + + expect(options.addClass).toBeFalsy(); + expect(options.removeClass).toBeFalsy(); + expect(options.to).toBeFalsy(); + expect(options.from).toBeFalsy(); + })); + it("should apply the provided [from] CSS to the element", inject(function($animateCss) { $animateCss(element, { from: { height: '50px' }}).start(); expect(element.css('height')).toBe('50px'); diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index e43dd89a912c..4d6cdd96bbce 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -2028,6 +2028,28 @@ describe("ngAnimate $animateCss", function() { expect(copiedOptions).toEqual(initialOptions); })); + it("should not create a copy of the provided options if they have already been prepared earlier", + inject(function($animate, $animateCss) { + + var options = { + from: { height: '50px' }, + to: { width: '50px' }, + addClass: 'one', + removeClass: 'two' + }; + + options.$$prepared = true; + var runner = $animateCss(element, options).start(); + runner.end(); + + $animate.flush(); + + expect(options.addClass).toBeFalsy(); + expect(options.removeClass).toBeFalsy(); + expect(options.to).toBeFalsy(); + expect(options.from).toBeFalsy(); + })); + describe("[$$skipPreparationClasses]", function() { it('should not apply and remove the preparation classes to the element when true', inject(function($animateCss) { diff --git a/test/ngAnimate/integrationSpec.js b/test/ngAnimate/integrationSpec.js index 5c67897c8492..7db9d10851e2 100644 --- a/test/ngAnimate/integrationSpec.js +++ b/test/ngAnimate/integrationSpec.js @@ -28,6 +28,27 @@ describe('ngAnimate integration tests', function() { describe('CSS animations', function() { if (!browserSupportsCssAnimations()) return; + it("should only create a single copy of the provided animation options", + inject(function($rootScope, $rootElement, $animate) { + + ss.addRule('.animate-me', 'transition:2s linear all;'); + + var element = jqLite('
'); + html(element); + + var myOptions = {to: { 'color': 'red' }}; + + var spy = spyOn(window, 'copy'); + expect(spy).not.toHaveBeenCalled(); + + var animation = $animate.leave(element, myOptions); + $rootScope.$digest(); + $animate.flush(); + + expect(spy).toHaveBeenCalledOnce(); + dealoc(element); + })); + they('should render an $prop animation', ['enter', 'leave', 'move', 'addClass', 'removeClass', 'setClass'], function(event) { @@ -637,50 +658,77 @@ describe('ngAnimate integration tests', function() { } }); }); + }); - it("should not alter the provided options values in anyway throughout the animation", function() { - var animationSpy = jasmine.createSpy(); - module(function($animateProvider) { - $animateProvider.register('.this-animation', function() { - return { - enter: function(element, done) { - animationSpy(); - done(); - } - }; - }); + it("should not alter the provided options values in anyway throughout the animation", function() { + var animationSpy = jasmine.createSpy(); + module(function($animateProvider) { + $animateProvider.register('.this-animation', function() { + return { + enter: function(element, done) { + animationSpy(); + done(); + } + }; }); + }); - inject(function($animate, $rootScope, $compile) { - element = jqLite('
'); - var child = jqLite('
'); + inject(function($animate, $rootScope, $compile) { + element = jqLite('
'); + var child = jqLite('
'); - var initialOptions = { - from: { height: '50px' }, - to: { width: '100px' }, - addClass: 'one', - removeClass: 'two' - }; + var initialOptions = { + from: { height: '50px' }, + to: { width: '100px' }, + addClass: 'one', + removeClass: 'two' + }; - var copiedOptions = copy(initialOptions); - expect(copiedOptions).toEqual(initialOptions); + var copiedOptions = copy(initialOptions); + expect(copiedOptions).toEqual(initialOptions); - html(element); - $compile(element)($rootScope); + html(element); + $compile(element)($rootScope); - $animate.enter(child, element, null, copiedOptions); - $rootScope.$digest(); - expect(copiedOptions).toEqual(initialOptions); + $animate.enter(child, element, null, copiedOptions); + $rootScope.$digest(); + expect(copiedOptions).toEqual(initialOptions); - $animate.flush(); - expect(copiedOptions).toEqual(initialOptions); + $animate.flush(); + expect(copiedOptions).toEqual(initialOptions); - expect(child).toHaveClass('one'); - expect(child).not.toHaveClass('two'); + expect(child).toHaveClass('one'); + expect(child).not.toHaveClass('two'); - expect(child.attr('style')).toContain('100px'); - expect(child.attr('style')).toContain('50px'); - }); + expect(child.attr('style')).toContain('100px'); + expect(child.attr('style')).toContain('50px'); + }); + }); + + it('should handle ng-if & ng-class with a class that is removed before its add animation has concluded', function() { + inject(function($animate, $rootScope, $compile, $timeout, $$rAF) { + + ss.addRule('.animate-me', 'transition: background-color 0.5s ease;'); + + element = jqLite('
'); + + html(element); + $rootScope.blue = true; + $rootScope.red = true; + $compile(element)($rootScope); + $rootScope.$digest(); + + var child = element.find('div'); + + // Trigger class removal before the add animation has been concluded + $rootScope.blue = false; + $animate.closeAndFlush(); + + expect(child).toHaveClass('red'); + expect(child).not.toHaveClass('blue'); }); }); });