From fe61f77971fe073f6979cba572f057b21a7e4874 Mon Sep 17 00:00:00 2001
From: Chris Chua <chris@thousandeyes.com>
Date: Sun, 24 Nov 2013 16:49:53 -0800
Subject: [PATCH 1/2] fix(collapse): Prevent consecutive transitions

Previously, there would be a case where the two transitions would run
as the cancel would asynchronously invoke the reject handler of the
transition which would set the currTransition to undefined even when
the currTransition has been replaced with a new transition.
---
 src/collapse/collapse.js           | 17 +++++++++++------
 src/collapse/test/collapse.spec.js | 17 +++++++++++++++++
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/collapse/collapse.js b/src/collapse/collapse.js
index ece61727c4..cd4b5c4ee7 100644
--- a/src/collapse/collapse.js
+++ b/src/collapse/collapse.js
@@ -8,16 +8,21 @@ angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
         var initialAnimSkip = true;
         var currentTransition;
 
-        function resetCurrentTransition() {
-          currentTransition = undefined;
-        }
-
         function doTransition(change) {
+          var newTransition = $transition(element, change);
           if (currentTransition) {
             currentTransition.cancel();
           }
-          (currentTransition = $transition(element, change)).then(resetCurrentTransition, resetCurrentTransition);
-          return currentTransition;
+          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() {
diff --git a/src/collapse/test/collapse.spec.js b/src/collapse/test/collapse.spec.js
index d3784ad4c6..f2069b1135 100644
--- a/src/collapse/test/collapse.spec.js
+++ b/src/collapse/test/collapse.spec.js
@@ -55,6 +55,23 @@ describe('collapse directive', function () {
     expect(element.height()).not.toBe(0);
   });
 
+  it('should expand if isCollapsed = true with animation on subsequent uses', function() {
+    scope.isCollapsed = false;
+    scope.$digest();
+    scope.isCollapsed = true;
+    scope.$digest();
+    scope.isCollapsed = false;
+    scope.$digest();
+    scope.isCollapsed = true;
+    scope.$digest();
+    $timeout.flush();
+    expect(element.height()).toBe(0);
+    if ($transition.transitionEndEventName) {
+      element.triggerHandler($transition.transitionEndEventName);
+      expect(element.height()).toBe(0);
+    }
+  });
+
   describe('dynamic content', function() {
     beforeEach(function() {
       element = angular.element('<div collapse="isCollapsed"><p>Initial content</p><div ng-show="exp">Additional content</div></div>');

From 2cc893129bfd68df93929b98ccb3259ac34bf43e Mon Sep 17 00:00:00 2001
From: Chris Chua <chris@thousandeyes.com>
Date: Sun, 24 Nov 2013 16:51:47 -0800
Subject: [PATCH 2/2] chore(collapse): Tidy up code duplication

---
 src/collapse/collapse.js | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/src/collapse/collapse.js b/src/collapse/collapse.js
index cd4b5c4ee7..8ce9955cf4 100644
--- a/src/collapse/collapse.js
+++ b/src/collapse/collapse.js
@@ -28,24 +28,23 @@ angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
         function expand() {
           if (initialAnimSkip) {
             initialAnimSkip = false;
-            element.removeClass('collapsing');
-            element.addClass('collapse in');
-            element.css({height: 'auto'});
+            expandDone();
           } else {
             element.removeClass('collapse').addClass('collapsing');
-            doTransition({ height: element[0].scrollHeight + 'px' }).then(function () {
-              element.removeClass('collapsing');
-              element.addClass('collapse in');
-              element.css({height: 'auto'});
-            });
+            doTransition({ 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;
-            element.removeClass('collapsing');
-            element.addClass('collapse');
+            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
@@ -55,13 +54,15 @@ angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
 
             element.removeClass('collapse in').addClass('collapsing');
 
-            doTransition({ height: 0 }).then(function () {
-              element.removeClass('collapsing');
-              element.addClass('collapse');
-            });
+            doTransition({ height: 0 }).then(collapseDone);
           }
         }
 
+        function collapseDone() {
+          element.removeClass('collapsing');
+          element.addClass('collapse');
+        }
+
         scope.$watch(attrs.collapse, function (shouldCollapse) {
           if (shouldCollapse) {
             collapse();