fix($animate): ensure hidden elements with ngShow/ngHide stay hidden during animations

matsko · IgorMinar · commit 39d0b36826a0 · 2014-10-08T13:48:25.000-07:00
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 angular#9103 Closes angular#9493
diff --git 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
@@ -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
@@ -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(
+        '<div class="show-hide" ng-show="on">data</div>'
+      ))($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(
+        '<div class="show-hide" ng-hide="on">data</div>'
+      ))($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');
+    }));
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`
`3`	`3`	`[ng\:cloak], [ng-cloak], [data-ng-cloak], [x-ng-cloak],`
`4`	`4`	`.ng-cloak, .x-ng-cloak,`
`5`		`-.ng-hide:not(.ng-animate) {`
	`5`	`+.ng-hide:not(.ng-hide-animate) {`
`6`	`6`	`display: none !important;`
`7`	`7`	`}`
`8`	`8`