Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Commit 5a36e28

Browse files
chrisirhcpkozlowski-opensource
authored andcommitted
fix(collapse): Prevent consecutive transitions & tidy up code
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.
1 parent cadc0f9 commit 5a36e28

File tree

2 files changed

+43
-20
lines changed

2 files changed

+43
-20
lines changed

src/collapse/collapse.js

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,39 +8,43 @@ angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
88
var initialAnimSkip = true;
99
var currentTransition;
1010

11-
function resetCurrentTransition() {
12-
currentTransition = undefined;
13-
}
14-
1511
function doTransition(change) {
12+
var newTransition = $transition(element, change);
1613
if (currentTransition) {
1714
currentTransition.cancel();
1815
}
19-
(currentTransition = $transition(element, change)).then(resetCurrentTransition, resetCurrentTransition);
20-
return currentTransition;
16+
currentTransition = newTransition;
17+
newTransition.then(newTransitionDone, newTransitionDone);
18+
return newTransition;
19+
20+
function newTransitionDone() {
21+
// Make sure it's this transition, otherwise, leave it alone.
22+
if (currentTransition === newTransition) {
23+
currentTransition = undefined;
24+
}
25+
}
2126
}
2227

2328
function expand() {
2429
if (initialAnimSkip) {
2530
initialAnimSkip = false;
26-
element.removeClass('collapsing');
27-
element.addClass('collapse in');
28-
element.css({height: 'auto'});
31+
expandDone();
2932
} else {
3033
element.removeClass('collapse').addClass('collapsing');
31-
doTransition({ height: element[0].scrollHeight + 'px' }).then(function () {
32-
element.removeClass('collapsing');
33-
element.addClass('collapse in');
34-
element.css({height: 'auto'});
35-
});
34+
doTransition({ height: element[0].scrollHeight + 'px' }).then(expandDone);
3635
}
3736
}
3837

38+
function expandDone() {
39+
element.removeClass('collapsing');
40+
element.addClass('collapse in');
41+
element.css({height: 'auto'});
42+
}
43+
3944
function collapse() {
4045
if (initialAnimSkip) {
4146
initialAnimSkip = false;
42-
element.removeClass('collapsing');
43-
element.addClass('collapse');
47+
collapseDone();
4448
element.css({height: 0});
4549
} else {
4650
// CSS transitions don't work with height: auto, so we have to manually change the height to a specific value
@@ -50,13 +54,15 @@ angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
5054

5155
element.removeClass('collapse in').addClass('collapsing');
5256

53-
doTransition({ height: 0 }).then(function () {
54-
element.removeClass('collapsing');
55-
element.addClass('collapse');
56-
});
57+
doTransition({ height: 0 }).then(collapseDone);
5758
}
5859
}
5960

61+
function collapseDone() {
62+
element.removeClass('collapsing');
63+
element.addClass('collapse');
64+
}
65+
6066
scope.$watch(attrs.collapse, function (shouldCollapse) {
6167
if (shouldCollapse) {
6268
collapse();

src/collapse/test/collapse.spec.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,23 @@ describe('collapse directive', function () {
5555
expect(element.height()).not.toBe(0);
5656
});
5757

58+
it('should expand if isCollapsed = true with animation on subsequent uses', function() {
59+
scope.isCollapsed = false;
60+
scope.$digest();
61+
scope.isCollapsed = true;
62+
scope.$digest();
63+
scope.isCollapsed = false;
64+
scope.$digest();
65+
scope.isCollapsed = true;
66+
scope.$digest();
67+
$timeout.flush();
68+
expect(element.height()).toBe(0);
69+
if ($transition.transitionEndEventName) {
70+
element.triggerHandler($transition.transitionEndEventName);
71+
expect(element.height()).toBe(0);
72+
}
73+
});
74+
5875
describe('dynamic content', function() {
5976
beforeEach(function() {
6077
element = angular.element('<div collapse="isCollapsed"><p>Initial content</p><div ng-show="exp">Additional content</div></div>');

0 commit comments

Comments
 (0)