refactor(collapse): use ngAnimate

- Animations are now opt-in, include ngAnimate to see collapse animations - ngAnimate handles initial state and doesn't animate if first reflow hasn't occurred. angular/angular.js@cc58460 - Tests may need more work. Right now they test for 'in' class. Fixes #1774 Fixes #2821 Fixes #2836 Closes #1274 Closes #1444
angular-ui · Mar 23, 2015 · 36e6363 · 36e6363
1 parent c5b63de
commit 36e6363
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 117 deletions.
diff --git a/src/accordion/accordion.js b/src/accordion/accordion.js
@@ -1,4 +1,4 @@
-angular.module('ui.bootstrap.accordion', [])
+angular.module('ui.bootstrap.accordion', ['ui.bootstrap.collapse'])
 
 .constant('accordionConfig', {
   closeOthers: true
@@ -129,61 +129,4 @@ angular.module('ui.bootstrap.accordion', [])
   };
 })
 
-/**
- * Animations based on addition and removal of `in` class
- * This requires the bootstrap classes to be present in order to take advantage
- * of the animation classes.
- */
-.animation('.panel-collapse', function () {
-  return {
-    beforeAddClass: function (element, className, done) {
-      if (className == 'in') {
-        element
-          .removeClass('collapse')
-          .addClass('collapsing')
-          ;
-      }
-      done();
-    },
-    addClass: function (element, className, done) {
-      if (className == 'in') {
-        element
-          .css({height: element[0].scrollHeight + 'px'})
-          .one('$animate:close', function closeFn() {
-            element
-              .removeClass('collapsing')
-              .css({height: 'auto'});
-          });
-      }
-      done();
-    },
-    beforeRemoveClass: function (element, className, done) {
-      if (className == 'in') {
-        element
-          // IMPORTANT: The height must be set before adding "collapsing" class.
-          // Otherwise, the browser attempts to animate from height 0 (in
-          // collapsing class) to the given height here.
-          .css({height: element[0].scrollHeight + 'px'})
-          // initially all panel collapse have the collapse class, this removal
-          // prevents the animation from jumping to collapsed state
-          .removeClass('collapse')
-          .addClass('collapsing');
-      }
-      done();
-    },
-    removeClass: function (element, className, done) {
-      if (className == 'in') {
-        element
-          .css({height: '0'})
-          .one('$animate:close', function closeFn() {
-            element
-              .removeClass('collapsing')
-              .addClass('collapse');
-          });
-      }
-      done();
-    }
-  };
-})
-
 ;
