diff --git a/src/.jshintrc b/src/.jshintrc index e2d6c01b7474..d11760dc1e26 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -92,6 +92,13 @@ "skipDestroyOnNextJQueryCleanData": true, + "NODE_TYPE_ELEMENT": false, + "NODE_TYPE_TEXT": false, + "NODE_TYPE_COMMENT": false, + "NODE_TYPE_COMMENT": false, + "NODE_TYPE_DOCUMENT": false, + "NODE_TYPE_DOCUMENT_FRAGMENT": false, + /* filters.js */ "getFirstThursdayOfYear": false, diff --git a/src/Angular.js b/src/Angular.js index 2a7540eb224e..2e37d8925bf8 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -83,6 +83,12 @@ getBlockNodes: true, hasOwnProperty: true, createMap: true, + + NODE_TYPE_ELEMENT: true, + NODE_TYPE_TEXT: true, + NODE_TYPE_COMMENT: true, + NODE_TYPE_DOCUMENT: true, + NODE_TYPE_DOCUMENT_FRAGMENT: true, */ //////////////////////////////////// @@ -192,7 +198,7 @@ function isArrayLike(obj) { var length = obj.length; - if (obj.nodeType === 1 && length) { + if (obj.nodeType === NODE_TYPE_ELEMENT && length) { return true; } @@ -1028,11 +1034,9 @@ function startingTag(element) { // are not allowed to have children. So we just ignore it. element.empty(); } catch(e) {} - // As Per DOM Standards - var TEXT_NODE = 3; var elemHtml = jqLite('<div>').append(element).html(); try { - return element[0].nodeType === TEXT_NODE ? lowercase(elemHtml) : + return element[0].nodeType === NODE_TYPE_TEXT ? lowercase(elemHtml) : elemHtml. match(/^(<[^>]+>)/)[1]. replace(/^<([\w\-]+)/, function(match, nodeName) { return '<' + lowercase(nodeName); }); @@ -1610,3 +1614,9 @@ function getBlockNodes(nodes) { function createMap() { return Object.create(null); } + +var NODE_TYPE_ELEMENT = 1; +var NODE_TYPE_TEXT = 3; +var NODE_TYPE_COMMENT = 8; +var NODE_TYPE_DOCUMENT = 9; +var NODE_TYPE_DOCUMENT_FRAGMENT = 11; diff --git a/src/AngularPublic.js b/src/AngularPublic.js index aaa4bb0376ac..c263e1a9c8cf 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -140,8 +140,7 @@ function publishExternalAPI(angular){ 'getTestability': getTestability, '$$minErr': minErr, '$$csp': csp, - 'reloadWithDebugInfo': reloadWithDebugInfo, - '$$hasClass': jqLiteHasClass + 'reloadWithDebugInfo': reloadWithDebugInfo }); angularModule = setupModuleLoader(window); diff --git a/src/jqLite.js b/src/jqLite.js index b199f390e329..0ed760801b73 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -166,7 +166,7 @@ function jqLiteAcceptsData(node) { // The window object can accept data but has no nodeType // Otherwise we are only interested in elements (1) and documents (9) var nodeType = node.nodeType; - return nodeType === 1 || !nodeType || nodeType === 9; + return nodeType === NODE_TYPE_ELEMENT || !nodeType || nodeType === NODE_TYPE_DOCUMENT; } function jqLiteBuildFragment(html, context) { @@ -419,7 +419,7 @@ function jqLiteController(element, name) { function jqLiteInheritedData(element, name, value) { // if element is the document object work with the html element instead // this makes $(document).scope() possible - if(element.nodeType == 9) { + if(element.nodeType == NODE_TYPE_DOCUMENT) { element = element.documentElement; } var names = isArray(name) ? name : [name]; @@ -432,7 +432,7 @@ function jqLiteInheritedData(element, name, value) { // If dealing with a document fragment node with a host element, and no parent, use the host // element as the parent. This enables directives within a Shadow DOM or polyfilled Shadow DOM // to lookup parent controllers. - element = element.parentNode || (element.nodeType === 11 && element.host); + element = element.parentNode || (element.nodeType === NODE_TYPE_DOCUMENT_FRAGMENT && element.host); } } @@ -610,7 +610,7 @@ forEach({ function getText(element, value) { if (isUndefined(value)) { var nodeType = element.nodeType; - return (nodeType === 1 || nodeType === 3) ? element.textContent : ''; + return (nodeType === NODE_TYPE_ELEMENT || nodeType === NODE_TYPE_TEXT) ? element.textContent : ''; } element.textContent = value; } @@ -832,7 +832,7 @@ forEach({ children: function(element) { var children = []; forEach(element.childNodes, function(element){ - if (element.nodeType === 1) + if (element.nodeType === NODE_TYPE_ELEMENT) children.push(element); }); return children; @@ -844,7 +844,7 @@ forEach({ append: function(element, node) { var nodeType = element.nodeType; - if (nodeType !== 1 && nodeType !== 11) return; + if (nodeType !== NODE_TYPE_ELEMENT && nodeType !== NODE_TYPE_DOCUMENT_FRAGMENT) return; node = new JQLite(node); @@ -855,7 +855,7 @@ forEach({ }, prepend: function(element, node) { - if (element.nodeType === 1) { + if (element.nodeType === NODE_TYPE_ELEMENT) { var index = element.firstChild; forEach(new JQLite(node), function(child){ element.insertBefore(child, index); @@ -906,7 +906,7 @@ forEach({ parent: function(element) { var parent = element.parentNode; - return parent && parent.nodeType !== 11 ? parent : null; + return parent && parent.nodeType !== NODE_TYPE_DOCUMENT_FRAGMENT ? parent : null; }, next: function(element) { diff --git a/src/ng/animate.js b/src/ng/animate.js index ababe39f96e9..f26099e7dac7 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -81,9 +81,57 @@ var $AnimateProvider = ['$provide', function($provide) { return this.$$classNameFilter; }; - this.$get = ['$$q', '$$asyncCallback', function($$q, $$asyncCallback) { + this.$get = ['$$q', '$$asyncCallback', '$rootScope', function($$q, $$asyncCallback, $rootScope) { var currentDefer; + + function runAnimationPostDigest(fn) { + var cancelFn, defer = $$q.defer(); + defer.promise.$$cancelFn = function ngAnimateMaybeCancel() { + cancelFn && cancelFn(); + }; + + $rootScope.$$postDigest(function ngAnimatePostDigest() { + cancelFn = fn(function ngAnimateNotifyComplete() { + defer.resolve(); + }); + }); + + return defer.promise; + } + + function resolveElementClasses(element, cache) { + var toAdd = [], toRemove = []; + + var hasClasses = {}; + forEach((element.attr('class') || '').replace(/\s+/g, ' ').split(' '), function(className) { + hasClasses[className] = true; + }); + + forEach(cache.classes, function(status, className) { + var hasClass = hasClasses[className] === true; + + // If the most recent class manipulation (via $animate) was to remove the class, and the + // element currently has the class, the class is scheduled for removal. Otherwise, if + // the most recent class manipulation (via $animate) was to add the class, and the + // element does not currently have the class, the class is scheduled to be added. + if (status === false && hasClass) { + toRemove.push(className); + } else if (status === true && !hasClass) { + toAdd.push(className); + } + }); + + return (toAdd.length + toRemove.length) > 0 && [toAdd.length && toAdd, toRemove.length && toRemove]; + } + + function cachedClassManipulation(cache, classes, op) { + for (var i=0, ii = classes.length; i < ii; ++i) { + var className = classes[i]; + cache[className] = op; + } + } + function asyncPromise() { // only serve one instance of a promise in order to save CPU cycles if (!currentDefer) { @@ -187,13 +235,17 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ addClass : function(element, className) { + return this.setClass(element, className, []); + }, + + $$addClassImmediately : function addClassImmediately(element, className) { + element = jqLite(element); className = !isString(className) ? (isArray(className) ? className.join(' ') : '') : className; forEach(element, function (element) { jqLiteAddClass(element, className); }); - return asyncPromise(); }, /** @@ -209,6 +261,11 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ removeClass : function(element, className) { + return this.setClass(element, [], className); + }, + + $$removeClassImmediately : function removeClassImmediately(element, className) { + element = jqLite(element); className = !isString(className) ? (isArray(className) ? className.join(' ') : '') : className; @@ -231,10 +288,53 @@ var $AnimateProvider = ['$provide', function($provide) { * @param {string} remove the CSS class which will be removed from the element * @return {Promise} the animation callback promise */ - setClass : function(element, add, remove) { - this.addClass(element, add); - this.removeClass(element, remove); - return asyncPromise(); + setClass : function(element, add, remove, runSynchronously) { + var self = this; + var STORAGE_KEY = '$$animateClasses'; + var createdCache = false; + element = jqLite(element); + + if (runSynchronously) { + // TODO(@caitp/@matsko): Remove undocumented `runSynchronously` parameter, and always + // perform DOM manipulation asynchronously or in postDigest. + self.$$addClassImmediately(element, add); + self.$$removeClassImmediately(element, remove); + return asyncPromise(); + } + + var cache = element.data(STORAGE_KEY); + if (!cache) { + cache = { + classes: {} + }; + createdCache = true; + } + + var classes = cache.classes; + + add = isArray(add) ? add : add.split(' '); + remove = isArray(remove) ? remove : remove.split(' '); + cachedClassManipulation(classes, add, true); + cachedClassManipulation(classes, remove, false); + + if (createdCache) { + cache.promise = runAnimationPostDigest(function(done) { + var cache = element.data(STORAGE_KEY); + element.removeData(STORAGE_KEY); + + var classes = cache && resolveElementClasses(element, cache); + + if (classes) { + if (classes[0]) self.$$addClassImmediately(element, classes[0]); + if (classes[1]) self.$$removeClassImmediately(element, classes[1]); + } + + done(); + }); + element.data(STORAGE_KEY, cache); + } + + return cache.promise; }, enabled : noop, diff --git a/src/ng/compile.js b/src/ng/compile.js index 8f93f7f6d76a..043bab07c860 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1141,7 +1141,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // We can not compile top level text elements since text nodes can be merged and we will // not be able to attach scope data to them, so we will wrap them in <span> forEach($compileNodes, function(node, index){ - if (node.nodeType == 3 /* text node */ && node.nodeValue.match(/\S+/) /* non-empty */ ) { + if (node.nodeType == NODE_TYPE_TEXT && node.nodeValue.match(/\S+/) /* non-empty */ ) { $compileNodes[index] = jqLite(node).wrap('<span></span>').parent()[0]; } }); @@ -1344,7 +1344,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { className; switch(nodeType) { - case 1: /* Element */ + case NODE_TYPE_ELEMENT: /* Element */ // use the node name: <directive> addDirective(directives, directiveNormalize(nodeName_(node)), 'E', maxPriority, ignoreDirective); @@ -1399,10 +1399,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } } break; - case 3: /* Text Node */ + case NODE_TYPE_TEXT: /* Text Node */ addTextInterpolateDirective(directives, node.nodeValue); break; - case 8: /* Comment */ + case NODE_TYPE_COMMENT: /* Comment */ try { match = COMMENT_DIRECTIVE_REGEXP.exec(node.nodeValue); if (match) { @@ -1442,7 +1442,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { "Unterminated attribute, found '{0}' but no matching '{1}' found.", attrStart, attrEnd); } - if (node.nodeType == 1 /** Element **/) { + if (node.nodeType == NODE_TYPE_ELEMENT) { if (node.hasAttribute(attrStart)) depth++; if (node.hasAttribute(attrEnd)) depth--; } @@ -1625,7 +1625,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } compileNode = $template[0]; - if ($template.length != 1 || compileNode.nodeType !== 1) { + if ($template.length != 1 || compileNode.nodeType !== NODE_TYPE_ELEMENT) { throw $compileMinErr('tplrt', "Template for directive '{0}' must have exactly one root element. {1}", directiveName, ''); @@ -2107,7 +2107,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } compileNode = $template[0]; - if ($template.length != 1 || compileNode.nodeType !== 1) { + if ($template.length != 1 || compileNode.nodeType !== NODE_TYPE_ELEMENT) { throw $compileMinErr('tplrt', "Template for directive '{0}' must have exactly one root element. {1}", origAsyncDirective.name, templateUrl); @@ -2533,7 +2533,7 @@ function removeComments(jqNodes) { while (i--) { var node = jqNodes[i]; - if (node.nodeType === 8) { + if (node.nodeType === NODE_TYPE_COMMENT) { splice.call(jqNodes, i, 1); } } diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index a8cb6263a0cf..55eb45a41ac7 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -477,9 +477,14 @@ angular.module('ngAnimate', ['ng']) }); }); + var hasClasses = {}; + forEach((element.attr('class') || '').replace(/\s+/g, ' ').split(' '), function(className) { + hasClasses[className] = true; + }); + var toAdd = [], toRemove = []; forEach(cache.classes, function(status, className) { - var hasClass = angular.$$hasClass(element[0], className); + var hasClass = hasClasses[className] === true; var matchingAnimation = lookup[className] || {}; // When addClass and removeClass is called then $animate will check to @@ -979,7 +984,10 @@ angular.module('ngAnimate', ['ng']) element = stripCommentsFromElement(element); if (classBasedAnimationsBlocked(element)) { - return $delegate.setClass(element, add, remove); + // TODO(@caitp/@matsko): Don't use private/undocumented API here --- we should not be + // changing the DOM synchronously in this case. The `true` parameter must eventually be + // removed. + return $delegate.setClass(element, add, remove, true); } // we're using a combined array for both the add and remove @@ -1033,7 +1041,8 @@ angular.module('ngAnimate', ['ng']) return !classes ? done() : performAnimation('setClass', classes, element, parentElement, null, function() { - $delegate.setClass(element, classes[0], classes[1]); + if (classes[0]) $delegate.$$addClassImmediately(element, classes[0]); + if (classes[1]) $delegate.$$removeClassImmediately(element, classes[1]); }, done); }); }, diff --git a/test/helpers/matchers.js b/test/helpers/matchers.js index 1c3b17c39212..43dc34d97514 100644 --- a/test/helpers/matchers.js +++ b/test/helpers/matchers.js @@ -167,9 +167,13 @@ beforeEach(function() { this.message = function() { return "Expected '" + angular.mock.dump(this.actual) + "' to have class '" + clazz + "'."; }; - return this.actual.hasClass ? - this.actual.hasClass(clazz) : - angular.element(this.actual).hasClass(clazz); + var classes = clazz.trim().split(/\s+/); + for (var i=0; i<classes.length; ++i) { + if (!jqLiteHasClass(this.actual[0], classes[i])) { + return false; + } + } + return true; }, toThrowMatching: function(expected) { diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js index 912012b160c4..ff1f15cbed8f 100644 --- a/test/ng/animateSpec.js +++ b/test/ng/animateSpec.js @@ -50,10 +50,12 @@ describe("$animate", function() { expect(element.text()).toBe('21'); })); - it("should still perform DOM operations even if animations are disabled", inject(function($animate) { + + it("should still perform DOM operations even if animations are disabled (post-digest)", inject(function($animate, $rootScope) { $animate.enabled(false); expect(element).toBeShown(); $animate.addClass(element, 'ng-hide'); + $rootScope.$digest(); expect(element).toBeHidden(); })); @@ -79,15 +81,17 @@ describe("$animate", function() { expect($animate.cancel()).toBeUndefined(); })); - it("should add and remove classes on SVG elements", inject(function($animate) { + it("should add and remove classes on SVG elements", inject(function($animate, $rootScope) { if (!window.SVGElement) return; var svg = jqLite('<svg><rect></rect></svg>'); var rect = svg.children(); $animate.enabled(false); expect(rect).toBeShown(); $animate.addClass(rect, 'ng-hide'); + $rootScope.$digest(); expect(rect).toBeHidden(); $animate.removeClass(rect, 'ng-hide'); + $rootScope.$digest(); expect(rect).not.toBeHidden(); })); @@ -100,4 +104,201 @@ describe("$animate", function() { inject(); }); }); + + describe('CSS class DOM manipulation', function() { + var element; + var addClass; + var removeClass; + + beforeEach(module(provideLog)); + + afterEach(function() { + dealoc(element); + }); + + function setupClassManipulationSpies() { + inject(function($animate) { + addClass = spyOn($animate, '$$addClassImmediately').andCallThrough(); + removeClass = spyOn($animate, '$$removeClassImmediately').andCallThrough(); + }); + } + + function setupClassManipulationLogger(log) { + inject(function($animate) { + var addClassImmediately = $animate.$$addClassImmediately; + var removeClassImmediately = $animate.$$removeClassImmediately; + addClass = spyOn($animate, '$$addClassImmediately').andCallFake(function(element, classes) { + var names = classes; + if (Object.prototype.toString.call(classes) === '[object Array]') names = classes.join( ' '); + log('addClass(' + names + ')'); + return addClassImmediately.call($animate, element, classes); + }); + removeClass = spyOn($animate, '$$removeClassImmediately').andCallFake(function(element, classes) { + var names = classes; + if (Object.prototype.toString.call(classes) === '[object Array]') names = classes.join( ' '); + log('removeClass(' + names + ')'); + return removeClassImmediately.call($animate, element, classes); + }); + }); + } + + it('should defer class manipulation until end of digest', inject(function($rootScope, $animate, log) { + setupClassManipulationLogger(log); + element = jqLite('<p>test</p>'); + + $rootScope.$apply(function() { + $animate.addClass(element, 'test-class1'); + expect(element).not.toHaveClass('test-class1'); + + $animate.removeClass(element, 'test-class1'); + + $animate.addClass(element, 'test-class2'); + expect(element).not.toHaveClass('test-class2'); + + $animate.setClass(element, 'test-class3', 'test-class4'); + expect(element).not.toHaveClass('test-class3'); + expect(element).not.toHaveClass('test-class4'); + expect(log).toEqual([]); + }); + + expect(element).not.toHaveClass('test-class1'); + expect(element).not.toHaveClass('test-class4'); + expect(element).toHaveClass('test-class2'); + expect(element).toHaveClass('test-class3'); + expect(log).toEqual(['addClass(test-class2 test-class3)']); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(0); + })); + + + it('should defer class manipulation until postDigest when outside of digest', inject(function($rootScope, $animate, log) { + setupClassManipulationLogger(log); + element = jqLite('<p class="test-class4">test</p>'); + + $animate.addClass(element, 'test-class1'); + $animate.removeClass(element, 'test-class1'); + $animate.addClass(element, 'test-class2'); + $animate.setClass(element, 'test-class3', 'test-class4'); + + expect(log).toEqual([]); + $rootScope.$digest(); + + + expect(log).toEqual(['addClass(test-class2 test-class3)', 'removeClass(test-class4)']); + expect(element).not.toHaveClass('test-class1'); + expect(element).toHaveClass('test-class2'); + expect(element).toHaveClass('test-class3'); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(1); + })); + + + it('should perform class manipulation in expected order at end of digest', inject(function($rootScope, $animate, log) { + element = jqLite('<p class="test-class3">test</p>'); + + setupClassManipulationLogger(log); + + $rootScope.$apply(function() { + $animate.addClass(element, 'test-class1'); + $animate.addClass(element, 'test-class2'); + $animate.removeClass(element, 'test-class1'); + $animate.removeClass(element, 'test-class3'); + $animate.addClass(element, 'test-class3'); + }); + expect(log).toEqual(['addClass(test-class2)']); + })); + + + it('should return a promise which is resolved on a different turn', inject(function(log, $animate, $browser, $rootScope) { + element = jqLite('<p class="test2">test</p>'); + + $animate.addClass(element, 'test1').then(log.fn('addClass(test1)')); + $animate.removeClass(element, 'test2').then(log.fn('removeClass(test2)')); + + $rootScope.$digest(); + expect(log).toEqual([]); + $browser.defer.flush(); + expect(log).toEqual(['addClass(test1)', 'removeClass(test2)']); + + log.reset(); + element = jqLite('<p class="test4">test</p>'); + + $rootScope.$apply(function() { + $animate.addClass(element, 'test3').then(log.fn('addClass(test3)')); + $animate.removeClass(element, 'test4').then(log.fn('removeClass(test4)')); + expect(log).toEqual([]); + }); + + $browser.defer.flush(); + expect(log).toEqual(['addClass(test3)', 'removeClass(test4)']); + })); + + + it('should defer class manipulation until end of digest for SVG', inject(function($rootScope, $animate) { + if (!window.SVGElement) return; + setupClassManipulationSpies(); + element = jqLite('<svg><g></g></svg>'); + var target = element.children().eq(0); + + $rootScope.$apply(function() { + $animate.addClass(target, 'test-class1'); + expect(target).not.toHaveClass('test-class1'); + + $animate.removeClass(target, 'test-class1'); + + $animate.addClass(target, 'test-class2'); + expect(target).not.toHaveClass('test-class2'); + + $animate.setClass(target, 'test-class3', 'test-class4'); + expect(target).not.toHaveClass('test-class3'); + expect(target).not.toHaveClass('test-class4'); + }); + + expect(target).not.toHaveClass('test-class1'); + expect(target).toHaveClass('test-class2'); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(0); + })); + + + it('should defer class manipulation until postDigest when outside of digest for SVG', inject(function($rootScope, $animate, log) { + if (!window.SVGElement) return; + setupClassManipulationLogger(log); + element = jqLite('<svg><g class="test-class4"></g></svg>'); + var target = element.children().eq(0); + + $animate.addClass(target, 'test-class1'); + $animate.removeClass(target, 'test-class1'); + $animate.addClass(target, 'test-class2'); + $animate.setClass(target, 'test-class3', 'test-class4'); + + expect(log).toEqual([]); + $rootScope.$digest(); + + expect(log).toEqual(['addClass(test-class2 test-class3)', 'removeClass(test-class4)']); + expect(target).not.toHaveClass('test-class1'); + expect(target).toHaveClass('test-class2'); + expect(target).toHaveClass('test-class3'); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(1); + })); + + + it('should perform class manipulation in expected order at end of digest for SVG', inject(function($rootScope, $animate, log) { + if (!window.SVGElement) return; + element = jqLite('<svg><g class="test-class3"></g></svg>'); + var target = element.children().eq(0); + + setupClassManipulationLogger(log); + + $rootScope.$apply(function() { + $animate.addClass(target, 'test-class1'); + $animate.addClass(target, 'test-class2'); + $animate.removeClass(target, 'test-class1'); + $animate.removeClass(target, 'test-class3'); + $animate.addClass(target, 'test-class3'); + }); + expect(log).toEqual(['addClass(test-class2)']); + })); + }); }); diff --git a/test/ng/directive/formSpec.js b/test/ng/directive/formSpec.js index febe0d8ebe54..6595ef9d6457 100644 --- a/test/ng/directive/formSpec.js +++ b/test/ng/directive/formSpec.js @@ -554,17 +554,20 @@ describe('form', function() { expect(doc).toBeValid(); control.$setValidity('error', false); + scope.$digest(); expect(doc).toBeInvalid(); expect(doc.hasClass('ng-valid-error')).toBe(false); expect(doc.hasClass('ng-invalid-error')).toBe(true); control.$setValidity('another', false); + scope.$digest(); expect(doc.hasClass('ng-valid-error')).toBe(false); expect(doc.hasClass('ng-invalid-error')).toBe(true); expect(doc.hasClass('ng-valid-another')).toBe(false); expect(doc.hasClass('ng-invalid-another')).toBe(true); control.$setValidity('error', true); + scope.$digest(); expect(doc).toBeInvalid(); expect(doc.hasClass('ng-valid-error')).toBe(true); expect(doc.hasClass('ng-invalid-error')).toBe(false); @@ -572,6 +575,7 @@ describe('form', function() { expect(doc.hasClass('ng-invalid-another')).toBe(true); control.$setValidity('another', true); + scope.$digest(); expect(doc).toBeValid(); expect(doc.hasClass('ng-valid-error')).toBe(true); expect(doc.hasClass('ng-invalid-error')).toBe(false); @@ -581,6 +585,7 @@ describe('form', function() { // validators are skipped, e.g. becuase of a parser error control.$setValidity('error', null); control.$setValidity('another', null); + scope.$digest(); expect(doc.hasClass('ng-valid-error')).toBe(false); expect(doc.hasClass('ng-invalid-error')).toBe(false); expect(doc.hasClass('ng-valid-another')).toBe(false); @@ -652,7 +657,9 @@ describe('form', function() { expect(input1).toBeDirty(); expect(input2).toBeDirty(); + formCtrl.$setPristine(); + scope.$digest(); expect(form).toBePristine(); expect(formCtrl.$pristine).toBe(true); expect(formCtrl.$dirty).toBe(false); @@ -685,6 +692,7 @@ describe('form', function() { expect(input).toBeDirty(); formCtrl.$setPristine(); + scope.$digest(); expect(form).toBePristine(); expect(formCtrl.$pristine).toBe(true); expect(formCtrl.$dirty).toBe(false); @@ -719,7 +727,9 @@ describe('form', function() { expect(nestedInput).toBeDirty(); formCtrl.$setPristine(); + scope.$digest(); expect(form).toBePristine(); + scope.$digest(); expect(formCtrl.$pristine).toBe(true); expect(formCtrl.$dirty).toBe(false); expect(nestedForm).toBePristine(); diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 6b0dd0d1df43..17aab79183c4 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -892,6 +892,45 @@ describe('NgModelController', function() { dealoc(element); })); + + it('should minimize janky setting of classes during $validate() and ngModelWatch', inject(function($animate, $compile, $rootScope) { + var addClass = $animate.$$addClassImmediately; + var removeClass = $animate.$$removeClassImmediately; + var addClassCallCount = 0; + var removeClassCallCount = 0; + var input; + $animate.$$addClassImmediately = function(element, className) { + if (input && element[0] === input[0]) ++addClassCallCount; + return addClass.call($animate, element, className); + }; + + $animate.$$removeClassImmediately = function(element, className) { + if (input && element[0] === input[0]) ++removeClassCallCount; + return removeClass.call($animate, element, className); + }; + + dealoc(element); + + $rootScope.value = "123456789"; + element = $compile( + '<form name="form">' + + '<input type="text" ng-model="value" name="alias" ng-maxlength="10">' + + '</form>' + )($rootScope); + + var form = $rootScope.form; + input = element.children().eq(0); + + $rootScope.$digest(); + + expect(input).toBeValid(); + expect(input).not.toHaveClass('ng-invalid-maxlength'); + expect(input).toHaveClass('ng-valid-maxlength'); + expect(addClassCallCount).toBe(1); + expect(removeClassCallCount).toBe(0); + + dealoc(element); + })); }); }); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index e2440b343c1a..43748bfdee98 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -1,7 +1,13 @@ 'use strict'; describe("ngAnimate", function() { - + var $originalAnimate; + beforeEach(module(function($provide) { + $provide.decorator('$animate', function($delegate) { + $originalAnimate = $delegate; + return $delegate; + }); + })); beforeEach(module('ngAnimate')); beforeEach(module('ngAnimateMock')); @@ -4871,4 +4877,202 @@ describe("ngAnimate", function() { })); }); }); + + + describe('CSS class DOM manipulation', function() { + var element; + var addClass; + var removeClass; + + beforeEach(module(provideLog)); + + afterEach(function() { + dealoc(element); + }); + + function setupClassManipulationSpies() { + inject(function($animate) { + addClass = spyOn($originalAnimate, '$$addClassImmediately').andCallThrough(); + removeClass = spyOn($originalAnimate, '$$removeClassImmediately').andCallThrough(); + }); + } + + function setupClassManipulationLogger(log) { + inject(function($animate) { + var addClassImmediately = $originalAnimate.$$addClassImmediately; + var removeClassImmediately = $originalAnimate.$$removeClassImmediately; + addClass = spyOn($originalAnimate, '$$addClassImmediately').andCallFake(function(element, classes) { + var names = classes; + if (Object.prototype.toString.call(classes) === '[object Array]') names = classes.join( ' '); + log('addClass(' + names + ')'); + return addClassImmediately.call($originalAnimate, element, classes); + }); + removeClass = spyOn($originalAnimate, '$$removeClassImmediately').andCallFake(function(element, classes) { + var names = classes; + if (Object.prototype.toString.call(classes) === '[object Array]') names = classes.join( ' '); + log('removeClass(' + names + ')'); + return removeClassImmediately.call($originalAnimate, element, classes); + }); + }); + } + + + it('should defer class manipulation until end of digest', inject(function($rootScope, $animate, log) { + setupClassManipulationLogger(log); + element = jqLite('<p>test</p>'); + + $rootScope.$apply(function() { + $animate.addClass(element, 'test-class1'); + expect(element).not.toHaveClass('test-class1'); + + $animate.removeClass(element, 'test-class1'); + + $animate.addClass(element, 'test-class2'); + expect(element).not.toHaveClass('test-class2'); + + $animate.setClass(element, 'test-class3', 'test-class4'); + expect(element).not.toHaveClass('test-class3'); + expect(element).not.toHaveClass('test-class4'); + expect(log).toEqual([]); + }); + + expect(element).not.toHaveClass('test-class1'); + expect(element).not.toHaveClass('test-class4'); + expect(element).toHaveClass('test-class2'); + expect(element).toHaveClass('test-class3'); + expect(log).toEqual(['addClass(test-class2 test-class3)']); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(0); + })); + + + it('should defer class manipulation until postDigest when outside of digest', inject(function($rootScope, $animate, log) { + setupClassManipulationLogger(log); + element = jqLite('<p class="test-class4">test</p>'); + + $animate.addClass(element, 'test-class1'); + $animate.removeClass(element, 'test-class1'); + $animate.addClass(element, 'test-class2'); + $animate.setClass(element, 'test-class3', 'test-class4'); + + expect(log).toEqual([]); + $rootScope.$digest(); + + expect(log).toEqual(['addClass(test-class2 test-class3)', 'removeClass(test-class4)']); + expect(element).not.toHaveClass('test-class1'); + expect(element).toHaveClass('test-class2'); + expect(element).toHaveClass('test-class3'); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(1); + })); + + + it('should perform class manipulation in expected order at end of digest', inject(function($rootScope, $animate, log) { + element = jqLite('<p class="test-class3">test</p>'); + + setupClassManipulationLogger(log); + + $rootScope.$apply(function() { + $animate.addClass(element, 'test-class1'); + $animate.addClass(element, 'test-class2'); + $animate.removeClass(element, 'test-class1'); + $animate.removeClass(element, 'test-class3'); + $animate.addClass(element, 'test-class3'); + }); + expect(log).toEqual(['addClass(test-class2)']); + })); + + + it('should return a promise which is resolved on a different turn', inject(function(log, $animate, $browser, $rootScope) { + element = jqLite('<p class="test2">test</p>'); + + $animate.addClass(element, 'test1').then(log.fn('addClass(test1)')); + $animate.removeClass(element, 'test2').then(log.fn('removeClass(test2)')); + + $rootScope.$digest(); + expect(log).toEqual([]); + $browser.defer.flush(); + expect(log).toEqual(['addClass(test1)', 'removeClass(test2)']); + + log.reset(); + element = jqLite('<p class="test4">test</p>'); + + $rootScope.$apply(function() { + $animate.addClass(element, 'test3').then(log.fn('addClass(test3)')); + $animate.removeClass(element, 'test4').then(log.fn('removeClass(test4)')); + expect(log).toEqual([]); + }); + + $browser.defer.flush(); + expect(log).toEqual(['addClass(test3)', 'removeClass(test4)']); + })); + + + it('should defer class manipulation until end of digest for SVG', inject(function($rootScope, $animate) { + if (!window.SVGElement) return; + setupClassManipulationSpies(); + element = jqLite('<svg><g></g></svg>'); + var target = element.children().eq(0); + + $rootScope.$apply(function() { + $animate.addClass(target, 'test-class1'); + expect(target).not.toHaveClass('test-class1'); + + $animate.removeClass(target, 'test-class1'); + + $animate.addClass(target, 'test-class2'); + expect(target).not.toHaveClass('test-class2'); + + $animate.setClass(target, 'test-class3', 'test-class4'); + expect(target).not.toHaveClass('test-class3'); + expect(target).not.toHaveClass('test-class4'); + }); + + expect(target).not.toHaveClass('test-class1'); + expect(target).toHaveClass('test-class2'); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(0); + })); + + + it('should defer class manipulation until postDigest when outside of digest for SVG', inject(function($rootScope, $animate, log) { + if (!window.SVGElement) return; + setupClassManipulationLogger(log); + element = jqLite('<svg><g class="test-class4"></g></svg>'); + var target = element.children().eq(0); + + $animate.addClass(target, 'test-class1'); + $animate.removeClass(target, 'test-class1'); + $animate.addClass(target, 'test-class2'); + $animate.setClass(target, 'test-class3', 'test-class4'); + + expect(log).toEqual([]); + $rootScope.$digest(); + + expect(log).toEqual(['addClass(test-class2 test-class3)', 'removeClass(test-class4)']); + expect(target).not.toHaveClass('test-class1'); + expect(target).toHaveClass('test-class2'); + expect(target).toHaveClass('test-class3'); + expect(addClass.callCount).toBe(1); + expect(removeClass.callCount).toBe(1); + })); + + + it('should perform class manipulation in expected order at end of digest for SVG', inject(function($rootScope, $animate, log) { + if (!window.SVGElement) return; + element = jqLite('<svg><g class="test-class3"></g></svg>'); + var target = element.children().eq(0); + + setupClassManipulationLogger(log); + + $rootScope.$apply(function() { + $animate.addClass(target, 'test-class1'); + $animate.addClass(target, 'test-class2'); + $animate.removeClass(target, 'test-class1'); + $animate.removeClass(target, 'test-class3'); + $animate.addClass(target, 'test-class3'); + }); + expect(log).toEqual(['addClass(test-class2)']); + })); + }); });