From ab0c7f251672dd4a5ecc67c9f5c8e25e62fe8306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 8 Oct 2014 14:29:57 +0300 Subject: [PATCH 1/2] feat($animate): enable temporary classes to be applied during an animation --- src/ngAnimate/animate.js | 61 +++++++++++++++++++++-------- src/ngMock/angular-mocks.js | 1 + test/ngAnimate/animateSpec.js | 72 +++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js index a8cb6263a0cf..945667b7eee2 100644 --- a/src/ngAnimate/animate.js +++ b/src/ngAnimate/animate.js @@ -377,6 +377,7 @@ angular.module('ngAnimate', ['ng']) var forEach = angular.forEach; var selectors = $animateProvider.$$selectors; var isArray = angular.isArray; + var isString = angular.isString; var ELEMENT_NODE = 1; var NG_ANIMATE_STATE = '$$ngAnimateState'; @@ -467,6 +468,14 @@ angular.module('ngAnimate', ['ng']) return defer.promise; } + function parseAnimateOptions(options) { + // some plugin code may still be passing in the callback + // function as the last param for the $animate methods so + // it's best to only allow string or array values for now + if (isArray(options)) return options; + if (isString(options)) return [options]; + } + function resolveElementClasses(element, cache, runningAnimations) { runningAnimations = runningAnimations || {}; @@ -779,7 +788,8 @@ angular.module('ngAnimate', ['ng']) * @param {DOMElement} afterElement the sibling element (which is the previous element) of the element that will be the focus of the enter animation * @return {Promise} the animation callback promise */ - enter : function(element, parentElement, afterElement) { + enter : function(element, parentElement, afterElement, options) { + options = parseAnimateOptions(options); element = angular.element(element); parentElement = prepareElement(parentElement); afterElement = prepareElement(afterElement); @@ -787,7 +797,7 @@ angular.module('ngAnimate', ['ng']) classBasedAnimationsBlocked(element, true); $delegate.enter(element, parentElement, afterElement); return runAnimationPostDigest(function(done) { - return performAnimation('enter', 'ng-enter', stripCommentsFromElement(element), parentElement, afterElement, noop, done); + return performAnimation('enter', 'ng-enter', stripCommentsFromElement(element), parentElement, afterElement, noop, options, done); }); }, @@ -821,7 +831,8 @@ angular.module('ngAnimate', ['ng']) * @param {DOMElement} element the element that will be the focus of the leave animation * @return {Promise} the animation callback promise */ - leave : function(element) { + leave : function(element, options) { + options = parseAnimateOptions(options); element = angular.element(element); cancelChildAnimations(element); @@ -830,7 +841,7 @@ angular.module('ngAnimate', ['ng']) return runAnimationPostDigest(function(done) { return performAnimation('leave', 'ng-leave', stripCommentsFromElement(element), null, null, function() { $delegate.leave(element); - }, done); + }, options, done); }); }, @@ -867,7 +878,8 @@ angular.module('ngAnimate', ['ng']) * @param {DOMElement} afterElement the sibling element (which is the previous element) of the element that will be the focus of the move animation * @return {Promise} the animation callback promise */ - move : function(element, parentElement, afterElement) { + move : function(element, parentElement, afterElement, options) { + options = parseAnimateOptions(options); element = angular.element(element); parentElement = prepareElement(parentElement); afterElement = prepareElement(afterElement); @@ -876,7 +888,7 @@ angular.module('ngAnimate', ['ng']) classBasedAnimationsBlocked(element, true); $delegate.move(element, parentElement, afterElement); return runAnimationPostDigest(function(done) { - return performAnimation('move', 'ng-move', stripCommentsFromElement(element), parentElement, afterElement, noop, done); + return performAnimation('move', 'ng-move', stripCommentsFromElement(element), parentElement, afterElement, noop, options, done); }); }, @@ -909,8 +921,8 @@ angular.module('ngAnimate', ['ng']) * @param {string} className the CSS class that will be added to the element and then animated * @return {Promise} the animation callback promise */ - addClass : function(element, className) { - return this.setClass(element, className, []); + addClass : function(element, className, options) { + return this.setClass(element, className, [], options); }, /** @@ -942,8 +954,8 @@ angular.module('ngAnimate', ['ng']) * @param {string} className the CSS class that will be animated and then removed from the element * @return {Promise} the animation callback promise */ - removeClass : function(element, className) { - return this.setClass(element, [], className); + removeClass : function(element, className, options) { + return this.setClass(element, [], className, options); }, /** @@ -973,7 +985,9 @@ angular.module('ngAnimate', ['ng']) * CSS classes have been set on the element * @return {Promise} the animation callback promise */ - setClass : function(element, add, remove) { + setClass : function(element, add, remove, options) { + options = parseAnimateOptions(options); + var STORAGE_KEY = '$$animateClasses'; element = angular.element(element); element = stripCommentsFromElement(element); @@ -1007,11 +1021,16 @@ angular.module('ngAnimate', ['ng']) }); if (hasCache) { + if (options && cache.options) { + cache.options = cache.options.concat(options); + } + //the digest cycle will combine all the animations into one function return cache.promise; } else { element.data(STORAGE_KEY, cache = { - classes : classes + classes : classes, + options : options }); } @@ -1034,7 +1053,7 @@ angular.module('ngAnimate', ['ng']) ? done() : performAnimation('setClass', classes, element, parentElement, null, function() { $delegate.setClass(element, classes[0], classes[1]); - }, done); + }, cache.options, done); }); }, @@ -1096,7 +1115,7 @@ angular.module('ngAnimate', ['ng']) CSS code. Element, parentElement and afterElement are provided DOM elements for the animation and the onComplete callback will be fired once the animation is fully complete. */ - function performAnimation(animationEvent, className, element, parentElement, afterElement, domOperation, doneCallback) { + function performAnimation(animationEvent, className, element, parentElement, afterElement, domOperation, options, doneCallback) { var noopCancel = noop; var runner = animationRunner(element, animationEvent, className); @@ -1204,6 +1223,11 @@ 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); + if (isArray(options)) { + forEach(options, function(className) { + element.addClass(className); + }); + } var localAnimationCount = globalAnimationCounter++; totalActiveAnimations++; @@ -1273,8 +1297,15 @@ angular.module('ngAnimate', ['ng']) function closeAnimation() { if (!closeAnimation.hasBeenRun) { closeAnimation.hasBeenRun = true; + if (isArray(options)) { + forEach(options, function(className) { + element.removeClass(className); + }); + } + var data = element.data(NG_ANIMATE_STATE); if (data) { + /* only structural animations wait for reflow before removing an animation, but class-based animations don't. An example of this failing would be when a parent HTML tag has a ng-class attribute @@ -1539,7 +1570,7 @@ angular.module('ngAnimate', ['ng']) function parseMaxTime(str) { var maxValue = 0; - var values = angular.isString(str) ? + var values = isString(str) ? str.split(/\s*,\s*/) : []; forEach(values, function(value) { diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 7d942204e56b..00e4700e2214 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -809,6 +809,7 @@ angular.mock.animate = angular.module('ngAnimateMock', ['ng']) animate.queue.push({ event : method, element : arguments[0], + options : arguments[arguments.length-1], args : arguments }); return $delegate[method].apply($delegate, arguments); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index e2440b343c1a..3815caf36a36 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -2450,6 +2450,78 @@ describe("ngAnimate", function() { })); }); + describe("options", function() { + + it('should add and remove the temporary className value is provided', function() { + var captures = {}; + module(function($animateProvider) { + $animateProvider.register('.capture', function() { + return { + enter : capture('enter'), + leave : capture('leave'), + move : capture('move'), + addClass : capture('addClass'), + removeClass : capture('removeClass'), + setClass : capture('setClass') + }; + + function capture(event) { + return function(element, add, remove, done) { + //some animations only have one extra param + done = done || remove || add; + captures[event]=done; + }; + } + }); + }); + inject(function($animate, $rootScope, $compile, $rootElement, $document) { + var container = jqLite('
'); + var container2 = jqLite('
'); + var element = jqLite('
'); + $rootElement.append(container); + $rootElement.append(container2); + angular.element($document[0].body).append($rootElement); + + $compile(element)($rootScope); + + assertTempClass('enter', 'temp-enter', function() { + $animate.enter(element, container, null, 'temp-enter'); + }); + + assertTempClass('move', 'temp-move', function() { + $animate.move(element, null, container2, 'temp-move'); + }); + + assertTempClass('addClass', 'temp-add', function() { + $animate.addClass(element, 'add', 'temp-add'); + }); + + assertTempClass('removeClass', 'temp-remove', function() { + $animate.removeClass(element, 'add', 'temp-remove'); + }); + + element.addClass('remove'); + assertTempClass('setClass', 'temp-set', function() { + $animate.setClass(element, 'add', 'remove', 'temp-set'); + }); + + assertTempClass('leave', 'temp-leave', function() { + $animate.leave(element, 'temp-leave'); + }); + + function assertTempClass(event, className, animationOperation) { + expect(element).not.toHaveClass(className); + animationOperation(); + $rootScope.$digest(); + expect(element).toHaveClass(className); + $animate.triggerReflow(); + captures[event](); + $animate.triggerCallbacks(); + expect(element).not.toHaveClass(className); + } + }); + }); + }); describe("addClass / removeClass", function() { From 000da62cb34acc96804b83499f08a6d71d74ce02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 8 Oct 2014 14:36:33 +0300 Subject: [PATCH 2/2] fix($animate): ensure hidden elements with ngShow/ngHide stay hidden during animations Prior to this fix if an element that contained ng-show or ng-hide was in its hidden state then any other animation run on the same element would cause the animation to appear despite the element itself already being hidden. This patch ensures that NO animations are visible even if the element is set as hidden. Closes #9103 --- css/angular.css | 2 +- src/ng/directive/ngShowHide.js | 12 ++++++-- test/ng/directive/ngShowHideSpec.js | 44 +++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/css/angular.css b/css/angular.css index b9eda79e745d..81da6c4a111c 100644 --- a/css/angular.css +++ b/css/angular.css @@ -2,7 +2,7 @@ [ng\:cloak], [ng-cloak], [data-ng-cloak], [x-ng-cloak], .ng-cloak, .x-ng-cloak, -.ng-hide:not(.ng-animate) { +.ng-hide:not(.ng-hide-animate) { display: none !important; } diff --git a/src/ng/directive/ngShowHide.js b/src/ng/directive/ngShowHide.js index 7c4baa730abc..ebcc05b14111 100644 --- a/src/ng/directive/ngShowHide.js +++ b/src/ng/directive/ngShowHide.js @@ -1,5 +1,7 @@ 'use strict'; +var NG_HIDE_CLASS = 'ng-hide'; +var NG_HIDE_IN_PROGRESS_CLASS = 'ng-hide-animate'; /** * @ngdoc directive * @name ngShow @@ -161,7 +163,11 @@ var ngShowDirective = ['$animate', function($animate) { multiElement: true, link: function(scope, element, attr) { scope.$watch(attr.ngShow, function ngShowWatchAction(value){ - $animate[value ? 'removeClass' : 'addClass'](element, 'ng-hide'); + // we're adding a temporary, animation-specific class for ng-hide since this way + // we can control when the element is actually displayed on screen without having + // to have a global/greedy CSS selector that breaks when other animations are run. + // Read: https://github.com/angular/angular.js/issues/9103#issuecomment-58335845 + $animate[value ? 'removeClass' : 'addClass'](element, NG_HIDE_CLASS, NG_HIDE_IN_PROGRESS_CLASS); }); } }; @@ -316,7 +322,9 @@ var ngHideDirective = ['$animate', function($animate) { multiElement: true, link: function(scope, element, attr) { scope.$watch(attr.ngHide, function ngHideWatchAction(value){ - $animate[value ? 'addClass' : 'removeClass'](element, 'ng-hide'); + // The comment inside of the ngShowDirective explains why we add and + // remove a temporary class for the show/hide animation + $animate[value ? 'addClass' : 'removeClass'](element,NG_HIDE_CLASS, NG_HIDE_IN_PROGRESS_CLASS); }); } }; diff --git a/test/ng/directive/ngShowHideSpec.js b/test/ng/directive/ngShowHideSpec.js index 7c73c0aa4195..5140df4fbef1 100644 --- a/test/ng/directive/ngShowHideSpec.js +++ b/test/ng/directive/ngShowHideSpec.js @@ -156,6 +156,28 @@ describe('ngShow / ngHide animations', function() { expect(item.element.text()).toBe('data'); expect(item.element).toBeHidden(); })); + + it('should apply the temporary `.ng-hide-animate` class to the element', + inject(function($compile, $rootScope, $animate) { + + var item; + var $scope = $rootScope.$new(); + $scope.on = false; + element = $compile(html( + '
data
' + ))($scope); + $scope.$digest(); + + item = $animate.queue.shift(); + expect(item.event).toEqual('addClass'); + expect(item.options).toEqual('ng-hide-animate'); + + $scope.on = true; + $scope.$digest(); + item = $animate.queue.shift(); + expect(item.event).toEqual('removeClass'); + expect(item.options).toEqual('ng-hide-animate'); + })); }); describe('ngHide', function() { @@ -181,5 +203,27 @@ describe('ngShow / ngHide animations', function() { expect(item.element.text()).toBe('datum'); expect(item.element).toBeShown(); })); + + it('should apply the temporary `.ng-hide-animate` class to the element', + inject(function($compile, $rootScope, $animate) { + + var item; + var $scope = $rootScope.$new(); + $scope.on = false; + element = $compile(html( + '
data
' + ))($scope); + $scope.$digest(); + + item = $animate.queue.shift(); + expect(item.event).toEqual('removeClass'); + expect(item.options).toEqual('ng-hide-animate'); + + $scope.on = true; + $scope.$digest(); + item = $animate.queue.shift(); + expect(item.event).toEqual('addClass'); + expect(item.options).toEqual('ng-hide-animate'); + })); }); });