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

Commit

Permalink
fix(ngAnimate): throw an error if a callback is passed to animate met…
Browse files Browse the repository at this point in the history
…hods

As of bf0f550 (released in 1.3.0) it is no longer
valid to pass a callback to the following functions: `enter`, `move`, `leave`, `addClass`,
`removeClass`, `setClass` and `animate`.

To prevent confusing error messages, this change asserts that this parameter is
not a function.

Closes #11826
Closes #11713
  • Loading branch information
petebacondarwin authored and matsko committed May 14, 2015
1 parent 9055b4e commit 39b078b
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/ng/animate.js
Expand Up @@ -170,6 +170,12 @@ var $AnimateProvider = ['$provide', function($provide) {
* page}.
*/
return {
$$assertNoCallback: function(param) {
if (isFunction(param)) {
throw $animateMinErr('nocb', 'Do not pass a callback to animate methods');
}
},

animate: function(element, from, to) {
applyStyles(element, { from: from, to: to });
return asyncPromise();
Expand All @@ -192,6 +198,8 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
enter: function(element, parent, after, options) {
this.$$assertNoCallback(options);

applyStyles(element, options);
after ? after.after(element)
: parent.prepend(element);
Expand All @@ -210,6 +218,8 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
leave: function(element, options) {
this.$$assertNoCallback(options);

applyStyles(element, options);
element.remove();
return asyncPromise();
Expand Down Expand Up @@ -312,6 +322,8 @@ var $AnimateProvider = ['$provide', function($provide) {
* @return {Promise} the animation callback promise
*/
setClass: function(element, add, remove, options) {
this.$$assertNoCallback(options);

var self = this;
var STORAGE_KEY = '$$animateClasses';
var createdCache = false;
Expand Down
2 changes: 2 additions & 0 deletions src/ngAnimate/animate.js
Expand Up @@ -548,6 +548,8 @@ angular.module('ngAnimate', ['ng'])
options.tempClasses = options.tempClasses.split(/\s+/);
}
return options;
} else {
$delegate.$$assertNoCallback(options);
}
}

Expand Down
39 changes: 39 additions & 0 deletions test/ng/animateSpec.js
Expand Up @@ -177,6 +177,45 @@ describe("$animate", function() {
}));
});

it('should throw an error when a callback function is passed as the options param',
inject(function($animate, $rootScope, $document, $rootElement) {

var invalidCallback = function() { };
var element = jqLite('<div></div>');
element.attr('id', 'crazy-man');
var parent = jqLite('<div></div>');
var parent2 = jqLite('<div></div>');
$rootElement.append(parent);
$rootElement.append(parent2);

expect(function() {
$animate.enter(element, parent, parent2, invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

expect(function() {
$animate.move(element, parent, parent2, invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

parent.append(element);

expect(function() {
$animate.addClass(element, 'klass', invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

element.className = 'klass';
expect(function() {
$animate.removeClass(element, 'klass', invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

expect(function() {
$animate.setClass(element, 'one', 'two', invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

expect(function() {
$animate.leave(element, invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
}));

describe('CSS class DOM manipulation', function() {
var element;
var addClass;
Expand Down
38 changes: 38 additions & 0 deletions test/ngAnimate/animateSpec.js
Expand Up @@ -2817,6 +2817,44 @@ describe("ngAnimate", function() {
});

describe("options", function() {
it('should throw an error when a callback function is passed as the options param',
inject(function($animate, $rootScope, $document, $rootElement) {

var invalidCallback = function() { };
var element = jqLite('<div></div>');
element.attr('id', 'crazy-man');
var parent = jqLite('<div></div>');
var parent2 = jqLite('<div></div>');
$rootElement.append(parent);
$rootElement.append(parent2);

expect(function() {
$animate.enter(element, parent, parent2, invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

expect(function() {
$animate.move(element, parent, parent2, invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

parent.append(element);

expect(function() {
$animate.addClass(element, 'klass', invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

element.className = 'klass';
expect(function() {
$animate.removeClass(element, 'klass', invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

expect(function() {
$animate.setClass(element, 'one', 'two', invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');

expect(function() {
$animate.leave(element, invalidCallback);
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
}));

it('should add and remove the temporary className value is provided', function() {
var captures = {};
Expand Down

0 comments on commit 39b078b

Please sign in to comment.