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

Commit 632eae4

Browse files
committed
fix(carousel): replaced $timeout with $interval when it was wrong
I had a to tweak a bit two tests since $interval never throws an exception after flushing, hope I did it OK. Ammend: addresses @christ critics
1 parent 4fc68b8 commit 632eae4

File tree

2 files changed

+31
-28
lines changed

2 files changed

+31
-28
lines changed

Diff for: src/carousel/carousel.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
*
88
*/
99
angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition'])
10-
.controller('CarouselController', ['$scope', '$timeout', '$transition', function ($scope, $timeout, $transition) {
10+
.controller('CarouselController', ['$scope', '$timeout', '$interval', '$transition', function ($scope, $timeout, $interval, $transition) {
1111
var self = this,
1212
slides = self.slides = $scope.slides = [],
1313
currentIndex = -1,
14-
currentTimeout, isPlaying;
14+
currentInterval, isPlaying;
1515
self.currentSlide = null;
1616

1717
var destroyed = false;
@@ -106,22 +106,22 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition'])
106106
function restartTimer() {
107107
resetTimer();
108108
var interval = +$scope.interval;
109-
if (!isNaN(interval) && interval>=0) {
110-
currentTimeout = $timeout(timerFn, interval);
109+
if (isPlaying && !isNaN(interval) && interval>=0) {
110+
currentInterval = $interval(timerFn, interval);
111111
}
112112
}
113113

114114
function resetTimer() {
115-
if (currentTimeout) {
116-
$timeout.cancel(currentTimeout);
117-
currentTimeout = null;
115+
if (currentInterval) {
116+
$interval.cancel(currentInterval);
117+
currentInterval = null;
118118
}
119119
}
120120

121121
function timerFn() {
122-
if (isPlaying) {
122+
var interval = +$scope.interval;
123+
if (isPlaying && !isNaN(interval) && interval>=0) {
123124
$scope.next();
124-
restartTimer();
125125
} else {
126126
$scope.pause();
127127
}

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

+22-19
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@ describe('carousel', function() {
1414
}));
1515
beforeEach(module('template/carousel/carousel.html', 'template/carousel/slide.html'));
1616

17-
var $rootScope, $compile, $controller, $timeout;
18-
beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$timeout_) {
17+
var $rootScope, $compile, $controller, $timeout, $interval;
18+
beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$timeout_, _$interval_) {
1919
$rootScope = _$rootScope_;
2020
$compile = _$compile_;
2121
$controller = _$controller_;
2222
$timeout = _$timeout_;
23+
$interval = _$interval_;
2324
}));
2425

2526
describe('basics', function() {
@@ -152,12 +153,12 @@ describe('carousel', function() {
152153
//no timeout to flush, interval watch doesn't make a new one when interval is invalid
153154
testSlideActive(0);
154155
scope.$apply('interval = 1000');
155-
$timeout.flush();
156+
$interval.flush(scope.interval);
156157
testSlideActive(1);
157158
scope.$apply('interval = false');
158159
testSlideActive(1);
159160
scope.$apply('interval = 1000');
160-
$timeout.flush();
161+
$interval.flush(scope.interval);
161162
testSlideActive(2);
162163
});
163164

@@ -182,34 +183,35 @@ describe('carousel', function() {
182183

183184
it('should be playing by default and cycle through slides', function() {
184185
testSlideActive(0);
185-
$timeout.flush();
186+
$interval.flush(scope.interval);
186187
testSlideActive(1);
187-
$timeout.flush();
188+
$interval.flush(scope.interval);
188189
testSlideActive(2);
189-
$timeout.flush();
190+
$interval.flush(scope.interval);
190191
testSlideActive(0);
191192
});
192193

193194
it('should pause and play on mouseover', function() {
194195
testSlideActive(0);
195-
$timeout.flush();
196+
$interval.flush(scope.interval);
196197
testSlideActive(1);
197198
elm.trigger('mouseenter');
198-
expect($timeout.flush).toThrow();//pause should cancel current timeout
199+
testSlideActive(1);
200+
$interval.flush(scope.interval);
199201
testSlideActive(1);
200202
elm.trigger('mouseleave');
201-
$timeout.flush();
203+
$interval.flush(scope.interval);
202204
testSlideActive(2);
203205
});
204206

205207
it('should not pause on mouseover if noPause', function() {
206208
scope.$apply('nopause = true');
207209
testSlideActive(0);
208210
elm.trigger('mouseenter');
209-
$timeout.flush();
211+
$interval.flush(scope.interval);
210212
testSlideActive(1);
211213
elm.trigger('mouseleave');
212-
$timeout.flush();
214+
$interval.flush(scope.interval);
213215
testSlideActive(2);
214216
});
215217

@@ -219,7 +221,7 @@ describe('carousel', function() {
219221
scope.$apply('slides.splice(0,1)');
220222
expect(elm.find('div.item').length).toBe(2);
221223
testSlideActive(1);
222-
$timeout.flush();
224+
$interval.flush(scope.interval);
223225
testSlideActive(0);
224226
scope.$apply('slides.splice(1,1)');
225227
expect(elm.find('div.item').length).toBe(1);
@@ -253,14 +255,15 @@ describe('carousel', function() {
253255

254256
it('issue 1414 - should not continue running timers after scope is destroyed', function() {
255257
testSlideActive(0);
256-
$timeout.flush();
258+
$interval.flush(scope.interval);
257259
testSlideActive(1);
258-
$timeout.flush();
260+
$interval.flush(scope.interval);
259261
testSlideActive(2);
260-
$timeout.flush();
262+
$interval.flush(scope.interval);
261263
testSlideActive(0);
264+
spyOn($interval, 'cancel');
262265
scope.$destroy();
263-
expect($timeout.flush).toThrow('No deferred tasks to be flushed');
266+
expect($interval.cancel).toHaveBeenCalled();
264267
});
265268

266269
});
@@ -326,12 +329,12 @@ describe('carousel', function() {
326329
scope.interval = 2000;
327330
scope.$digest();
328331

329-
$timeout.flush();
332+
$interval.flush(scope.interval);
330333
expect(scope.next.calls.length).toBe(1);
331334

332335
scope.$destroy();
333336

334-
$timeout.flush(scope.interval);
337+
$interval.flush(scope.interval);
335338
expect(scope.next.calls.length).toBe(1);
336339
});
337340
});

0 commit comments

Comments
 (0)