diff --git a/src/.jshintrc b/src/.jshintrc index 2467d667e896..f32caa451ed6 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -100,7 +100,6 @@ "assertNotHasOwnProperty": false, "getter": false, "getBlockElements": false, - "tokenDifference": false, /* AngularPublic.js */ "version": false, diff --git a/src/Angular.js b/src/Angular.js index b27f4b068a01..11222118db3a 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -81,7 +81,6 @@ -assertNotHasOwnProperty, -getter, -getBlockElements, - -tokenDifference */ @@ -1351,24 +1350,3 @@ function getBlockElements(block) { return jqLite(elements); } - -/** - * Return the string difference between tokens of the original string compared to the old string - * @param {str1} string original string value - * @param {str2} string new string value - */ -function tokenDifference(str1, str2) { - var values = '', - tokens1 = str1.split(/\s+/), - tokens2 = str2.split(/\s+/); - - outer: - for(var i=0;i 0 ? ' ' : '') + token; - } - return values; -} diff --git a/src/ng/compile.js b/src/ng/compile.js index ce3d0514e7bb..d977f173bf7d 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -672,6 +672,24 @@ function $CompileProvider($provide) { } }, + /** + * @ngdoc function + * @name ng.$compile.directive.Attributes#$updateClass + * @methodOf ng.$compile.directive.Attributes + * @function + * + * @description + * Adds and removes the appropriate CSS class values to the element based on the difference + * between the new and old CSS class values (specified as newClasses and oldClasses). + * + * @param {string} newClasses The current CSS className value + * @param {string} oldClasses The former CSS className value + */ + $updateClass : function(newClasses, oldClasses) { + this.$removeClass(tokenDifference(oldClasses, newClasses)); + this.$addClass(tokenDifference(newClasses, oldClasses)); + }, + /** * Set a normalized attribute on the element in a way such that all directives * can share the attribute. This function properly handles boolean attributes. @@ -682,59 +700,53 @@ function $CompileProvider($provide) { * @param {string=} attrName Optional none normalized name. Defaults to key. */ $set: function(key, value, writeAttr, attrName) { - //special case for class attribute addition + removal - //so that class changes can tap into the animation - //hooks provided by the $animate service - if(key == 'class') { - value = value || ''; - var current = this.$$element.attr('class') || ''; - this.$removeClass(tokenDifference(current, value)); - this.$addClass(tokenDifference(value, current)); - } else { - var booleanKey = getBooleanAttrName(this.$$element[0], key), - normalizedVal, - nodeName; + // TODO: decide whether or not to throw an error if "class" + //is set through this function since it may cause $updateClass to + //become unstable. - if (booleanKey) { - this.$$element.prop(key, value); - attrName = booleanKey; - } + var booleanKey = getBooleanAttrName(this.$$element[0], key), + normalizedVal, + nodeName; - this[key] = value; + if (booleanKey) { + this.$$element.prop(key, value); + attrName = booleanKey; + } - // translate normalized key to actual key - if (attrName) { - this.$attr[key] = attrName; - } else { - attrName = this.$attr[key]; - if (!attrName) { - this.$attr[key] = attrName = snake_case(key, '-'); - } + this[key] = value; + + // translate normalized key to actual key + if (attrName) { + this.$attr[key] = attrName; + } else { + attrName = this.$attr[key]; + if (!attrName) { + this.$attr[key] = attrName = snake_case(key, '-'); } + } - nodeName = nodeName_(this.$$element); - - // sanitize a[href] and img[src] values - if ((nodeName === 'A' && key === 'href') || - (nodeName === 'IMG' && key === 'src')) { - // NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case. - if (!msie || msie >= 8 ) { - normalizedVal = urlResolve(value).href; - if (normalizedVal !== '') { - if ((key === 'href' && !normalizedVal.match(aHrefSanitizationWhitelist)) || - (key === 'src' && !normalizedVal.match(imgSrcSanitizationWhitelist))) { - this[key] = value = 'unsafe:' + normalizedVal; - } + nodeName = nodeName_(this.$$element); + + // sanitize a[href] and img[src] values + if ((nodeName === 'A' && key === 'href') || + (nodeName === 'IMG' && key === 'src')) { + // NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case. + if (!msie || msie >= 8 ) { + normalizedVal = urlResolve(value).href; + if (normalizedVal !== '') { + if ((key === 'href' && !normalizedVal.match(aHrefSanitizationWhitelist)) || + (key === 'src' && !normalizedVal.match(imgSrcSanitizationWhitelist))) { + this[key] = value = 'unsafe:' + normalizedVal; } } } + } - if (writeAttr !== false) { - if (value === null || value === undefined) { - this.$$element.removeAttr(attrName); - } else { - this.$$element.attr(attrName, value); - } + if (writeAttr !== false) { + if (value === null || value === undefined) { + this.$$element.removeAttr(attrName); + } else { + this.$$element.attr(attrName, value); } } @@ -1816,9 +1828,19 @@ function $CompileProvider($provide) { attr[name] = interpolateFn(scope); ($$observers[name] || ($$observers[name] = [])).$$inter = true; (attr.$$observers && attr.$$observers[name].$$scope || scope). - $watch(interpolateFn, function interpolateFnWatchAction(value) { - attr.$set(name, value); - }); + $watch(interpolateFn, function interpolateFnWatchAction(newValue, oldValue) { + //special case for class attribute addition + removal + //so that class changes can tap into the animation + //hooks provided by the $animate service. Be sure to + //skip animations when the first digest occurs (when + //both the new and the old values are the same) since + //the CSS classes are the non-interpolated values + if(name === 'class' && newValue != oldValue) { + attr.$updateClass(newValue, oldValue); + } else { + attr.$set(name, newValue); + } + }); } }; } @@ -1958,3 +1980,19 @@ function directiveLinkingFn( /* Element */ rootElement, /* function(Function) */ boundTranscludeFn ){} + +function tokenDifference(str1, str2) { + var values = '', + tokens1 = str1.split(/\s+/), + tokens2 = str2.split(/\s+/); + + outer: + for(var i = 0; i < tokens1.length; i++) { + var token = tokens1[i]; + for(var j = 0; j < tokens2.length; j++) { + if(token == tokens2[j]) continue outer; + } + values += (values.length > 0 ? ' ' : '') + token; + } + return values; +} diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js index 10ef7fd1f94b..21316c5745d6 100644 --- a/src/ng/directive/ngClass.js +++ b/src/ng/directive/ngClass.js @@ -20,11 +20,10 @@ function classDirective(name, selector) { // jshint bitwise: false var mod = $index & 1; if (mod !== old$index & 1) { - if (mod === selector) { - addClass(flattenClasses(scope.$eval(attr[name]))); - } else { - removeClass(flattenClasses(scope.$eval(attr[name]))); - } + var classes = flattenClasses(scope.$eval(attr[name])); + mod === selector ? + attr.$addClass(classes) : + attr.$removeClass(classes); } }); } @@ -33,34 +32,16 @@ function classDirective(name, selector) { function ngClassWatchAction(newVal) { if (selector === true || scope.$index % 2 === selector) { var newClasses = flattenClasses(newVal || ''); - if (oldVal && !equals(newVal,oldVal)) { - var oldClasses = flattenClasses(oldVal); - var toRemove = tokenDifference(oldClasses, newClasses); - if(toRemove.length > 0) { - removeClass(toRemove); - } - - var toAdd = tokenDifference(newClasses, oldClasses); - if(toAdd.length > 0) { - addClass(toAdd); - } - } else { - addClass(newClasses); + if(!oldVal) { + attr.$addClass(newClasses); + } else if(!equals(newVal,oldVal)) { + attr.$updateClass(newClasses, flattenClasses(oldVal)); } } oldVal = copy(newVal); } - function removeClass(classVal) { - attr.$removeClass(classVal); - } - - - function addClass(classVal) { - attr.$addClass(classVal); - } - function flattenClasses(classVal) { if(isArray(classVal)) { return classVal.join(' '); diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 1a287e91df20..81d200431b59 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -549,7 +549,8 @@ angular.module('ngAnimate', ['ng']) and the onComplete callback will be fired once the animation is fully complete. */ function performAnimation(animationEvent, className, element, parentElement, afterElement, domOperation, doneCallback) { - var classes = (element.attr('class') || '') + ' ' + className; + var currentClassName = element.attr('class') || ''; + var classes = currentClassName + ' ' + className; var animationLookup = (' ' + classes).replace(/\s+/g,'.'); if (!parentElement) { parentElement = afterElement ? afterElement.parent() : element.parent(); @@ -602,21 +603,42 @@ angular.module('ngAnimate', ['ng']) return; } + //this value will be searched for class-based CSS className lookup. Therefore, + //we prefix and suffix the current className value with spaces to avoid substring + //lookups of className tokens + var futureClassName = ' ' + currentClassName + ' '; if(ngAnimateState.running) { //if an animation is currently running on the element then lets take the steps //to cancel that animation and fire any required callbacks $timeout.cancel(ngAnimateState.closeAnimationTimeout); cleanup(element); cancelAnimations(ngAnimateState.animations); - (ngAnimateState.done || noop)(true); + + //if the class is removed during the reflow then it will revert the styles temporarily + //back to the base class CSS styling causing a jump-like effect to occur. This check + //here ensures that the domOperation is only performed after the reflow has commenced + if(ngAnimateState.beforeComplete) { + (ngAnimateState.done || noop)(true); + } else if(isClassBased && !ngAnimateState.structural) { + //class-based animations will compare element className values after cancelling the + //previous animation to see if the element properties already contain the final CSS + //class and if so then the animation will be skipped. Since the domOperation will + //be performed only after the reflow is complete then our element's className value + //will be invalid. Therefore the same string manipulation that would occur within the + //DOM operation will be performed below so that the class comparison is valid... + futureClassName = ngAnimateState.event == 'removeClass' ? + futureClassName.replace(ngAnimateState.className, '') : + futureClassName + ngAnimateState.className + ' '; + } } //There is no point in perform a class-based animation if the element already contains //(on addClass) or doesn't contain (on removeClass) the className being animated. //The reason why this is being called after the previous animations are cancelled //is so that the CSS classes present on the element can be properly examined. - if((animationEvent == 'addClass' && element.hasClass(className)) || - (animationEvent == 'removeClass' && !element.hasClass(className))) { + var classNameToken = ' ' + className + ' '; + if((animationEvent == 'addClass' && futureClassName.indexOf(classNameToken) >= 0) || + (animationEvent == 'removeClass' && futureClassName.indexOf(classNameToken) == -1)) { fireDOMOperation(); fireDoneCallbackAsync(); return; @@ -628,6 +650,7 @@ angular.module('ngAnimate', ['ng']) element.data(NG_ANIMATE_STATE, { running:true, + event:animationEvent, className:className, structural:!isClassBased, animations:animations, @@ -748,10 +771,10 @@ angular.module('ngAnimate', ['ng']) function cancelAnimations(animations) { var isCancelledFlag = true; forEach(animations, function(animation) { - if(!animations['beforeComplete']) { + if(!animations.beforeComplete) { (animation.beforeEnd || noop)(isCancelledFlag); } - if(!animations['afterComplete']) { + if(!animations.afterComplete) { (animation.afterEnd || noop)(isCancelledFlag); } }); @@ -978,7 +1001,9 @@ angular.module('ngAnimate', ['ng']) if(timings.transitionDuration > 0) { element.addClass(NG_ANIMATE_FALLBACK_CLASS_NAME); activeClassName += NG_ANIMATE_FALLBACK_ACTIVE_CLASS_NAME + ' '; - node.style[TRANSITION_PROP + PROPERTY_KEY] = 'none'; + blockTransitions(element); + } else { + blockKeyframeAnimations(element); } forEach(className.split(' '), function(klass, i) { @@ -998,6 +1023,25 @@ angular.module('ngAnimate', ['ng']) return true; } + function blockTransitions(element) { + element[0].style[TRANSITION_PROP + PROPERTY_KEY] = 'none'; + } + + function blockKeyframeAnimations(element) { + element[0].style[ANIMATION_PROP] = 'none 0s'; + } + + function unblockTransitions(element) { + var node = element[0], prop = TRANSITION_PROP + PROPERTY_KEY; + if(node.style[prop] && node.style[prop].length > 0) { + node.style[prop] = ''; + } + } + + function unblockKeyframeAnimations(element) { + element[0].style[ANIMATION_PROP] = ''; + } + function animateRun(element, className, activeAnimationComplete) { var data = element.data(NG_ANIMATE_CSS_DATA_KEY); if(!element.hasClass(className) || !data) { @@ -1018,8 +1062,6 @@ angular.module('ngAnimate', ['ng']) var applyFallbackStyle, style = ''; if(timings.transitionDuration > 0) { - node.style[TRANSITION_PROP + PROPERTY_KEY] = ''; - var propertyStyle = timings.transitionPropertyStyle; if(propertyStyle.indexOf('all') == -1) { applyFallbackStyle = true; @@ -1027,6 +1069,8 @@ angular.module('ngAnimate', ['ng']) style += CSS_PREFIX + 'transition-property: ' + propertyStyle + ', ' + fallbackProperty + '; '; style += CSS_PREFIX + 'transition-duration: ' + timings.transitionDurationStyle + ', ' + timings.transitionDuration + 's; '; } + } else { + unblockKeyframeAnimations(element); } if(ii > 0) { @@ -1127,6 +1171,7 @@ angular.module('ngAnimate', ['ng']) //happen in the first place var cancel = preReflowCancellation; afterReflow(function() { + unblockTransitions(element); //once the reflow is complete then we point cancel to //the new cancellation function which will remove all of the //animation properties from the active animation @@ -1190,7 +1235,10 @@ angular.module('ngAnimate', ['ng']) beforeAddClass : function(element, className, animationCompleted) { var cancellationMethod = animateBefore(element, suffixClasses(className, '-add')); if(cancellationMethod) { - afterReflow(animationCompleted); + afterReflow(function() { + unblockTransitions(element); + animationCompleted(); + }); return cancellationMethod; } animationCompleted(); @@ -1203,7 +1251,10 @@ angular.module('ngAnimate', ['ng']) beforeRemoveClass : function(element, className, animationCompleted) { var cancellationMethod = animateBefore(element, suffixClasses(className, '-remove')); if(cancellationMethod) { - afterReflow(animationCompleted); + afterReflow(function() { + unblockTransitions(element); + animationCompleted(); + }); return cancellationMethod; } animationCompleted(); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 85ee17d2ae6e..c25c5040e730 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4479,7 +4479,6 @@ describe('$compile', function() { $compile(element)($rootScope); $rootScope.$digest(); - data = $animate.flushNext('removeClass'); expect(element.hasClass('fire')).toBe(true); diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js index 62733c85beb7..c7307069582b 100644 --- a/test/ng/directive/ngClassSpec.js +++ b/test/ng/directive/ngClassSpec.js @@ -321,7 +321,6 @@ describe('ngClass animations', function() { $rootScope.val = 'one'; $rootScope.$digest(); $animate.flushNext('addClass'); - $animate.flushNext('addClass'); expect($animate.queue.length).toBe(0); $rootScope.val = ''; @@ -428,7 +427,6 @@ describe('ngClass animations', function() { //this fires twice due to the class observer firing className = $animate.flushNext('addClass').params[1]; - className = $animate.flushNext('addClass').params[1]; expect(className).toBe('one two three'); expect($animate.queue.length).toBe(0); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 48ea00414f97..cbf9de63a18d 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -2515,6 +2515,36 @@ describe("ngAnimate", function() { expect(element.hasClass('yellow-add')).toBe(true); })); + it("should cancel and perform the dom operation only after the reflow has run", + inject(function($compile, $rootScope, $animate, $sniffer, $timeout) { + + if (!$sniffer.transitions) return; + + ss.addRule('.green-add', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + ss.addRule('.red-add', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + var element = $compile('
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $animate.addClass(element, 'green'); + expect(element.hasClass('green-add')).toBe(true); + + $animate.addClass(element, 'red'); + expect(element.hasClass('red-add')).toBe(true); + + expect(element.hasClass('green')).toBe(false); + expect(element.hasClass('red')).toBe(false); + + $timeout.flush(); + + expect(element.hasClass('green')).toBe(true); + expect(element.hasClass('red')).toBe(true); + })); + it('should enable and disable animations properly on the root element', function() { var count = 0; module(function($animateProvider) { @@ -2633,4 +2663,62 @@ describe("ngAnimate", function() { expect(element.hasClass('base-class')).toBe(true); })); + it('should block and unblock transitions before the dom operation occurs', + inject(function($rootScope, $compile, $rootElement, $document, $animate, $sniffer, $timeout) { + + if (!$sniffer.transitions) return; + + $animate.enabled(true); + + ss.addRule('.cross-animation', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + var capturedProperty = 'none'; + + var element = $compile('
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + var node = element[0]; + node._setAttribute = node.setAttribute; + node.setAttribute = function(prop, val) { + if(prop == 'class' && val.indexOf('trigger-class') >= 0) { + var propertyKey = ($sniffer.vendorPrefix == 'Webkit' ? '-webkit-' : '') + 'transition-property'; + capturedProperty = element.css(propertyKey); + } + node._setAttribute(prop, val); + }; + + $animate.addClass(element, 'trigger-class'); + + $timeout.flush(); + + expect(capturedProperty).not.toBe('none'); + })); + + it('should block and unblock keyframe animations around the reflow operation', + inject(function($rootScope, $compile, $rootElement, $document, $animate, $sniffer, $timeout) { + + if (!$sniffer.animations) return; + + $animate.enabled(true); + + ss.addRule('.cross-animation', '-webkit-animation:1s my_animation;' + + 'animation:1s my_animation;'); + + var element = $compile('
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + var node = element[0]; + var animationKey = $sniffer.vendorPrefix == 'Webkit' ? 'WebkitAnimation' : 'animation'; + + $animate.addClass(element, 'trigger-class'); + + expect(node.style[animationKey]).toContain('none'); + + $timeout.flush(); + + expect(node.style[animationKey]).not.toContain('none'); + })); }); diff --git a/test/ngRoute/directive/ngViewSpec.js b/test/ngRoute/directive/ngViewSpec.js index e96da022aa80..533f0b53cde6 100644 --- a/test/ngRoute/directive/ngViewSpec.js +++ b/test/ngRoute/directive/ngViewSpec.js @@ -656,7 +656,6 @@ describe('ngView animations', function() { item = $animate.flushNext('enter').element; - $animate.flushNext('addClass').element; $animate.flushNext('addClass').element; expect(item.hasClass('classy')).toBe(true); @@ -676,7 +675,6 @@ describe('ngView animations', function() { $animate.flushNext('enter').element; item = $animate.flushNext('leave').element; - $animate.flushNext('addClass').element; $animate.flushNext('addClass').element; expect(item.hasClass('boring')).toBe(true);