From 994ca7d78e3374079ffa93250312c4c38089e24f Mon Sep 17 00:00:00 2001 From: Wesley Cho Date: Sun, 12 Jun 2016 10:23:29 -0700 Subject: [PATCH] feat(carousel): remove replace: true usage - Remove `replace: true` usage from the carousel and the slide BREAKING CHANGE: Due to the removal of `replace: true`, this causes a slight HTML structure change to the carousel and the slide elements - see documentation demos to see how it changes. This also caused removal of the ngTouch built in support - if one is using ng-touch, one needs to add the `ng-swipe-left` and `ng-swipe-right` directives to the carousel element with relevant logic. --- src/carousel/carousel.js | 17 +++-- src/carousel/docs/demo.html | 8 +-- src/carousel/test/carousel.spec.js | 100 +++++++++++------------------ template/carousel/carousel.html | 30 ++++----- template/carousel/slide.html | 4 +- 5 files changed, 69 insertions(+), 90 deletions(-) diff --git a/src/carousel/carousel.js b/src/carousel/carousel.js index f745229c76..12fb6f188c 100644 --- a/src/carousel/carousel.js +++ b/src/carousel/carousel.js @@ -8,6 +8,7 @@ angular.module('ui.bootstrap.carousel', []) currentInterval, isPlaying, bufferedTransitions = []; var destroyed = false; + $element.addClass('carousel'); self.addSlide = function(slide, element) { slides.push({ @@ -145,6 +146,9 @@ angular.module('ui.bootstrap.carousel', []) } }; + $element.on('mouseenter', $scope.pause); + $element.on('mouseleave', $scope.play); + $scope.$on('$destroy', function() { destroyed = true; resetTimer(); @@ -280,9 +284,9 @@ angular.module('ui.bootstrap.carousel', []) .directive('uibCarousel', function() { return { transclude: true, - replace: true, controller: 'UibCarouselController', controllerAs: 'carousel', + restrict: 'A', templateUrl: function(element, attrs) { return attrs.templateUrl || 'uib/template/carousel/carousel.html'; }, @@ -296,11 +300,11 @@ angular.module('ui.bootstrap.carousel', []) }; }) -.directive('uibSlide', function() { +.directive('uibSlide', ['$animate', function($animate) { return { require: '^uibCarousel', + restrict: 'A', transclude: true, - replace: true, templateUrl: function(element, attrs) { return attrs.templateUrl || 'uib/template/carousel/slide.html'; }, @@ -309,14 +313,19 @@ angular.module('ui.bootstrap.carousel', []) index: '=?' }, link: function (scope, element, attrs, carouselCtrl) { + element.addClass('item'); carouselCtrl.addSlide(scope, element); //when the scope is destroyed then remove the slide from the current slides array scope.$on('$destroy', function() { carouselCtrl.removeSlide(scope); }); + + scope.$watch('active', function(active) { + $animate[active ? 'addClass' : 'removeClass'](element, 'active'); + }); } }; -}) +}]) .animation('.item', ['$animateCss', function($animateCss) { diff --git a/src/carousel/docs/demo.html b/src/carousel/docs/demo.html index 16ef7fc9dd..f2a6f48672 100644 --- a/src/carousel/docs/demo.html +++ b/src/carousel/docs/demo.html @@ -1,14 +1,14 @@
- - +
+
- - +
+
diff --git a/src/carousel/test/carousel.spec.js b/src/carousel/test/carousel.spec.js index e43ce5f6dd..87b586dbd4 100644 --- a/src/carousel/test/carousel.spec.js +++ b/src/carousel/test/carousel.spec.js @@ -1,17 +1,5 @@ describe('carousel', function() { - beforeEach(module('ui.bootstrap.carousel', function($compileProvider, $provide) { - angular.forEach(['ngSwipeLeft', 'ngSwipeRight'], makeMock); - function makeMock(name) { - $provide.value(name + 'Directive', []); //remove existing directive if it exists - $compileProvider.directive(name, function() { - return function(scope, element, attr) { - element.on(name, function() { - scope.$apply(attr[name]); - }); - }; - }); - } - })); + beforeEach(module('ui.bootstrap.carousel')); beforeEach(module('ngAnimateMock')); beforeEach(module('uib/template/carousel/carousel.html', 'uib/template/carousel/slide.html')); @@ -36,11 +24,11 @@ describe('carousel', function() { {content: 'three', index: 2} ]; elm = $compile( - '' + - '' + + '
' + + '
' + '{{slide.content}}' + - '' + - '' + '
' + + '
' )(scope); scope.interval = 5000; scope.nopause = undefined; @@ -60,19 +48,19 @@ describe('carousel', function() { it('should allow overriding of the carousel template', function() { $templateCache.put('foo/bar.html', '
foo
'); - elm = $compile('')(scope); + elm = $compile('
')(scope); $rootScope.$digest(); - expect(elm.html()).toBe('foo'); + expect(elm.html()).toBe('
foo
'); }); it('should allow overriding of the slide template', function() { $templateCache.put('foo/bar.html', '
bar
'); elm = $compile( - '' + - '' + - '' + '
' + + '
' + + '
' )(scope); $rootScope.$digest(); @@ -101,11 +89,11 @@ describe('carousel', function() { it('should stop cycling slides forward when noWrap is truthy', function () { elm = $compile( - '' + - '' + + '
' + + '
' + '{{slide.content}}' + - '' + - '' + '
' + + '
' )(scope); scope.noWrap = true; @@ -124,11 +112,11 @@ describe('carousel', function() { it('should stop cycling slides backward when noWrap is truthy', function () { elm = $compile( - '' + - '' + + '
' + + '
' + '{{slide.content}}' + - '' + - '' + '
' + + '
' )(scope); scope.noWrap = true; @@ -147,11 +135,11 @@ describe('carousel', function() { scope.slides = [{active:false,content:'one'}]; scope.$apply(); elm = $compile( - '' + - '' + + '
' + + '
' + '{{slide.content}}' + - '' + - '' + '
' + + '
' )(scope); var indicators = elm.find('ol.carousel-indicators > li'); expect(indicators.length).toBe(0); @@ -228,20 +216,6 @@ describe('carousel', function() { testSlideActive(0); }); - describe('swiping', function() { - it('should go next on swipeLeft', function() { - testSlideActive(0); - elm.triggerHandler('ngSwipeLeft'); - testSlideActive(1); - }); - - it('should go prev on swipeRight', function() { - testSlideActive(0); - elm.triggerHandler('ngSwipeRight'); - testSlideActive(2); - }); - }); - it('should select a slide when clicking on slide indicators', function () { var indicators = elm.find('ol.carousel-indicators > li'); indicators.eq(1).click(); @@ -269,7 +243,7 @@ describe('carousel', function() { }); it('should bind the content to slides', function() { - var contents = elm.find('div.item'); + var contents = elm.find('div.item [ng-transclude]'); expect(contents.length).toBe(3); expect(contents.eq(0).text()).toBe('one'); @@ -343,7 +317,7 @@ describe('carousel', function() { {content:'new3', index: 6} ]; scope.$apply(); - var contents = elm.find('div.item'); + var contents = elm.find('div.item [ng-transclude]'); expect(contents.length).toBe(3); expect(contents.eq(0).text()).toBe('new1'); expect(contents.eq(1).text()).toBe('new2'); @@ -441,11 +415,11 @@ describe('carousel', function() { {content: 'three', id: 2} ]; elm = $compile( - '' + - '' + + '
' + + '
' + '{{slide.content}}' + - '' + - '' + '
' + + '
' )(scope); scope.$apply(); }); @@ -465,7 +439,7 @@ describe('carousel', function() { scope.slides[1].id = 2; scope.slides[2].id = 1; scope.$apply(); - var contents = elm.find('div.item'); + var contents = elm.find('div.item [ng-transclude]'); expect(contents.length).toBe(3); expect(contents.eq(0).text()).toBe('three'); expect(contents.eq(1).text()).toBe('two'); @@ -491,7 +465,7 @@ describe('carousel', function() { scope.slides[2].id = 4; scope.slides.push({content:'four', id: 5}); scope.$apply(); - var contents = elm.find('div.item'); + var contents = elm.find('div.item [ng-transclude]'); expect(contents.length).toBe(4); expect(contents.eq(0).text()).toBe('two'); expect(contents.eq(1).text()).toBe('one'); @@ -503,7 +477,7 @@ describe('carousel', function() { testSlideActive(1); scope.slides.splice(1, 1); scope.$apply(); - var contents = elm.find('div.item'); + var contents = elm.find('div.item [ng-transclude]'); expect(contents.length).toBe(2); expect(contents.eq(0).text()).toBe('three'); expect(contents.eq(1).text()).toBe('one'); @@ -583,7 +557,7 @@ describe('carousel', function() { $templateCache.put('uib/template/carousel/carousel.html', '
{{carousel.text}}
'); var scope = $rootScope.$new(); - var elm = $compile('')(scope); + var elm = $compile('
')(scope); $rootScope.$digest(); var ctrl = elm.controller('uibCarousel'); @@ -593,7 +567,7 @@ describe('carousel', function() { ctrl.text = 'foo'; $rootScope.$digest(); - expect(elm.html()).toBe('foo'); + expect(elm.html()).toBe('
foo
'); })); }); @@ -605,11 +579,11 @@ describe('carousel', function() { {active:false,content:'three'} ]; var elm = $compile( - '' + - '' + + '
' + + '
' + '{{slide.content}}' + - '' + - '' + '
' + + '
' )(scope); $rootScope.$digest(); diff --git a/template/carousel/carousel.html b/template/carousel/carousel.html index dc98a5aaed..1d76a98958 100644 --- a/template/carousel/carousel.html +++ b/template/carousel/carousel.html @@ -1,16 +1,14 @@ - + + + + previous + + + + next + + diff --git a/template/carousel/slide.html b/template/carousel/slide.html index 522013922a..d2938998e9 100644 --- a/template/carousel/slide.html +++ b/template/carousel/slide.html @@ -1,3 +1 @@ -
+