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

Commit 9b06c30

Browse files
SidhNorpkozlowski-opensource
authored andcommitted
fix(collapsible): fix transition and height issues
Closes #82 #94
1 parent 4629071 commit 9b06c30

File tree

2 files changed

+60
-11
lines changed

2 files changed

+60
-11
lines changed

Diff for: src/collapse/collapse.js

+33-8
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,20 @@ angular.module('ui.bootstrap.collapse',['ui.bootstrap.transition'])
2121
link: function(scope, element, attrs) {
2222

2323
var isCollapsed;
24-
24+
var initialAnimSkip = true;
25+
scope.$watch(function (){ return element[0].scrollHeight; }, function (value) {
26+
//The listener is called when scollHeight changes
27+
//It actually does on 2 scenarios:
28+
// 1. Parent is set to display none
29+
// 2. angular bindings inside are resolved
30+
//When we have a change of scrollHeight we are setting again the correct height if the group is opened
31+
if (element[0].scrollHeight !== 0) {
32+
if (!isCollapsed) {
33+
fixUpHeight(scope, element, element[0].scrollHeight + 'px');
34+
}
35+
}
36+
});
37+
2538
scope.$watch(attrs.collapse, function(value) {
2639
if (value) {
2740
collapse();
@@ -45,21 +58,33 @@ angular.module('ui.bootstrap.collapse',['ui.bootstrap.transition'])
4558
};
4659

4760
var expand = function() {
48-
doTransition({ height : element[0].scrollHeight + 'px' })
49-
.then(function() {
50-
// This check ensures that we don't accidentally update the height if the user has closed
51-
// the group while the animation was still running
61+
if (initialAnimSkip) {
62+
initialAnimSkip = false;
5263
if ( !isCollapsed ) {
5364
fixUpHeight(scope, element, 'auto');
5465
}
55-
});
66+
} else {
67+
doTransition({ height : element[0].scrollHeight + 'px' })
68+
.then(function() {
69+
// This check ensures that we don't accidentally update the height if the user has closed
70+
// the group while the animation was still running
71+
if ( !isCollapsed ) {
72+
fixUpHeight(scope, element, 'auto');
73+
}
74+
});
75+
}
5676
isCollapsed = false;
5777
};
5878

5979
var collapse = function() {
6080
isCollapsed = true;
61-
fixUpHeight(scope, element, element[0].scrollHeight + 'px');
62-
doTransition({'height':'0'});
81+
if (initialAnimSkip) {
82+
initialAnimSkip = false;
83+
fixUpHeight(scope, element, 0);
84+
} else {
85+
fixUpHeight(scope, element, element[0].scrollHeight + 'px');
86+
doTransition({'height':'0'});
87+
}
6388
};
6489
}
6590
};

Diff for: src/collapse/test/collapse.spec.js

+27-3
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,41 @@ describe('collapse directive', function () {
1717
angular.element(document.body).append(element);
1818
});
1919

20-
it('should collapse if isCollapsed = true', function() {
20+
afterEach(function() {
21+
element.remove();
22+
});
23+
24+
it('should be hidden on initialization if isCollapsed = true without transition', function() {
25+
scope.isCollapsed = true;
26+
scope.$digest();
27+
//No animation timeout here
28+
expect(element.height()).toBe(0);
29+
});
30+
31+
it('should collapse if isCollapsed = true with animation on subsequent use', function() {
32+
scope.isCollapsed = false;
33+
scope.$digest();
2134
scope.isCollapsed = true;
2235
scope.$digest();
2336
$timeout.flush();
2437
expect(element.height()).toBe(0);
2538
});
2639

27-
it('should expand if isCollapsed = false', function() {
40+
it('should be shown on initialization if isCollapsed = false without transition', function() {
41+
scope.isCollapsed = false;
42+
scope.$digest();
43+
//No animation timeout here
44+
expect(element.height()).not.toBe(0);
45+
});
46+
47+
it('should expand if isCollapsed = false with animation on subsequent use', function() {
48+
scope.isCollapsed = false;
49+
scope.$digest();
50+
scope.isCollapsed = true;
51+
scope.$digest();
2852
scope.isCollapsed = false;
2953
scope.$digest();
3054
$timeout.flush();
3155
expect(element.height()).not.toBe(0);
32-
});
56+
});
3357
});

0 commit comments

Comments
 (0)