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

fix(ngAnimate): do not use jQuery class API #10329

Closed
wants to merge 2 commits into from
Closed

fix(ngAnimate): do not use jQuery class API #10329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0656ac9
fix(ngAnimate): do not use jQuery class API
caitp Dec 4, 2014
9b442ba
feat($$jqLite): export jqLite as a private service
caitp Dec 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/AngularPublic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@
$TimeoutProvider,
$$RAFProvider,
$$AsyncCallbackProvider,
$WindowProvider
$WindowProvider,
$$jqLiteProvider
*/


Expand Down Expand Up @@ -236,7 +237,8 @@ function publishExternalAPI(angular) {
$timeout: $TimeoutProvider,
$window: $WindowProvider,
$$rAF: $$RAFProvider,
$$asyncCallback: $$AsyncCallbackProvider
$$asyncCallback: $$AsyncCallbackProvider,
$$jqLite: $$jqLiteProvider
});
}
]);
Expand Down
21 changes: 21 additions & 0 deletions src/jqLite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1003,3 +1003,24 @@ forEach({
JQLite.prototype.bind = JQLite.prototype.on;
JQLite.prototype.unbind = JQLite.prototype.off;
});


// Provider for private $$jqLite service
function $$jqLiteProvider() {
this.$get = function $$jqLite() {
return extend(JQLite, {
hasClass: function(node, classes) {
if (node.attr) node = node[0];
return jqLiteHasClass(node, classes);
},
addClass: function(node, classes) {
if (node.attr) node = node[0];
return jqLiteAddClass(node, classes);
},
removeClass: function(node, classes) {
if (node.attr) node = node[0];
return jqLiteRemoveClass(node, classes);
}
});
};
}
39 changes: 20 additions & 19 deletions src/ngAnimate/animate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,12 @@ angular.module('ngAnimate', ['ng'])
function isMatchingElement(elm1, elm2) {
return extractElementNode(elm1) == extractElementNode(elm2);
}

var $$jqLite;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL --- I've added a commit which exposes jqLite via a private service.

Unfortunately, because of the structure of ngAnimate, not every function has access to it. I found it easier to just put a variable here, but I don't see why these functions need to be outside of the decorator, so it's easy to move them too.

What would you prefer?

