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

fix(carousel): resolve rendering issues #4984

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 39 additions & 10 deletions src/carousel/carousel.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
angular.module('ui.bootstrap.carousel', [])

.controller('UibCarouselController', ['$scope', '$element', '$interval', '$animate', function($scope, $element, $interval, $animate) {
.controller('UibCarouselController', ['$scope', '$element', '$interval', '$timeout', '$animate', function($scope, $element, $interval, $timeout, $animate) {
var self = this,
slides = self.slides = $scope.slides = [],
SLIDE_DIRECTION = 'uib-slideDirection',
currentIndex = -1,
currentInterval, isPlaying;
currentInterval, isPlaying, bufferedTransitions = [];
self.currentSlide = null;

var destroyed = false;
Expand All @@ -19,6 +19,8 @@ angular.module('ui.bootstrap.carousel', [])
//Prevent this user-triggered transition from occurring if there is already one in progress
if (nextSlide && nextSlide !== self.currentSlide && !$scope.$currentTransition) {
goNext(nextSlide, nextIndex, direction);
} else if (nextSlide && nextSlide !== self.currentSlide && $scope.$currentTransition) {
bufferedTransitions.push(nextSlide);
}
};

Expand All @@ -40,6 +42,14 @@ angular.module('ui.bootstrap.carousel', [])
if (phase === 'close') {
$scope.$currentTransition = null;
$animate.off('addClass', element);
if (bufferedTransitions.length) {
var nextSlide = bufferedTransitions.pop();
var nextIndex = $scope.indexOfSlide(nextSlide);
var nextDirection = nextIndex > self.getCurrentIndex() ? 'next' : 'prev';
clearBufferedTransitions();

goNext(nextSlide, nextIndex, nextDirection);
}
}
});
}
Expand Down Expand Up @@ -134,6 +144,13 @@ angular.module('ui.bootstrap.carousel', [])
function resetTransition(slides) {
if (!slides.length) {
$scope.$currentTransition = null;
clearBufferedTransitions();
}
}

function clearBufferedTransitions() {
while (bufferedTransitions.length) {
bufferedTransitions.shift();
}
}

Expand All @@ -155,6 +172,10 @@ angular.module('ui.bootstrap.carousel', [])
slides.push(slide);
//if this is the first slide or the slide is set to active, select it
if (slides.length === 1 || slide.active) {
if ($scope.$currentTransition) {
$scope.$currentTransition = null;
}

self.select(slides[slides.length - 1]);
if (slides.length === 1) {
$scope.play();
Expand All @@ -170,22 +191,30 @@ angular.module('ui.bootstrap.carousel', [])
return +a.index > +b.index;
});
}

var bufferedIndex = bufferedTransitions.indexOf(slide);
if (bufferedIndex !== -1) {
bufferedTransitions.splice(bufferedIndex, 1);
}
//get the index of the slide inside the carousel
var index = slides.indexOf(slide);
slides.splice(index, 1);
if (slides.length > 0 && slide.active) {
if (index >= slides.length) {
self.select(slides[index - 1]);
} else {
self.select(slides[index]);
$timeout(function() {
if (slides.length > 0 && slide.active) {
if (index >= slides.length) {
self.select(slides[index - 1]);
} else {
self.select(slides[index]);
}
} else if (currentIndex > index) {
currentIndex--;
}
} else if (currentIndex > index) {
currentIndex--;
}
});

//clean the currentSlide when no more slide
if (slides.length === 0) {
self.currentSlide = null;
clearBufferedTransitions();
}
};

Expand Down
45 changes: 43 additions & 2 deletions src/carousel/test/carousel.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ describe('carousel', function() {
});
}
}));
beforeEach(module('ngAnimateMock'));
beforeEach(module('uib/template/carousel/carousel.html', 'uib/template/carousel/slide.html'));

var $rootScope, $compile, $controller, $interval, $templateCache;
beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$interval_, _$templateCache_) {
var $rootScope, $compile, $controller, $interval, $templateCache, $timeout, $animate;
beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$interval_, _$templateCache_, _$timeout_, _$animate_) {
$rootScope = _$rootScope_;
$compile = _$compile_;
$controller = _$controller_;
$interval = _$interval_;
$templateCache = _$templateCache_;
$timeout = _$timeout_;
$animate = _$animate_;
}));

describe('basics', function() {
Expand Down Expand Up @@ -291,11 +294,13 @@ describe('carousel', function() {
scope.$apply('slides[2].active = true');
testSlideActive(2);
scope.$apply('slides.splice(0,1)');
$timeout.flush(0);
expect(elm.find('div.item').length).toBe(2);
testSlideActive(1);
$interval.flush(scope.interval);
testSlideActive(0);
scope.$apply('slides.splice(1,1)');
$timeout.flush(0);
expect(elm.find('div.item').length).toBe(1);
testSlideActive(0);
});
Expand Down Expand Up @@ -325,6 +330,39 @@ describe('carousel', function() {
testSlideActive(1);
});

it('should buffer the slides if transition is clicked and only transition to the last requested', function() {
var carouselScope = elm.children().scope();

testSlideActive(0);
carouselScope.$currentTransition = null;
carouselScope.select(carouselScope.slides[1]);
$animate.flush();

testSlideActive(1);

carouselScope.$currentTransition = true;
carouselScope.select(carouselScope.slides[2]);
scope.$apply();

testSlideActive(1);

carouselScope.select(carouselScope.slides[0]);
scope.$apply();

testSlideActive(1);

carouselScope.$currentTransition = null;
$interval.flush(scope.interval);
$animate.flush();

testSlideActive(2);

$interval.flush(scope.interval);
$animate.flush();

testSlideActive(0);
});

it('issue 1414 - should not continue running timers after scope is destroyed', function() {
testSlideActive(0);
$interval.flush(scope.interval);
Expand Down Expand Up @@ -468,13 +506,16 @@ describe('carousel', function() {
it('should remove slide and change active slide if needed', function() {
expect(ctrl.slides.length).toBe(4);
ctrl.removeSlide(ctrl.slides[0]);
$timeout.flush(0);
expect(ctrl.slides.length).toBe(3);
expect(ctrl.currentSlide).toBe(ctrl.slides[0]);
ctrl.select(ctrl.slides[2]);
ctrl.removeSlide(ctrl.slides[2]);
$timeout.flush(0);
expect(ctrl.slides.length).toBe(2);
expect(ctrl.currentSlide).toBe(ctrl.slides[1]);
ctrl.removeSlide(ctrl.slides[0]);
$timeout.flush(0);
expect(ctrl.slides.length).toBe(1);
expect(ctrl.currentSlide).toBe(ctrl.slides[0]);
});
Expand Down