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. Closes #5987
angular-ui · Jun 12, 2016 · 5046bd4 · 5046bd4
1 parent 2458c28
commit 5046bd4
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 90 deletions.
diff --git 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
@@ -1,14 +1,14 @@
 <div ng-controller="CarouselDemoCtrl">
   <div style="height: 305px">
-    <uib-carousel active="active" interval="myInterval" no-wrap="noWrapSlides">
-      <uib-slide ng-repeat="slide in slides track by slide.id" index="slide.id">
+    <div uib-carousel active="active" interval="myInterval" no-wrap="noWrapSlides">
+      <div uib-slide ng-repeat="slide in slides track by slide.id" index="slide.id">
         <img ng-src="{{slide.image}}" style="margin:auto;">
         <div class="carousel-caption">
           <h4>Slide {{slide.id}}</h4>
           <p>{{slide.text}}</p>
         </div>
-      </uib-slide>
-    </uib-carousel>
+      </div>
+    </div>
   </div>
   <div class="row">
     <div class="col-md-6">

diff --git 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(
-        '<uib-carousel active="active" interval="interval" no-transition="true" no-pause="nopause">' +
-          '<uib-slide ng-repeat="slide in slides track by slide.index" index="slide.index">' +
+        '<div uib-carousel active="active" interval="interval" no-transition="true" no-pause="nopause">' +
+          '<div uib-slide ng-repeat="slide in slides track by slide.index" index="slide.index">' +
             '{{slide.content}}' +
-          '</uib-slide>' +
-        '</uib-carousel>'
+          '</div>' +
+        '</div>'
       )(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', '<div>foo</div>');
 
-      elm = $compile('<uib-carousel template-url="foo/bar.html"></uib-carousel>')(scope);
+      elm = $compile('<div uib-carousel template-url="foo/bar.html"></div>')(scope);
       $rootScope.$digest();
 
-      expect(elm.html()).toBe('foo');
+      expect(elm.html()).toBe('<div>foo</div>');
     });
 
     it('should allow overriding of the slide template', function() {
       $templateCache.put('foo/bar.html', '<div class="slide">bar</div>');
 
       elm = $compile(
-        '<uib-carousel interval="interval" no-transition="true" no-pause="nopause">' +
-          '<uib-slide template-url="foo/bar.html"></uib-slide>' +
-        '</uib-carousel>'
+        '<div uib-carousel interval="interval" no-transition="true" no-pause="nopause">' +
+          '<div uib-slide template-url="foo/bar.html"></div>' +
+        '</div>'
       )(scope);
       $rootScope.$digest();
 
@@ -101,11 +89,11 @@ describe('carousel', function() {
 
     it('should stop cycling slides forward when noWrap is truthy', function () {
       elm = $compile(
-        '<uib-carousel active="active" interval="interval" no-wrap="noWrap">' +
-          '<uib-slide ng-repeat="slide in slides track by slide.index" index="slide.index">' +
+        '<div uib-carousel active="active" interval="interval" no-wrap="noWrap">' +
+          '<div uib-slide ng-repeat="slide in slides track by slide.index" index="slide.index">' +
             '{{slide.content}}' +
-          '</uib-slide>' +
-        '</uib-carousel>'
+          '</div>' +
+        '</div>'
       )(scope);
 
       scope.noWrap = true;
@@ -124,11 +112,11 @@ describe('carousel', function() {
 
     it('should stop cycling slides backward when noWrap is truthy', function () {
       elm = $compile(
-        '<uib-carousel active="active" interval="interval" no-wrap="noWrap">' +
-          '<uib-slide ng-repeat="slide in slides track by slide.index" index="slide.index">' +
+        '<div uib-carousel active="active" interval="interval" no-wrap="noWrap">' +
+          '<div uib-slide ng-repeat="slide in slides track by slide.index" index="slide.index">' +
             '{{slide.content}}' +
-          '</uib-slide>' +
-        '</uib-carousel>'
+          '</div>' +
+        '</div>'
       )(scope);
 
       scope.noWrap = true;
@@ -147,11 +135,11 @@ describe('carousel', function() {
       scope.slides = [{active:false,content:'one'}];
       scope.$apply();
       elm = $compile(
-        '<uib-carousel active="active" interval="interval" no-transition="true">' +
-          '<uib-slide ng-repeat="slide in slides" index="$index">' +
+        '<div uib-carousel active="active" interval="interval" no-transition="true">' +
+          '<div uib-slide ng-repeat="slide in slides" index="$index">' +
             '{{slide.content}}' +
-          '</uib-slide>' +
-        '</uib-carousel>'
+          '</div>' +
+        '</div>'
       )(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(
-        '<uib-carousel active="active" interval="interval" no-transition="true" no-pause="nopause">' +
-          '<uib-slide ng-repeat="slide in slides | orderBy: \'id\' track by slide.id" index="slide.id">' +
+        '<div uib-carousel active="active" interval="interval" no-transition="true" no-pause="nopause">' +
+          '<div uib-slide ng-repeat="slide in slides | orderBy: \'id\' track by slide.id" index="slide.id">' +
             '{{slide.content}}' +
-          '</uib-slide>' +
-        '</uib-carousel>'
+          '</div>' +
+        '</div>'
       )(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', '<div>{{carousel.text}}</div>');
 
       var scope = $rootScope.$new();
-      var elm = $compile('<uib-carousel interval="bar" no-transition="false" no-pause="true"></uib-carousel>')(scope);
+      var elm = $compile('<div uib-carousel interval="bar" no-transition="false" no-pause="true"></div>')(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('<div class="ng-binding">foo</div>');
     }));
   });
 
@@ -605,11 +579,11 @@ describe('carousel', function() {
       {active:false,content:'three'}
     ];
     var elm = $compile(
-      '<uib-carousel active="active" interval="interval" no-transition="true" no-pause="nopause">' +
-        '<uib-slide ng-repeat="slide in slides" index="$index" actual="slide">' +
+      '<div uib-carousel active="active" interval="interval" no-transition="true" no-pause="nopause">' +
+        '<div uib-slide ng-repeat="slide in slides" index="$index" actual="slide">' +
           '{{slide.content}}' +
-        '</uib-slide>' +
-      '</uib-carousel>'
+        '</div>' +
+      '</div>'
     )(scope);
     $rootScope.$digest();
 

diff --git a/template/carousel/carousel.html b/template/carousel/carousel.html
@@ -1,16 +1,14 @@
-<div ng-mouseenter="pause()" ng-mouseleave="play()" class="carousel" ng-swipe-right="prev()" ng-swipe-left="next()">
-  <div class="carousel-inner" ng-transclude></div>
-  <a role="button" href class="left carousel-control" ng-click="prev()" ng-class="{ disabled: isPrevDisabled() }" ng-show="slides.length > 1">
-    <span aria-hidden="true" class="glyphicon glyphicon-chevron-left"></span>
-    <span class="sr-only">previous</span>
-  </a>
-  <a role="button" href class="right carousel-control" ng-click="next()" ng-class="{ disabled: isNextDisabled() }" ng-show="slides.length > 1">
-    <span aria-hidden="true" class="glyphicon glyphicon-chevron-right"></span>
-    <span class="sr-only">next</span>
-  </a>
-  <ol class="carousel-indicators" ng-show="slides.length > 1">
-    <li ng-repeat="slide in slides | orderBy:indexOfSlide track by $index" ng-class="{ active: isActive(slide) }" ng-click="select(slide)">
-      <span class="sr-only">slide {{ $index + 1 }} of {{ slides.length }}<span ng-if="isActive(slide)">, currently active</span></span>
-    </li>
-  </ol>
-</div>
+<div class="carousel-inner" ng-transclude></div>
+<a role="button" href class="left carousel-control" ng-click="prev()" ng-class="{ disabled: isPrevDisabled() }" ng-show="slides.length > 1">
+  <span aria-hidden="true" class="glyphicon glyphicon-chevron-left"></span>
+  <span class="sr-only">previous</span>
+</a>
+<a role="button" href class="right carousel-control" ng-click="next()" ng-class="{ disabled: isNextDisabled() }" ng-show="slides.length > 1">
+  <span aria-hidden="true" class="glyphicon glyphicon-chevron-right"></span>
+  <span class="sr-only">next</span>
+</a>
+<ol class="carousel-indicators" ng-show="slides.length > 1">
+  <li ng-repeat="slide in slides | orderBy:indexOfSlide track by $index" ng-class="{ active: isActive(slide) }" ng-click="select(slide)">
+    <span class="sr-only">slide {{ $index + 1 }} of {{ slides.length }}<span ng-if="isActive(slide)">, currently active</span></span>
+  </li>
+</ol>
diff --git a/template/carousel/slide.html b/template/carousel/slide.html
@@ -1,3 +1 @@
-<div ng-class="{
-    'active': active
-  }" class="item text-center" ng-transclude></div>
+<div class="text-center" ng-transclude></div>