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

Commit 683e488

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 683e488

File tree

2 files changed

+31
-29
lines changed

2 files changed

+31
-29
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 (!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-20
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ 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, $interval;
18+
beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$interval_) {
1919
$rootScope = _$rootScope_;
2020
$compile = _$compile_;
2121
$controller = _$controller_;
22-
$timeout = _$timeout_;
22+
$interval = _$interval_;
2323
}));
2424

2525
describe('basics', function() {
@@ -152,12 +152,12 @@ describe('carousel', function() {
152152
//no timeout to flush, interval watch doesn't make a new one when interval is invalid
153153
testSlideActive(0);
154154
scope.$apply('interval = 1000');
155-
$timeout.flush();
155+
$interval.flush(scope.interval);
156156
testSlideActive(1);
157157
scope.$apply('interval = false');
158158
testSlideActive(1);
159159
scope.$apply('interval = 1000');
160-
$timeout.flush();
160+
$interval.flush(scope.interval);
161161
testSlideActive(2);
162162
});
163163

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

183183
it('should be playing by default and cycle through slides', function() {
184184
testSlideActive(0);
185-
$timeout.flush();
185+
$interval.flush(scope.interval);
186186
testSlideActive(1);
187-
$timeout.flush();
187+
$interval.flush(scope.interval);
188188
testSlideActive(2);
189-
$timeout.flush();
189+
$interval.flush(scope.interval);
190190
testSlideActive(0);
191191
});
192192

193193
it('should pause and play on mouseover', function() {
194194
testSlideActive(0);
195-
$timeout.flush();
195+
$interval.flush(scope.interval);
196196
testSlideActive(1);
197197
elm.trigger('mouseenter');
198-
expect($timeout.flush).toThrow();//pause should cancel current timeout
198+
testSlideActive(1);
199+
$interval.flush(scope.interval);
199200
testSlideActive(1);
200201
elm.trigger('mouseleave');
201-
$timeout.flush();
202+
$interval.flush(scope.interval);
202203
testSlideActive(2);
203204
});
204205

205206
it('should not pause on mouseover if noPause', function() {
206207
scope.$apply('nopause = true');
207208
testSlideActive(0);
208209
elm.trigger('mouseenter');
209-
$timeout.flush();
210+
$interval.flush(scope.interval);
210211
testSlideActive(1);
211212
elm.trigger('mouseleave');
212-
$timeout.flush();
213+
$interval.flush(scope.interval);
213214
testSlideActive(2);
214215
});
215216

@@ -219,7 +220,7 @@ describe('carousel', function() {
219220
scope.$apply('slides.splice(0,1)');
220221
expect(elm.find('div.item').length).toBe(2);
221222
testSlideActive(1);
222-
$timeout.flush();
223+
$interval.flush(scope.interval);
223224
testSlideActive(0);
224225
scope.$apply('slides.splice(1,1)');
225226
expect(elm.find('div.item').length).toBe(1);
@@ -253,14 +254,15 @@ describe('carousel', function() {
253254

254255
it('issue 1414 - should not continue running timers after scope is destroyed', function() {
255256
testSlideActive(0);
256-
$timeout.flush();
257+
$interval.flush(scope.interval);
257258
testSlideActive(1);
258-
$timeout.flush();
259+
$interval.flush(scope.interval);
259260
testSlideActive(2);
260-
$timeout.flush();
261+
$interval.flush(scope.interval);
261262
testSlideActive(0);
263+
spyOn($interval, 'cancel');
262264
scope.$destroy();
263-
expect($timeout.flush).toThrow('No deferred tasks to be flushed');
265+
expect($interval.cancel).toHaveBeenCalled();
264266
});
265267

266268
});
@@ -326,12 +328,12 @@ describe('carousel', function() {
326328
scope.interval = 2000;
327329
scope.$digest();
328330

329-
$timeout.flush();
331+
$interval.flush(scope.interval);
330332
expect(scope.next.calls.length).toBe(1);
331333

332334
scope.$destroy();
333335

334-
$timeout.flush(scope.interval);
336+
$interval.flush(scope.interval);
335337
expect(scope.next.calls.length).toBe(1);
336338
});
337339
});

0 commit comments

Comments
 (0)