Skip to content

Commit

Permalink
fix($animate): ignore invalid option parameter values
Browse files Browse the repository at this point in the history
Prior to this fix there was another patch that threw an exception if the
provided options value was not of an object type. While this is correct
in terms of logic, it caused issues with plugins and tools that are
designed to work with multiple version of Angular. This fix ensures that
these plugins work since an invalid options value is ignored by
`$animate`.

Closes angular#11826
  • Loading branch information
matsko committed May 26, 2015
1 parent 0fc5851 commit c3e149f
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 7 deletions.
20 changes: 13 additions & 7 deletions src/ng/animate.js
Expand Up @@ -40,6 +40,12 @@ function splitClasses(classes) {
return obj;
}

function prepareAnimateOptions(options) {
return isObject(options)
? options
: {};
}

var $$CoreAnimateRunnerProvider = function() {
this.$get = ['$q', '$$rAF', function($q, $$rAF) {
function AnimateRunner() {}
Expand Down Expand Up @@ -420,7 +426,7 @@ var $AnimateProvider = ['$provide', function($provide) {
after = after && jqLite(after);
parent = parent || after.parent();
domInsert(element, parent, after);
return $$animateQueue.push(element, 'enter', options);
return $$animateQueue.push(element, 'enter', prepareAnimateOptions(options));
},

/**
Expand All @@ -446,7 +452,7 @@ var $AnimateProvider = ['$provide', function($provide) {
after = after && jqLite(after);
parent = parent || after.parent();
domInsert(element, parent, after);
return $$animateQueue.push(element, 'move', options);
return $$animateQueue.push(element, 'move', prepareAnimateOptions(options));
},

/**
Expand All @@ -463,7 +469,7 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
leave: function(element, options) {
return $$animateQueue.push(element, 'leave', options, function() {
return $$animateQueue.push(element, 'leave', prepareAnimateOptions(options), function() {
element.remove();
});
},
Expand All @@ -487,7 +493,7 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
addClass: function(element, className, options) {
options = options || {};
options = prepareAnimateOptions(options);
options.addClass = mergeClasses(options.addclass, className);
return $$animateQueue.push(element, 'addClass', options);
},
Expand All @@ -511,7 +517,7 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
removeClass: function(element, className, options) {
options = options || {};
options = prepareAnimateOptions(options);
options.removeClass = mergeClasses(options.removeClass, className);
return $$animateQueue.push(element, 'removeClass', options);
},
Expand All @@ -536,7 +542,7 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
setClass: function(element, add, remove, options) {
options = options || {};
options = prepareAnimateOptions(options);
options.addClass = mergeClasses(options.addClass, add);
options.removeClass = mergeClasses(options.removeClass, remove);
return $$animateQueue.push(element, 'setClass', options);
Expand Down Expand Up @@ -564,7 +570,7 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
animate: function(element, from, to, className, options) {
options = options || {};
options = prepareAnimateOptions(options);
options.from = options.from ? extend(options.from, from) : from;
options.to = options.to ? extend(options.to, to) : to;

Expand Down
76 changes: 76 additions & 0 deletions test/ng/animateSpec.js
Expand Up @@ -242,6 +242,82 @@ describe("$animate", function() {
expect(element[0].previousSibling).toBe(after);
});
});

they('$prop() should operate using a native DOM element',
['enter', 'move', 'leave', 'addClass', 'removeClass', 'setClass', 'animate'], function(event) {

var captureSpy = jasmine.createSpy();

module(function($provide) {
$provide.value('$$animateQueue', {
push: captureSpy
});
});

inject(function($animate, $rootScope, $document, $rootElement) {
var element = jqLite('<div></div>');
var parent2 = jqLite('<div></div>');
var parent = $rootElement;
parent.append(parent2);

if (event !== 'enter' && event !== 'move') {
parent.append(element);
}

var fn, invalidOptions = function() { };

switch (event) {
case 'enter':
case 'move':
fn = function() {
$animate[event](element, parent, parent2, invalidOptions);
};
break;

case 'addClass':
fn = function() {
$animate.addClass(element, 'klass', invalidOptions);
};
break;

case 'removeClass':
element.className = 'klass';
fn = function() {
$animate.removeClass(element, 'klass', invalidOptions);
};
break;

case 'setClass':
element.className = 'two';
fn = function() {
$animate.setClass(element, 'one', 'two', invalidOptions);
};
break;

case 'leave':
fn = function() {
$animate.leave(element, invalidOptions);
};
break;

case 'animate':
var toStyles = { color: 'red' };
fn = function() {
$animate.animate(element, {}, toStyles, 'klass', invalidOptions);
};
break;
}

expect(function() {
fn();
$rootScope.$digest();
}).not.toThrow();

var optionsArg = captureSpy.mostRecentCall.args[2];
expect(optionsArg).not.toBe(invalidOptions);
expect(isObject(optionsArg)).toBeTruthy();
});
});
});

describe('CSS class DOM manipulation', function() {
Expand Down

0 comments on commit c3e149f

Please sign in to comment.