$provide.decorator('$animate',
['$delegate', '$$q', '$injector', '$sniffer', '$rootElement', '$$asyncCallback', '$rootScope', '$document', '$templateRequest',
function($delegate, $$q, $injector, $sniffer, $rootElement, $$asyncCallback, $rootScope, $document, $templateRequest) {
['$delegate', '$$q', '$injector', '$sniffer', '$rootElement', '$$asyncCallback', '$rootScope', '$document', '$templateRequest', '$$jqLite',
function($delegate, $$q, $injector, $sniffer, $rootElement, $$asyncCallback, $rootScope, $document, $templateRequest, $$$jqLite) {

$$jqLite = $$$jqLite;
$rootElement.data(NG_ANIMATE_STATE, rootAnimateState);

// Wait until all directive and route-related templates are downloaded and
Expand Down Expand Up @@ -1380,10 +1381,10 @@ angular.module('ngAnimate', ['ng'])

//the ng-animate class does nothing, but it's here to allow for
//parent animations to find and cancel child animations when needed
element.addClass(NG_ANIMATE_CLASS_NAME);
$$jqLite.addClass(element, NG_ANIMATE_CLASS_NAME);
if (options && options.tempClasses) {
forEach(options.tempClasses, function(className) {
element.addClass(className);
$$jqLite.addClass(element, className);
});
}

Expand Down Expand Up @@ -1461,7 +1462,7 @@ angular.module('ngAnimate', ['ng'])
closeAnimation.hasBeenRun = true;
if (options && options.tempClasses) {
forEach(options.tempClasses, function(className) {
element.removeClass(className);
$$jqLite.removeClass(element, className);
});
}

Expand Down Expand Up @@ -1523,7 +1524,7 @@ angular.module('ngAnimate', ['ng'])
}

if (removeAnimations || !data.totalActive) {
element.removeClass(NG_ANIMATE_CLASS_NAME);
$$jqLite.removeClass(element, NG_ANIMATE_CLASS_NAME);
element.removeData(NG_ANIMATE_STATE);
}
}
Expand Down Expand Up @@ -1764,22 +1765,22 @@ angular.module('ngAnimate', ['ng'])
var staggerCacheKey = cacheKey + ' ' + staggerClassName;
var applyClasses = !lookupCache[staggerCacheKey];

applyClasses && element.addClass(staggerClassName);
applyClasses && $$jqLite.addClass(element, staggerClassName);

stagger = getElementAnimationDetails(element, staggerCacheKey);

applyClasses && element.removeClass(staggerClassName);
applyClasses && $$jqLite.removeClass(element, staggerClassName);
}

element.addClass(className);
$$jqLite.addClass(element, className);

var formerData = element.data(NG_ANIMATE_CSS_DATA_KEY) || {};
var timings = getElementAnimationDetails(element, eventCacheKey);
var transitionDuration = timings.transitionDuration;
var animationDuration = timings.animationDuration;

if (structural && transitionDuration === 0 && animationDuration === 0) {
element.removeClass(className);
$$jqLite.removeClass(element, className);
return false;
}

Expand Down Expand Up @@ -1851,7 +1852,7 @@ angular.module('ngAnimate', ['ng'])
}

if (!staggerTime) {
element.addClass(activeClassName);
$$jqLite.addClass(element, activeClassName);
if (elementData.blockTransition) {
blockTransitions(node, false);
}
Expand All @@ -1861,7 +1862,7 @@ angular.module('ngAnimate', ['ng'])
var timings = getElementAnimationDetails(element, eventCacheKey);
var maxDuration = Math.max(timings.transitionDuration, timings.animationDuration);
if (maxDuration === 0) {
element.removeClass(activeClassName);
$$jqLite.removeClass(element, activeClassName);
animateClose(element, className);
activeAnimationComplete();
return;
Expand Down Expand Up @@ -1896,7 +1897,7 @@ angular.module('ngAnimate', ['ng'])

var staggerTimeout;
if (staggerTime > 0) {
element.addClass(pendingClassName);
$$jqLite.addClass(element, pendingClassName);
staggerTimeout = $timeout(function() {
staggerTimeout = null;

Expand All @@ -1907,8 +1908,8 @@ angular.module('ngAnimate', ['ng'])
blockAnimations(node, false);
}

element.addClass(activeClassName);
element.removeClass(pendingClassName);
$$jqLite.addClass(element, activeClassName);
$$jqLite.removeClass(element, pendingClassName);

if (styles) {
if (timings.transitionDuration === 0) {
Expand All @@ -1935,8 +1936,8 @@ angular.module('ngAnimate', ['ng'])
// timeout done method.
function onEnd() {
element.off(css3AnimationEvents, onAnimationProgress);
element.removeClass(activeClassName);
element.removeClass(pendingClassName);
$$jqLite.removeClass(element, activeClassName);
$$jqLite.removeClass(element, pendingClassName);
if (staggerTimeout) {
$timeout.cancel(staggerTimeout);
}
Expand Down Expand Up @@ -2024,7 +2025,7 @@ angular.module('ngAnimate', ['ng'])
}

function animateClose(element, className) {
element.removeClass(className);
$$jqLite.removeClass(element, className);
var data = element.data(NG_ANIMATE_CSS_DATA_KEY);
if (data) {
if (data.running) {
Expand Down
10 changes: 5 additions & 5 deletions test/ngAnimate/animateSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5316,7 +5316,7 @@ describe("ngAnimate", function() {
//jQuery doesn't handle SVG elements natively. Instead, an add-on library
//is required which is called jquery.svg.js. Therefore, when jQuery is
//active here there is no point to test this since it won't work by default.
if (!$sniffer.transitions || !_jqLiteMode) return;
if (!$sniffer.transitions) return;

ss.addRule('circle.ng-enter', '-webkit-transition:1s linear all;' +
'transition:1s linear all;');
Expand All @@ -5336,13 +5336,13 @@ describe("ngAnimate", function() {

var child = element.find('circle');

expect(child.hasClass('ng-enter')).toBe(true);
expect(child.hasClass('ng-enter-active')).toBe(true);
expect(jqLiteHasClass(child[0], 'ng-enter')).toBe(true);
expect(jqLiteHasClass(child[0], 'ng-enter-active')).toBe(true);

browserTrigger(child, 'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1 });

expect(child.hasClass('ng-enter')).toBe(false);
expect(child.hasClass('ng-enter-active')).toBe(false);
expect(jqLiteHasClass(child[0], 'ng-enter')).toBe(false);
expect(jqLiteHasClass(child[0], 'ng-enter-active')).toBe(false);
}));


Expand Down