Skip to content

Commit 5750a42

Browse files
chrisirhcayakovlevgh
authored andcommitted
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 angular-ui#1774 Fixes angular-ui#2821 Fixes angular-ui#2836 Closes angular-ui#1274 Closes angular-ui#1444
1 parent 0db03a1 commit 5750a42

File tree

5 files changed

+34
-117
lines changed

5 files changed

+34
-117
lines changed

src/accordion/accordion.js

Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
angular.module('ui.bootstrap.accordion', [])
1+
angular.module('ui.bootstrap.accordion', ['ui.bootstrap.collapse'])
22

33
.constant('accordionConfig', {
44
closeOthers: true
@@ -129,61 +129,4 @@ angular.module('ui.bootstrap.accordion', [])
129129
};
130130
})
131131

132-
/**
133-
* Animations based on addition and removal of `in` class
134-
* This requires the bootstrap classes to be present in order to take advantage
135-
* of the animation classes.
136-
*/
137-
.animation('.panel-collapse', function () {
138-
return {
139-
beforeAddClass: function (element, className, done) {
140-
if (className == 'in') {
141-
element
142-
.removeClass('collapse')
143-
.addClass('collapsing')
144-
;
145-
}
146-
done();
147-
},
148-
addClass: function (element, className, done) {
149-
if (className == 'in') {
150-
element
151-
.css({height: element[0].scrollHeight + 'px'})
152-
.one('$animate:close', function closeFn() {
153-
element
154-
.removeClass('collapsing')
155-
.css({height: 'auto'});
156-
});
157-
}
158-
done();
159-
},
160-
beforeRemoveClass: function (element, className, done) {
161-
if (className == 'in') {
162-
element
163-
// IMPORTANT: The height must be set before adding "collapsing" class.
164-
// Otherwise, the browser attempts to animate from height 0 (in
165-
// collapsing class) to the given height here.
166-
.css({height: element[0].scrollHeight + 'px'})
167-
// initially all panel collapse have the collapse class, this removal
168-
// prevents the animation from jumping to collapsed state
169-
.removeClass('collapse')
170-
.addClass('collapsing');
171-
}
172-
done();
173-
},
174-
removeClass: function (element, className, done) {
175-
if (className == 'in') {
176-
element
177-
.css({height: '0'})
178-
.one('$animate:close', function closeFn() {
179-
element
180-
.removeClass('collapsing')
181-
.addClass('collapse');
182-
});
183-
}
184-
done();
185-
}
186-
};
187-
})
188-
189132
;

