From bfa58c0a8d99e2111dbadaf21e91b7a9ecb5fef3 Mon Sep 17 00:00:00 2001 From: Kury Kruitbosch Date: Tue, 11 Nov 2014 11:08:23 -0700 Subject: [PATCH 1/2] fix(carousel): #488 Carousel fails to update model changes. - Uses dom element index. - Adds order to indicators to match slide order. --- src/carousel/carousel.js | 67 +++++++++++++++++++++++++----- src/carousel/test/carousel.spec.js | 23 +++++++++- template/carousel/carousel.html | 2 +- 3 files changed, 80 insertions(+), 12 deletions(-) diff --git a/src/carousel/carousel.js b/src/carousel/carousel.js index 9d91004da7..e96d2c27eb 100644 --- a/src/carousel/carousel.js +++ b/src/carousel/carousel.js @@ -17,7 +17,7 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) var destroyed = false; /* direction: "prev" or "next" */ self.select = $scope.select = function(nextSlide, direction) { - var nextIndex = slides.indexOf(nextSlide); + var nextIndex = self.indexOfSlide(nextSlide); //Decide direction if it's not given if (direction === undefined) { direction = nextIndex > currentIndex ? 'next' : 'prev'; @@ -59,7 +59,7 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) transitionDone(nextSlide, self.currentSlide); } self.currentSlide = nextSlide; - currentIndex = nextIndex; + currentIndex = self.indexOfSlide(nextSlide); //every time you change slides, reset the timer restartTimer(); } @@ -72,27 +72,55 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) $scope.$on('$destroy', function () { destroyed = true; }); - /* Allow outside people to call indexOf on slides array */ self.indexOfSlide = function(slide) { - return slides.indexOf(slide); + if(angular.isUndefined(slide.$element)) { + return slides.indexOf(slide); + } + var slideElements = slide.$element.parent().children(); + for(var i = 0; i < slideElements.length; i++){ + if(slideElements[i] === slide.$element[0]){ + return i; + } + } + return false; }; - $scope.next = function() { - var newIndex = (currentIndex + 1) % slides.length; + $scope.indexOfSlide = function (slide) { + return self.indexOfSlide(slide); + }; + $scope.next = function() { //Prevent this user-triggered transition from occurring if there is already one in progress if (!$scope.$currentTransition) { - return self.select(slides[newIndex], 'next'); + var nextSlide = null; + if(angular.isUndefined(self.currentSlide.$element)) { + return false; + } + var nextElement = self.currentSlide.$element.next(); + if(nextElement.length === 1){ + nextSlide = getSlideFromElement(nextElement); + } else { + nextSlide = getFirstSlide(); + } + return self.select(nextSlide, 'next'); } }; $scope.prev = function() { - var newIndex = currentIndex - 1 < 0 ? slides.length - 1 : currentIndex - 1; - //Prevent this user-triggered transition from occurring if there is already one in progress if (!$scope.$currentTransition) { - return self.select(slides[newIndex], 'prev'); + var prevSlide = null; + if(angular.isUndefined(self.currentSlide.$element)) { + return false; + } + var prevElement = self.currentSlide.$element[0].previousElementSibling; + if(prevElement !== null){ + prevSlide = getSlideFromElement(prevElement); + } else { + prevSlide = getLastSlide(); + } + return self.select(prevSlide, 'next'); } }; @@ -140,6 +168,25 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) } }; + function getSlideFromElement(element) { + return angular.element(element).scope().$$childHead; + } + + function getSlideElements() { + return self.currentSlide.$element.parent().children(); + } + + function getLastSlide() { + var slideElements = getSlideElements(); + var lastSlideElement = slideElements[slideElements.length-1]; + return getSlideFromElement(lastSlideElement); + } + + function getFirstSlide(){ + var firstSlideElement = getSlideElements()[0]; + return getSlideFromElement(firstSlideElement); + } + self.addSlide = function(slide, element) { slide.$element = element; slides.push(slide); diff --git a/src/carousel/test/carousel.spec.js b/src/carousel/test/carousel.spec.js index f8dac14457..f7e19646f0 100644 --- a/src/carousel/test/carousel.spec.js +++ b/src/carousel/test/carousel.spec.js @@ -266,7 +266,28 @@ describe('carousel', function() { scope.$destroy(); expect($interval.cancel).toHaveBeenCalled(); }); - + it('should respect changes in order', function() { + scope.slides[0].order = 1; + scope.slides[1].order = 2; + scope.slides[2].order = 3; + var elm = $compile( + '' + + '' + + '{{slide.content}}' + + '' + + '' + )(scope); + $rootScope.$digest(); + scope.slides[0].order = 3; + scope.slides[1].order = 2; + scope.slides[2].order = 1; + scope.slides[0].active = true; + $rootScope.$digest(); + var navNext = elm.find('a.right'); + testSlideActive(0); + navNext.click(); + testSlideActive(2); + }); }); describe('controller', function() { diff --git a/template/carousel/carousel.html b/template/carousel/carousel.html index 9769b383a6..6577b73f45 100644 --- a/template/carousel/carousel.html +++ b/template/carousel/carousel.html @@ -1,6 +1,6 @@
    -
  1. +
From efcf9d5d13b3b4d25cddfd0bccf98a227d890127 Mon Sep 17 00:00:00 2001 From: Kury Kruitbosch Date: Tue, 11 Nov 2014 14:22:39 -0700 Subject: [PATCH 2/2] fix(carousel): Prev direction and dont rely on scope hierarchy - Prev function calls change slide with directon of 'prev'. - Find element in slides array rather than through scope's $$childHead. --- src/carousel/carousel.js | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/carousel/carousel.js b/src/carousel/carousel.js index e96d2c27eb..ba65882d27 100644 --- a/src/carousel/carousel.js +++ b/src/carousel/carousel.js @@ -99,7 +99,7 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) } var nextElement = self.currentSlide.$element.next(); if(nextElement.length === 1){ - nextSlide = getSlideFromElement(nextElement); + nextSlide = getSlideFromDomElement(nextElement); } else { nextSlide = getFirstSlide(); } @@ -116,11 +116,11 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) } var prevElement = self.currentSlide.$element[0].previousElementSibling; if(prevElement !== null){ - prevSlide = getSlideFromElement(prevElement); + prevSlide = getSlideFromDomElement(prevElement); } else { prevSlide = getLastSlide(); } - return self.select(prevSlide, 'next'); + return self.select(prevSlide, 'prev'); } }; @@ -168,8 +168,16 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) } }; - function getSlideFromElement(element) { - return angular.element(element).scope().$$childHead; + function getSlideFromDomElement(element) { + if(element.length){ + element = element[0]; + } + for (var i = slides.length - 1; i >= 0; i--) { + if(slides[i].$element[0] === element){ + return slides[i]; + } + } + return false; } function getSlideElements() { @@ -179,12 +187,12 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition']) function getLastSlide() { var slideElements = getSlideElements(); var lastSlideElement = slideElements[slideElements.length-1]; - return getSlideFromElement(lastSlideElement); + return getSlideFromDomElement(lastSlideElement); } function getFirstSlide(){ var firstSlideElement = getSlideElements()[0]; - return getSlideFromElement(firstSlideElement); + return getSlideFromDomElement(firstSlideElement); } self.addSlide = function(slide, element) {