Skip to content

Commit e7d8eee

Browse files
gkalpakgkalpak
committed
refactor(ngAnimate): simplify functions and remove redundant args/calls
Simplifies/Optimizes the following functions: - `areAnimationsAllowed()` - `cleanupEventListeners()` - `closeChildAnimations()` - `clearElementAnimationState()` - `markElementAnimationState()` - `findCallbacks()` Although not its primary aim, this commit also offers a small performance boost to animations (~5% as measured with the `animation-bp` benchmark).
1 parent 565ca6a commit e7d8eee

1 file changed

Lines changed: 58 additions & 71 deletions

File tree

‎src/ngAnimate/animateQueue.js‎

Lines changed: 58 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
3636
}
3737
}
3838

39-
function isAllowed(ruleType, element, currentAnimation, previousAnimation) {
39+
function isAllowed(ruleType, currentAnimation, previousAnimation) {
4040
return rules[ruleType].some(function(fn) {
41-
return fn(element, currentAnimation, previousAnimation);
41+
return fn(currentAnimation, previousAnimation);
4242
});
4343
}
4444

@@ -48,40 +48,40 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
4848
return and ? a && b : a || b;
4949
}
5050

51-
rules.join.push(function(element, newAnimation, currentAnimation) {
51+
rules.join.push(function(newAnimation, currentAnimation) {
5252
// if the new animation is class-based then we can just tack that on
5353
return !newAnimation.structural && hasAnimationClasses(newAnimation);
5454
});
5555

56-
rules.skip.push(function(element, newAnimation, currentAnimation) {
56+
rules.skip.push(function(newAnimation, currentAnimation) {
5757
// there is no need to animate anything if no classes are being added and
5858
// there is no structural animation that will be triggered
5959
return !newAnimation.structural && !hasAnimationClasses(newAnimation);
6060
});
6161

62-
rules.skip.push(function(element, newAnimation, currentAnimation) {
62+
rules.skip.push(function(newAnimation, currentAnimation) {
6363
// why should we trigger a new structural animation if the element will
6464
// be removed from the DOM anyway?
6565
return currentAnimation.event === 'leave' && newAnimation.structural;
6666
});
6767

68-
rules.skip.push(function(element, newAnimation, currentAnimation) {
68+
rules.skip.push(function(newAnimation, currentAnimation) {
6969
// if there is an ongoing current animation then don't even bother running the class-based animation
7070
return currentAnimation.structural && currentAnimation.state === RUNNING_STATE && !newAnimation.structural;
7171
});
7272

73-
rules.cancel.push(function(element, newAnimation, currentAnimation) {
73+
rules.cancel.push(function(newAnimation, currentAnimation) {
7474
// there can never be two structural animations running at the same time
7575
return currentAnimation.structural && newAnimation.structural;
7676
});
7777

78-
rules.cancel.push(function(element, newAnimation, currentAnimation) {
78+
rules.cancel.push(function(newAnimation, currentAnimation) {
7979
// if the previous animation is already running, but the new animation will
8080
// be triggered, but the new animation is structural
8181
return currentAnimation.state === RUNNING_STATE && newAnimation.structural;
8282
});
8383

84-
rules.cancel.push(function(element, newAnimation, currentAnimation) {
84+
rules.cancel.push(function(newAnimation, currentAnimation) {
8585
// cancel the animation if classes added / removed in both animation cancel each other out,
8686
// but only if the current animation isn't structural
8787

@@ -181,10 +181,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
181181
return this === arg || !!(this.compareDocumentPosition(arg) & 16);
182182
};
183183

184-
function findCallbacks(parent, element, event) {
185-
var targetNode = getDomNode(element);
186-
var targetParentNode = getDomNode(parent);
187-
184+
function findCallbacks(targetParentNode, targetNode, event) {
188185
var matches = [];
189186
var entries = callbackRegistry[event];
190187
if (entries) {
@@ -209,11 +206,11 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
209206
});
210207
}
211208

212-
function cleanupEventListeners(phase, element) {
213-
if (phase === 'close' && !element[0].parentNode) {
209+
function cleanupEventListeners(phase, node) {
210+
if (phase === 'close' && !node.parentNode) {
214211
// If the element is not attached to a parentNode, it has been removed by
215212
// the domOperation, and we can safely remove the event callbacks
216-
$animate.off(element);
213+
$animate.off(node);
217214
}
218215
}
219216

@@ -311,12 +308,9 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
311308
// the input data when running `$animateCss`.
312309
var options = copy(initialOptions);
313310

314-
var node, parent;
315311
element = stripCommentsFromElement(element);
316-
if (element) {
317-
node = getDomNode(element);
318-
parent = element.parent();
319-
}
312+
var node = getDomNode(element);
313+
var parentNode = node && node.parentNode;
320314

321315
options = prepareAnimationOptions(options);
322316

@@ -381,7 +375,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
381375
// there is no point in traversing the same collection of parent ancestors if a followup
382376
// animation will be run on the same element that already did all that checking work
383377
if (!skipAnimations && (!hasExistingAnimation || existingAnimation.state !== PRE_DIGEST_STATE)) {
384-
skipAnimations = !areAnimationsAllowed(element, parent, event);
378+
skipAnimations = !areAnimationsAllowed(node, parentNode, event);
385379
}
386380

387381
if (skipAnimations) {
@@ -393,7 +387,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
393387
}
394388

395389
if (isStructural) {
396-
closeChildAnimations(element);
390+
closeChildAnimations(node);
397391
}
398392

399393
var newAnimation = {
@@ -408,7 +402,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
408402
};
409403

410404
if (hasExistingAnimation) {
411-
var skipAnimationFlag = isAllowed('skip', element, newAnimation, existingAnimation);
405+
var skipAnimationFlag = isAllowed('skip', newAnimation, existingAnimation);
412406
if (skipAnimationFlag) {
413407
if (existingAnimation.state === RUNNING_STATE) {
414408
close();
@@ -418,7 +412,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
418412
return existingAnimation.runner;
419413
}
420414
}
421-
var cancelAnimationFlag = isAllowed('cancel', element, newAnimation, existingAnimation);
415+
var cancelAnimationFlag = isAllowed('cancel', newAnimation, existingAnimation);
422416
if (cancelAnimationFlag) {
423417
if (existingAnimation.state === RUNNING_STATE) {
424418
// this will end the animation right away and it is safe
@@ -440,7 +434,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
440434
// a joined animation means that this animation will take over the existing one
441435
// so an example would involve a leave animation taking over an enter. Then when
442436
// the postDigest kicks in the enter will be ignored.
443-
var joinAnimationFlag = isAllowed('join', element, newAnimation, existingAnimation);
437+
var joinAnimationFlag = isAllowed('join', newAnimation, existingAnimation);
444438
if (joinAnimationFlag) {
445439
if (existingAnimation.state === RUNNING_STATE) {
446440
normalizeAnimationDetails(element, newAnimation);
@@ -474,15 +468,15 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
474468

475469
if (!isValidAnimation) {
476470
close();
477-
clearElementAnimationState(element);
471+
clearElementAnimationState(node);
478472
return runner;
479473
}
480474

481475
// the counter keeps track of cancelled animations
482476
var counter = (existingAnimation.counter || 0) + 1;
483477
newAnimation.counter = counter;
484478

485-
markElementAnimationState(element, PRE_DIGEST_STATE, newAnimation);
479+
markElementAnimationState(node, PRE_DIGEST_STATE, newAnimation);
486480

487481
$rootScope.$$postDigest(function() {
488482
var animationDetails = activeAnimationsLookup.get(node);
@@ -523,7 +517,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
523517
// isn't allowed to animate from here then we need to clear the state of the element
524518
// so that any future animations won't read the expired animation data.
525519
if (!isValidAnimation) {
526-
clearElementAnimationState(element);
520+
clearElementAnimationState(node);
527521
}
528522

529523
return;
@@ -535,7 +529,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
535529
? 'setClass'
536530
: animationDetails.event;
537531

538-
markElementAnimationState(element, RUNNING_STATE);
532+
markElementAnimationState(node, RUNNING_STATE);
539533
var realRunner = $$animation(element, event, animationDetails.options);
540534

541535
// this will update the runner's flow-control events based on
@@ -547,7 +541,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
547541
close(!status);
548542
var animationDetails = activeAnimationsLookup.get(node);
549543
if (animationDetails && animationDetails.counter === counter) {
550-
clearElementAnimationState(getDomNode(element));
544+
clearElementAnimationState(node);
551545
}
552546
notifyProgress(runner, event, 'close', {});
553547
});
@@ -557,7 +551,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
557551

558552
function notifyProgress(runner, event, phase, data) {
559553
runInNextPostDigestOrNow(function() {
560-
var callbacks = findCallbacks(parent, element, event);
554+
var callbacks = findCallbacks(parentNode, node, event);
561555
if (callbacks.length) {
562556
// do not optimize this call here to RAF because
563557
// we don't know how heavy the callback code here will
@@ -567,10 +561,10 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
567561
forEach(callbacks, function(callback) {
568562
callback(element, phase, data);
569563
});
570-
cleanupEventListeners(phase, element);
564+
cleanupEventListeners(phase, node);
571565
});
572566
} else {
573-
cleanupEventListeners(phase, element);
567+
cleanupEventListeners(phase, node);
574568
}
575569
});
576570
runner.progress(event, phase, data);
@@ -585,8 +579,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
585579
}
586580
}
587581

588-
function closeChildAnimations(element) {
589-
var node = getDomNode(element);
582+
function closeChildAnimations(node) {
590583
var children = node.querySelectorAll('[' + NG_ANIMATE_ATTR_NAME + ']');
591584
forEach(children, function(child) {
592585
var state = parseInt(child.getAttribute(NG_ANIMATE_ATTR_NAME), 10);
@@ -604,71 +597,66 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
604597
});
605598
}
606599

607-
function clearElementAnimationState(element) {
608-
var node = getDomNode(element);
600+
function clearElementAnimationState(node) {
609601
node.removeAttribute(NG_ANIMATE_ATTR_NAME);
610602
activeAnimationsLookup.remove(node);
611603
}
612604

613-
function isMatchingElement(nodeOrElmA, nodeOrElmB) {
614-
return getDomNode(nodeOrElmA) === getDomNode(nodeOrElmB);
615-
}
616-
617605
/**
618606
* This fn returns false if any of the following is true:
619607
* a) animations on any parent element are disabled, and animations on the element aren't explicitly allowed
620608
* b) a parent element has an ongoing structural animation, and animateChildren is false
621609
* c) the element is not a child of the body
622610
* d) the element is not a child of the $rootElement
623611
*/
624-
function areAnimationsAllowed(element, parentElement, event) {
625-
var bodyElement = jqLite($document[0].body);
626-
var bodyElementDetected = isMatchingElement(element, bodyElement) || element[0].nodeName === 'HTML';
627-
var rootElementDetected = isMatchingElement(element, $rootElement);
612+
function areAnimationsAllowed(node, parentNode, event) {
613+
var bodyNode = $document[0].body;
614+
var rootNode = getDomNode($rootElement);
615+
616+
var bodyNodeDetected = (node === bodyNode) || node.nodeName === 'HTML';
617+
var rootNodeDetected = (node === rootNode);
628618
var parentAnimationDetected = false;
619+
var elementDisabled = disabledElementsLookup.get(node);
629620
var animateChildren;
630-
var elementDisabled = disabledElementsLookup.get(getDomNode(element));
631621

632-
var parentHost = jqLite.data(element[0], NG_ANIMATE_PIN_DATA);
622+
var parentHost = jqLite.data(node, NG_ANIMATE_PIN_DATA);
633623
if (parentHost) {
634-
parentElement = parentHost;
624+
parentNode = getDomNode(parentHost);
635625
}
636626

637-
parentElement = getDomNode(parentElement);
638-
639-
while (parentElement) {
640-
if (!rootElementDetected) {
627+
while (parentNode) {
628+
if (!rootNodeDetected) {
641629
// angular doesn't want to attempt to animate elements outside of the application
642630
// therefore we need to ensure that the rootElement is an ancestor of the current element
643-
rootElementDetected = isMatchingElement(parentElement, $rootElement);
631+
rootNodeDetected = (parentNode === rootNode);
644632
}
645633

646-
if (parentElement.nodeType !== ELEMENT_NODE) {
634+
if (parentNode.nodeType !== ELEMENT_NODE) {
647635
// no point in inspecting the #document element
648636
break;
649637
}
650638

651-
var details = activeAnimationsLookup.get(parentElement) || {};
639+
var details = activeAnimationsLookup.get(parentNode) || {};
652640
// either an enter, leave or move animation will commence
653641
// therefore we can't allow any animations to take place
654642
// but if a parent animation is class-based then that's ok
655643
if (!parentAnimationDetected) {
656-
var parentElementDisabled = disabledElementsLookup.get(parentElement);
644+
var parentNodeDisabled = disabledElementsLookup.get(parentNode);
657645

658-
if (parentElementDisabled === true && elementDisabled !== false) {
646+
if (parentNodeDisabled === true && elementDisabled !== false) {
659647
// disable animations if the user hasn't explicitly enabled animations on the
660648
// current element
661649
elementDisabled = true;
662650
// element is disabled via parent element, no need to check anything else
663651
break;
664-
} else if (parentElementDisabled === false) {
652+
} else if (parentNodeDisabled === false) {
665653
elementDisabled = false;
666654
}
667655
parentAnimationDetected = details.structural;
668656
}
669657

670658
if (isUndefined(animateChildren) || animateChildren === true) {
671-
var value = jqLite.data(parentElement, NG_ANIMATE_CHILDREN_DATA);
659+
var value = jqLite.data(parentNode, NG_ANIMATE_CHILDREN_DATA);
672660
if (isDefined(value)) {
673661
animateChildren = value;
674662
}
@@ -677,40 +665,39 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
677665
// there is no need to continue traversing at this point
678666
if (parentAnimationDetected && animateChildren === false) break;
679667

680-
if (!bodyElementDetected) {
668+
if (!bodyNodeDetected) {
681669
// we also need to ensure that the element is or will be a part of the body element
682670
// otherwise it is pointless to even issue an animation to be rendered
683-
bodyElementDetected = isMatchingElement(parentElement, bodyElement);
671+
bodyNodeDetected = (parentNode === bodyNode);
684672
}
685673

686-
if (bodyElementDetected && rootElementDetected) {
674+
if (bodyNodeDetected && rootNodeDetected) {
687675
// If both body and root have been found, any other checks are pointless,
688676
// as no animation data should live outside the application
689677
break;
690678
}
691679

692-
if (!rootElementDetected) {
693-
// If no rootElement is detected, check if the parentElement is pinned to another element
694-
parentHost = jqLite.data(parentElement, NG_ANIMATE_PIN_DATA);
680+
if (!rootNodeDetected) {
681+
// If `rootNode` is not detected, check if `parentNode` is pinned to another element
682+
parentHost = jqLite.data(parentNode, NG_ANIMATE_PIN_DATA);
695683
if (parentHost) {
696684
// The pin target element becomes the next parent element
697-
parentElement = getDomNode(parentHost);
685+
parentNode = getDomNode(parentHost);
698686
continue;
699687
}
700688
}
701689

702-
parentElement = parentElement.parentNode;
690+
parentNode = parentNode.parentNode;
703691
}
704692

705693
var allowAnimation = (!parentAnimationDetected || animateChildren) && elementDisabled !== true;
706-
return allowAnimation && rootElementDetected && bodyElementDetected;
694+
return allowAnimation && rootNodeDetected && bodyNodeDetected;
707695
}
708696

709-
function markElementAnimationState(element, state, details) {
697+
function markElementAnimationState(node, state, details) {
710698
details = details || {};
711699
details.state = state;
712700

713-
var node = getDomNode(element);
714701
node.setAttribute(NG_ANIMATE_ATTR_NAME, state);
715702

716703
var oldValue = activeAnimationsLookup.get(node);

0 commit comments

Comments
 (0)