Fix no-transition attribute data storage lookup

[steven-pribilinskiy]

Following code

  $scope.$watch('noTransition', function(noTransition) {
    $element.data(NO_TRANSITION, noTransition);
  });

sets a data attribute value on the carousel element.

The element.parent() that can be found in animation mistakenly targets the child .carousel-inner hence the attribute have no effect on the slider.

This PR fixes this

[wesleycho]

I don't like this, but I am not sure if there is a better way in this case. My only concern is that this implementation is too heavily tied to the DOM, but I am not sure of a better way here.

[wesleycho]

Spending some time thinking about this, I think this is not quite the right approach.

A better way to accomplish this would be to add a unique identifier for each carousel and store it on the DOM node, i.e. $element.data(CAROUSEL_ID, uuid), and then store it on the the slide element. Inside the animation portion, the carousel id can be retrieved from the slide element, and then it traverses up the DOM tree until it finds a parent node with matching data. If the data matches, it then checks for the NO_TRANSITION data key. This guarantees that any custom templates will not break from this change.

[steven-pribilinskiy]

Ok, for now I've wrote a decorator to work around this

.config(function($provide) {
    $provide.decorator('carouselDirective', function($delegate) {
        var carouselDirective = $delegate[0];

        carouselDirective.compile = function() {
            return function(scope, element, attrs) {
                var NO_TRANSITION = 'uib-noTransition';

                scope.$watch('noTransition', function(noTransition) {
                    element.find('> .carousel-inner').data(NO_TRANSITION, noTransition);
                });
            };
        };

        return $delegate;
    });
})

Don't really understand why an uuid is needed when the carousel directive has an isolate scope with the noTransition attribute that is unique for each instance of this component

[wesleycho]

The DOM traversal cannot be allowed to stay due to allowance of custom templates.

[steven-pribilinskiy]

How about just adding additional watch like in the above decorator, e.g.

  $scope.$watch('noTransition', function(noTransition) {
    $element.data(NO_TRANSITION, noTransition);
  });
  $scope.$watch('noTransition', function(noTransition) {
    var carouselInnerEl = $element.querySelector('> .carousel-inner');
    carouselInnerEl && angular.element(carouselInnerEl).data(NO_TRANSITION, noTransition);
  });

[wesleycho]

Question about this - do you desire to disable the transition behavior while still consuming ngAnimate?

I revisited this just now with @Foxandxss, and I am wondering whether we should just strip out all of the noTransition logic completely, but I'd like to understand if this is a genuine problem for you or just cruft that you saw was handled incorrectly.

[wesleycho]

Going to merge this for now as a quick fix, but will open an issue for the broader question about the logic being too tied to the DOM.