diff --git a/src/.jshintrc b/src/.jshintrc index e29b09f3a848..2467d667e896 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -100,6 +100,7 @@ "assertNotHasOwnProperty": false, "getter": false, "getBlockElements": false, + "tokenDifference": false, /* AngularPublic.js */ "version": false, @@ -162,4 +163,4 @@ "nullFormCtrl": false } -} \ No newline at end of file +} diff --git a/src/Angular.js b/src/Angular.js index 797e4b811ed5..9a8b35b9f082 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -80,7 +80,8 @@ -assertArgFn, -assertNotHasOwnProperty, -getter, - -getBlockElements + -getBlockElements, + -tokenDifference */ @@ -1338,3 +1339,24 @@ 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 0b81853542ee..c9df080ca506 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -677,8 +677,8 @@ function $CompileProvider($provide) { if(key == 'class') { value = value || ''; var current = this.$$element.attr('class') || ''; - this.$removeClass(tokenDifference(current, value).join(' ')); - this.$addClass(tokenDifference(value, current).join(' ')); + this.$removeClass(tokenDifference(current, value)); + this.$addClass(tokenDifference(value, current)); } else { var booleanKey = getBooleanAttrName(this.$$element[0], key), normalizedVal, @@ -736,22 +736,6 @@ function $CompileProvider($provide) { $exceptionHandler(e); } }); - - function tokenDifference(str1, str2) { - var values = [], - tokens1 = str1.split(/\s+/), - tokens2 = str2.split(/\s+/); - - outer: - for(var i=0;i 0) { + removeClass(toRemove); + } + + var toAdd = tokenDifference(newClasses, oldClasses); + if(toAdd.length > 0) { + addClass(toAdd); + } + } + else { + addClass(newClasses); } - addClass(newVal); } oldVal = copy(newVal); } function removeClass(classVal) { - attr.$removeClass(flattenClasses(classVal)); + attr.$removeClass(classVal); } function addClass(classVal) { - attr.$addClass(flattenClasses(classVal)); + attr.$addClass(classVal); } function flattenClasses(classVal) { diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index 3b3c29b92990..335fac9b8960 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -563,7 +563,7 @@ angular.module('ngAnimate', ['ng']) //the animation if any matching animations are not found at all. //NOTE: IE8 + IE9 should close properly (run closeAnimation()) in case a NO animation is not found. if (animationsDisabled(element, parentElement) || matches.length === 0) { - domOperation(); + fireDOMOperation(); closeAnimation(); return; } @@ -596,7 +596,7 @@ angular.module('ngAnimate', ['ng']) //this would mean that an animation was not allowed so let the existing //animation do it's thing and close this one early if(animations.length === 0) { - domOperation(); + fireDOMOperation(); fireDoneCallbackAsync(); return; } @@ -616,7 +616,7 @@ angular.module('ngAnimate', ['ng']) //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))) { - domOperation(); + fireDOMOperation(); fireDoneCallbackAsync(); return; } @@ -627,6 +627,7 @@ angular.module('ngAnimate', ['ng']) element.data(NG_ANIMATE_STATE, { running:true, + className:className, structural:!isClassBased, animations:animations, done:onBeforeAnimationsComplete @@ -637,7 +638,7 @@ angular.module('ngAnimate', ['ng']) invokeRegisteredAnimationFns(animations, 'before', onBeforeAnimationsComplete); function onBeforeAnimationsComplete(cancelled) { - domOperation(); + fireDOMOperation(); if(cancelled === true) { closeAnimation(); return; @@ -695,6 +696,15 @@ angular.module('ngAnimate', ['ng']) doneCallback && $timeout(doneCallback, 0, false); } + //it is less complicated to use a flag than managing and cancelling + //timeouts containing multiple callbacks. + function fireDOMOperation() { + if(!fireDOMOperation.hasBeenRun) { + fireDOMOperation.hasBeenRun = true; + domOperation(); + } + } + function closeAnimation() { if(!closeAnimation.hasBeenRun) { closeAnimation.hasBeenRun = true; diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js index ab996e4d6f1e..62733c85beb7 100644 --- a/test/ng/directive/ngClassSpec.js +++ b/test/ng/directive/ngClassSpec.js @@ -411,4 +411,47 @@ describe('ngClass animations', function() { expect(enterComplete).toBe(true); }); }); + + it("should not remove classes if they're going to be added back right after", function() { + module('mock.animate'); + + inject(function($rootScope, $compile, $animate) { + var className; + + $rootScope.one = true; + $rootScope.two = true; + $rootScope.three = true; + + var element = angular.element('
'); + $compile(element)($rootScope); + $rootScope.$digest(); + + //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); + + $rootScope.three = false; + $rootScope.$digest(); + + className = $animate.flushNext('removeClass').params[1]; + expect(className).toBe('three'); + + expect($animate.queue.length).toBe(0); + + $rootScope.two = false; + $rootScope.three = true; + $rootScope.$digest(); + + className = $animate.flushNext('removeClass').params[1]; + expect(className).toBe('two'); + + className = $animate.flushNext('addClass').params[1]; + expect(className).toBe('three'); + + expect($animate.queue.length).toBe(0); + }); + }); }); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 8ad7ed796686..46bb9538a5df 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -2599,4 +2599,38 @@ describe("ngAnimate", function() { }); }); + it('should only perform the DOM operation once', + inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) { + + if (!$sniffer.transitions) return; + + ss.addRule('.base-class', '-webkit-transition:1s linear all;' + + 'transition:1s linear all;'); + + $animate.enabled(true); + + var element = $compile('
')($rootScope); + $rootElement.append(element); + jqLite($document[0].body).append($rootElement); + + $animate.removeClass(element, 'base-class one two'); + + //still true since we're before the reflow + expect(element.hasClass('base-class')).toBe(true); + + //this will cancel the remove animation + $animate.addClass(element, 'base-class one two'); + + //the cancellation was a success and the class was added right away + //since there was no successive animation for the after animation + expect(element.hasClass('base-class')).toBe(true); + + //the reflow... + $timeout.flush(); + + //the reflow DOM operation was commenced but it ran before so it + //shouldn't run agaun + expect(element.hasClass('base-class')).toBe(true); + })); + });