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

Commit 9bb4d6c

Browse files
petebacondarwinmatsko
authored andcommitted
fix(ngAnimate): throw an error if a callback is passed to animate methods
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
1 parent 64c66d0 commit 9bb4d6c

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

src/ng/animate.js

+14
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ var $animateMinErr = minErr('$animate');
44
var ELEMENT_NODE = 1;
55
var NG_ANIMATE_CLASSNAME = 'ng-animate';
66

7+
8+
function assertNoCallback(param) {
9+
if (isFunction(param)) {
10+
throw $animateMinErr('nocb', 'Do not pass a callback to animate methods');
11+
}
12+
}
13+
714
function mergeClasses(a,b) {
815
if (!a && !b) return '';
916
if (!a) return b;
@@ -416,6 +423,7 @@ var $AnimateProvider = ['$provide', function($provide) {
416423
* @return {Promise} the animation callback promise
417424
*/
418425
enter: function(element, parent, after, options) {
426+
assertNoCallback(options);
419427
parent = parent || after.parent();
420428
domInsert(element, parent, after);
421429
return $$animateQueue.push(element, 'enter', options);
@@ -440,6 +448,7 @@ var $AnimateProvider = ['$provide', function($provide) {
440448
* @return {Promise} the animation callback promise
441449
*/
442450
move: function(element, parent, after, options) {
451+
assertNoCallback(options);
443452
parent = parent || after.parent();
444453
domInsert(element, parent, after);
445454
return $$animateQueue.push(element, 'move', options);
@@ -459,6 +468,7 @@ var $AnimateProvider = ['$provide', function($provide) {
459468
* @return {Promise} the animation callback promise
460469
*/
461470
leave: function(element, options) {
471+
assertNoCallback(options);
462472
return $$animateQueue.push(element, 'leave', options, function() {
463473
element.remove();
464474
});
@@ -483,6 +493,7 @@ var $AnimateProvider = ['$provide', function($provide) {
483493
* @return {Promise} the animation callback promise
484494
*/
485495
addClass: function(element, className, options) {
496+
assertNoCallback(options);
486497
options = options || {};
487498
options.addClass = mergeClasses(options.addclass, className);
488499
return $$animateQueue.push(element, 'addClass', options);
@@ -507,6 +518,7 @@ var $AnimateProvider = ['$provide', function($provide) {
507518
* @return {Promise} the animation callback promise
508519
*/
509520
removeClass: function(element, className, options) {
521+
assertNoCallback(options);
510522
options = options || {};
511523
options.removeClass = mergeClasses(options.removeClass, className);
512524
return $$animateQueue.push(element, 'removeClass', options);
@@ -532,6 +544,7 @@ var $AnimateProvider = ['$provide', function($provide) {
532544
* @return {Promise} the animation callback promise
533545
*/
534546
setClass: function(element, add, remove, options) {
547+
assertNoCallback(options);
535548
options = options || {};
536549
options.addClass = mergeClasses(options.addClass, add);
537550
options.removeClass = mergeClasses(options.removeClass, remove);
@@ -560,6 +573,7 @@ var $AnimateProvider = ['$provide', function($provide) {
560573
* @return {Promise} the animation callback promise
561574
*/
562575
animate: function(element, from, to, className, options) {
576+
assertNoCallback(options);
563577
options = options || {};
564578
options.from = options.from ? extend(options.from, from) : from;
565579
options.to = options.to ? extend(options.to, to) : to;

test/ngAnimate/animateSpec.js

+39
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,45 @@ describe("animations", function() {
507507
});
508508
});
509509

510+
it('should throw an error when a callback function is passed as the options param', inject(function($animate, $rootScope, $document) {
511+
512+
var invalidCallback = function() { };
513+
var element = $document[0].createElement('div');
514+
element.setAttribute('id', 'crazy-man');
515+
516+
expect(function() {
517+
$animate.enter(element, parent, parent2, invalidCallback);
518+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
519+
520+
expect(function() {
521+
$animate.move(element, parent, parent2, invalidCallback);
522+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
523+
524+
parent.append(element);
525+
526+
expect(function() {
527+
$animate.addClass(element, 'klass', invalidCallback);
528+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
529+
530+
element.className = 'klass';
531+
expect(function() {
532+
$animate.removeClass(element, 'klass', invalidCallback);
533+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
534+
535+
expect(function() {
536+
$animate.setClass(element, 'one', 'two', invalidCallback);
537+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
538+
539+
expect(function() {
540+
$animate.leave(element, invalidCallback);
541+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
542+
543+
var toStyles = { color: 'red' };
544+
expect(function() {
545+
$animate.animate(element, {}, toStyles, 'klass', invalidCallback);
546+
}).toThrowMinErr('$animate', 'nocb', 'Do not pass a callback to animate methods');
547+
}));
548+
510549
describe('addClass / removeClass', function() {
511550
it('should not perform an animation if there are no valid CSS classes to add',
512551
inject(function($animate, $rootScope) {

0 commit comments

Comments
 (0)