src/accordion/test/accordion.spec.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ describe('accordion', function () {
22
var $scope;
33

44
beforeEach(module('ui.bootstrap.accordion'));
5+
beforeEach(module('ui.bootstrap.collapse'));
56
beforeEach(module('template/accordion/accordion.html'));
67
beforeEach(module('template/accordion/accordion-group.html'));
78

src/collapse/collapse.js

Lines changed: 19 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,39 @@
1-
/**
2-
* @deprecated Switching over to using ngAnimate for animations
3-
*/
41
angular.module('ui.bootstrap.collapse', ['ui.bootstrap.transition'])
52

6-
.directive('collapse', ['$transition', function ($transition) {
3+
.directive('collapse', ['$animate', function ($animate) {
74

85
return {
96
link: function (scope, element, attrs) {
10-
11-
var initialAnimSkip = true;
12-
var currentTransition;
13-
14-
function doTransition(change) {
15-
var newTransition = $transition(element, change);
16-
if (currentTransition) {
17-
currentTransition.cancel();
18-
}
19-
currentTransition = newTransition;
20-
newTransition.then(newTransitionDone, newTransitionDone);
21-
return newTransition;
22-
23-
function newTransitionDone() {
24-
// Make sure it's this transition, otherwise, leave it alone.
25-
if (currentTransition === newTransition) {
26-
currentTransition = undefined;
27-
}
28-
}
29-
}
30-
317
function expand() {
32-
if (initialAnimSkip) {
33-
initialAnimSkip = false;
34-
expandDone();
35-
} else {
36-
element.removeClass('collapse').addClass('collapsing');
37-
doTransition({ height: element[0].scrollHeight + 'px' }).then(expandDone);
38-
}
8+
element.removeClass('collapse').addClass('collapsing');
9+
$animate.addClass(element, 'in', {
10+
to: { height: element[0].scrollHeight + 'px' }
11+
}).then(expandDone);
3912
}
4013

4114
function expandDone() {
4215
element.removeClass('collapsing');
43-
element.addClass('collapse in');
4416
element.css({height: 'auto'});
4517
}
4618

4719
function collapse() {
48-
if (initialAnimSkip) {
49-
initialAnimSkip = false;
50-
collapseDone();
51-
element.css({height: 0});
52-
} else {
53-
// CSS transitions don't work with height: auto, so we have to manually change the height to a specific value
54-
element.css({ height: element[0].scrollHeight + 'px' });
55-
//trigger reflow so a browser realizes that height was updated from auto to a specific value
56-
var x = element[0].offsetWidth;
57-
58-
element.removeClass('collapse in').addClass('collapsing');
59-
60-
doTransition({ height: 0 }).then(collapseDone);
61-
}
20+
element
21+
// IMPORTANT: The height must be set before adding "collapsing" class.
22+
// Otherwise, the browser attempts to animate from height 0 (in
23+
// collapsing class) to the given height here.
24+
.css({height: element[0].scrollHeight + 'px'})
25+
// initially all panel collapse have the collapse class, this removal
26+
// prevents the animation from jumping to collapsed state
27+
.removeClass('collapse')
28+
.addClass('collapsing');
29+
30+
$animate.removeClass(element, 'in', {
31+
to: {height: '0'}
32+
}).then(collapseDone);
6233
}
6334

6435
function collapseDone() {
36+
element.css({height: '0'}); // Required so that collapse works when animation is disabled
6537
element.removeClass('collapsing');
6638
element.addClass('collapse');
6739
}

src/collapse/test/collapse.spec.js

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
describe('collapse directive', function () {
22

3-
var scope, $compile, $timeout, $transition;
3+
var scope, $compile, $animate;
44
var element;
55

66
beforeEach(module('ui.bootstrap.collapse'));
7-
beforeEach(inject(function(_$rootScope_, _$compile_, _$timeout_, _$transition_) {
7+
beforeEach(module('ngAnimateMock'));
8+
beforeEach(inject(function(_$rootScope_, _$compile_, _$animate_) {
89
scope = _$rootScope_;
910
$compile = _$compile_;
10-
$timeout = _$timeout_;
11-
$transition = _$transition_;
11+
$animate = _$animate_;
1212
}));
1313

1414
beforeEach(function() {
@@ -23,6 +23,7 @@ describe('collapse directive', function () {
2323
it('should be hidden on initialization if isCollapsed = true without transition', function() {
2424
scope.isCollapsed = true;
2525
scope.$digest();
26+
$animate.triggerCallbacks();
2627
//No animation timeout here
2728
expect(element.height()).toBe(0);
2829
});
@@ -32,7 +33,7 @@ describe('collapse directive', function () {
3233
scope.$digest();
3334
scope.isCollapsed = true;
3435
scope.$digest();
35-
$timeout.flush();
36+
$animate.triggerCallbacks();
3637
expect(element.height()).toBe(0);
3738
});
3839

@@ -50,7 +51,7 @@ describe('collapse directive', function () {
5051
scope.$digest();
5152
scope.isCollapsed = false;
5253
scope.$digest();
53-
$timeout.flush();
54+
$animate.triggerCallbacks();
5455
expect(element.height()).not.toBe(0);
5556
});
5657

@@ -63,12 +64,10 @@ describe('collapse directive', function () {
6364
scope.$digest();
6465
scope.isCollapsed = true;
6566
scope.$digest();
66-
$timeout.flush();
67+
$animate.triggerCallbacks();
68+
expect(element.height()).toBe(0);
69+
$animate.triggerCallbacks();
6770
expect(element.height()).toBe(0);
68-
if ($transition.transitionEndEventName) {
69-
element.triggerHandler($transition.transitionEndEventName);
70-
expect(element.height()).toBe(0);
71-
}
7271
});
7372

7473
describe('dynamic content', function() {
@@ -89,6 +88,7 @@ describe('collapse directive', function () {
8988
scope.exp = false;
9089
scope.isCollapsed = false;
9190
scope.$digest();
91+
$animate.triggerCallbacks();
9292
var collapseHeight = element.height();
9393
scope.exp = true;
9494
scope.$digest();
@@ -99,6 +99,7 @@ describe('collapse directive', function () {
9999
scope.exp = true;
100100
scope.isCollapsed = false;
101101
scope.$digest();
102+
$animate.triggerCallbacks();
102103
var collapseHeight = element.height();
103104
scope.exp = false;
104105
scope.$digest();

template/accordion/accordion-group.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ <h4 class="panel-title">
44
<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>
55
</h4>
66
</div>
7-
<div class="panel-collapse collapse" ng-class="{in: isOpen}">
7+
<div class="panel-collapse collapse" collapse="!isOpen">
88
<div class="panel-body" ng-transclude></div>
99
</div>
1010
</div>

0 commit comments

Comments
 (0)