diff --git a/src/accordion/test/accordion.spec.js b/src/accordion/test/accordion.spec.js
@@ -2,6 +2,7 @@ describe('accordion', function () {
   var $scope;
 
   beforeEach(module('ui.bootstrap.accordion'));
+  beforeEach(module('ui.bootstrap.collapse'));
   beforeEach(module('template/accordion/accordion.html'));
   beforeEach(module('template/accordion/accordion-group.html'));
 

diff --git a/src/collapse/collapse.js b/src/collapse/collapse.js
@@ -1,67 +1,39 @@
-/**
- * @deprecated Switching over to using ngAnimate for animations
- */
 angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
 
-  .directive('collapse', ['$transition', function ($transition) {
+  .directive('collapse', ['$animate', function ($animate) {
 
     return {
       link: function (scope, element, attrs) {
-
-        var initialAnimSkip = true;
-        var currentTransition;
-
-        function doTransition(change) {
-          var newTransition = $transition(element, change);
-          if (currentTransition) {
-            currentTransition.cancel();
-          }
-          currentTransition = newTransition;
-          newTransition.then(newTransitionDone, newTransitionDone);
-          return newTransition;
-
-          function newTransitionDone() {
-            // Make sure it's this transition, otherwise, leave it alone.
-            if (currentTransition === newTransition) {
-              currentTransition = undefined;
-            }
-          }
-        }
-
         function expand() {
-          if (initialAnimSkip) {
-            initialAnimSkip = false;
-            expandDone();
-          } else {
-            element.removeClass('collapse').addClass('collapsing');
-            doTransition({ height: element[0].scrollHeight + 'px' }).then(expandDone);
-          }
+          element.removeClass('collapse').addClass('collapsing');
+          $animate.addClass(element, 'in', {
+            to: { height: element[0].scrollHeight + 'px' }
+          }).then(expandDone);
         }
 
         function expandDone() {
           element.removeClass('collapsing');
-          element.addClass('collapse in');
           element.css({height: 'auto'});
         }
 
         function collapse() {
-          if (initialAnimSkip) {
-            initialAnimSkip = false;
-            collapseDone();
-            element.css({height: 0});
-          } else {
-            // CSS transitions don't work with height: auto, so we have to manually change the height to a specific value
-            element.css({ height: element[0].scrollHeight + 'px' });
-            //trigger reflow so a browser realizes that height was updated from auto to a specific value
-            var x = element[0].offsetWidth;
-
-            element.removeClass('collapse in').addClass('collapsing');
-
-            doTransition({ height: 0 }).then(collapseDone);
-          }
+          element
+            // IMPORTANT: The height must be set before adding "collapsing" class.
+            // Otherwise, the browser attempts to animate from height 0 (in
+            // collapsing class) to the given height here.
+            .css({height: element[0].scrollHeight + 'px'})
+            // initially all panel collapse have the collapse class, this removal
+            // prevents the animation from jumping to collapsed state
+            .removeClass('collapse')
+            .addClass('collapsing');
+
+          $animate.removeClass(element, 'in', {
+            to: {height: '0'}
+          }).then(collapseDone);
         }
 
         function collapseDone() {
+          element.css({height: '0'}); // Required so that collapse works when animation is disabled
           element.removeClass('collapsing');
           element.addClass('collapse');
         }

diff --git a/src/collapse/test/collapse.spec.js b/src/collapse/test/collapse.spec.js
@@ -1,14 +1,14 @@
 describe('collapse directive', function () {
 
-  var scope, $compile, $timeout, $transition;
+  var scope, $compile, $animate;
   var element;
 
   beforeEach(module('ui.bootstrap.collapse'));
-  beforeEach(inject(function(_$rootScope_, _$compile_, _$timeout_, _$transition_) {
+  beforeEach(module('ngAnimateMock'));
+  beforeEach(inject(function(_$rootScope_, _$compile_, _$animate_) {
     scope = _$rootScope_;
     $compile = _$compile_;
-    $timeout = _$timeout_;
-    $transition = _$transition_;
+    $animate = _$animate_;
   }));
 
   beforeEach(function() {
@@ -23,6 +23,7 @@ describe('collapse directive', function () {
   it('should be hidden on initialization if isCollapsed = true without transition', function() {
     scope.isCollapsed = true;
     scope.$digest();
+    $animate.triggerCallbacks();
     //No animation timeout here
     expect(element.height()).toBe(0);
   });
@@ -32,7 +33,7 @@ describe('collapse directive', function () {
     scope.$digest();
     scope.isCollapsed = true;
     scope.$digest();
-    $timeout.flush();
+    $animate.triggerCallbacks();
     expect(element.height()).toBe(0);
   });
 
@@ -50,7 +51,7 @@ describe('collapse directive', function () {
     scope.$digest();
     scope.isCollapsed = false;
     scope.$digest();
-    $timeout.flush();
+    $animate.triggerCallbacks();
     expect(element.height()).not.toBe(0);
   });
 
@@ -63,12 +64,10 @@ describe('collapse directive', function () {
     scope.$digest();
     scope.isCollapsed = true;
     scope.$digest();
-    $timeout.flush();
+    $animate.triggerCallbacks();
+    expect(element.height()).toBe(0);
+    $animate.triggerCallbacks();
     expect(element.height()).toBe(0);
-    if ($transition.transitionEndEventName) {
-      element.triggerHandler($transition.transitionEndEventName);
-      expect(element.height()).toBe(0);
-    }
   });
 
   describe('dynamic content', function() {
@@ -89,6 +88,7 @@ describe('collapse directive', function () {
       scope.exp = false;
       scope.isCollapsed = false;
       scope.$digest();
+      $animate.triggerCallbacks();
       var collapseHeight = element.height();
       scope.exp = true;
       scope.$digest();
@@ -99,6 +99,7 @@ describe('collapse directive', function () {
       scope.exp = true;
       scope.isCollapsed = false;
       scope.$digest();
+      $animate.triggerCallbacks();
       var collapseHeight = element.height();
       scope.exp = false;
       scope.$digest();

diff --git a/template/accordion/accordion-group.html b/template/accordion/accordion-group.html
@@ -4,7 +4,7 @@ <h4 class="panel-title">
       <a href="javascript:void(0)" tabindex="0" class="accordion-toggle" ng-click="toggleOpen()" accordion-transclude="heading"><span ng-class="{'text-muted': isDisabled}">{{heading}}</span></a>
     </h4>
   </div>
-  <div class="panel-collapse collapse" ng-class="{in: isOpen}">
+  <div class="panel-collapse collapse" collapse="!isOpen">
 	  <div class="panel-body" ng-transclude></div>
   </div>
 </div>