2
\$\begingroup\$

I am working on a custom Bootstrap 3 carousel, with vertical, unidirectional, slide transitions.

I want to prevent slide transitions from overlapping when the bullets are clicked in random order and rapid succession.

I came up with this solution that relies on disabling them as long as a slide transition is in progress, then enabling them between transitions:

var carouselDuration = function() {
  $.fn.carousel.Constructor.TRANSITION_DURATION = 1000;
}

carouselDuration();
// While a transition is in progress (slide event), do this
$('#myCarousel').on('slide.bs.carousel', function (e) {
    var $indicator = $(this).find('.carousel-indicators>li');
    $indicator.css('pointer-events', 'none');
});

// While a transition has finished (slid event), do this
$('#myCarousel').on('slid.bs.carousel', function (e) {
    var $indicator = $(this).find('.carousel-indicators>li');
    $indicator.css('pointer-events', 'auto');
});
.carousel.vertical {
  position: relative;
}

.carousel.vertical .carousel-inner {
  height: 100%;
  width: auto;
}

.carousel.vertical .carousel-inner>.item {
  width: auto;
  transition: 1s ease-in-out;
  transform: translate3d(0, 100%, 0);
  top: 0;
}

.carousel.vertical .carousel-inner>.next,
.carousel.vertical .carousel-inner>.prev,
.carousel.vertical .carousel-inner>.right {
  transform: translate3d(0, 100%, 0);
  top: 0;
}

.carousel.vertical .carousel-inner>.left,
.carousel.vertical .carousel-inner>.prev.right,
.carousel.vertical .carousel-inner>.next.left,
.carousel.vertical .carousel-inner>.active {
  transform: translate3d(0, 0, 0);
  top: 0;
}

.carousel.vertical .carousel-inner>.active.right,
.carousel.vertical .carousel-inner>.active.left {
  transform: translate3d(0, -100%, 0);
  top: 0;
}

.carousel.vertical .carousel-indicators {
  display: inline-block;
  width: auto;
  padding: 0;
  margin: 0;
  left: auto;
  right: 10px;
  bottom: 2px;
  z-index: 9;
  font-size: 0;
}

.carousel.vertical .carousel-indicators li {
  border: none;
  cursor: pointer;
  display: inline-block;
  width: 18px;
  height: 18px;
  text-indent: -9999px;
  background: url("https://grgs.ro/1/i/sprite.png") no-repeat -528px -502px;
}

.carousel.vertical .carousel-indicators li.active {
  background-position: -528px -524px;
}
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>


<div class="container">
  <div id="myCarousel" class="carousel vertical slide" data-ride="carousel" data-interval="9000">
    <!-- Wrapper for slides -->
    <div class="carousel-inner">
      <div class="item active">
        <img src="https://picsum.photos/1200/300/?gravity=east" alt="">
      </div>

      <div class="item">
        <img src="https://picsum.photos/1200/300/?gravity=south" alt="">
      </div>

      <div class="item">
        <img src="https://picsum.photos/1200/300/?gravity=west" alt="">
      </div>
    </div>

    <!-- Indicators -->
    <ol class="carousel-indicators">
      <li data-target="#myCarousel" data-slide-to="0" class="active"></li>
      <li data-target="#myCarousel" data-slide-to="1"></li>
      <li data-target="#myCarousel" data-slide-to="2"></li>
    </ol>
  </div>
</div>

Still, I am looking for a queuing mechanism, as it is more natural and elegant.

\$\endgroup\$
1
\$\begingroup\$

Feedback

The code seems fairly concise, though I do have suggestions below that can simplify it even more. I was hoping there was some class name applied to the element with class carousel during the slide transition but apparently the bootstrap code does not do that, though you could use the event callback functions to apply a class name yourself. That way the CSS style for pointer-events can be moved into the CSS.

Because the .on() method accepts multiple events separated by a space, you can use one call for both events, and a single Partially applied function to call the .toggleClass() method, which will add and then remove a class to the carousel element:

var carouselCollection = $('#myCarousel');
carouselCollection.on('slide.bs.carousel slid.bs.carousel', 
    carouselCollection.toggleClass.bind(carouselCollection, 'sliding'));

Then the CSS can contain the style:

.carousel.vertical.sliding .carousel-indicators li {
  pointer-events: none;
}

See that working in the rewritten code below.

I don't see much point in creating the function carouselDuration() and calling it immediately afterwards but then never again. Why not just move that single line out where it is called? I could see a point in doing that after some condition is met (e.g. DOM ready, jQuery/carousel ready, etc).

Some of the CSS rulesets are a bit redundant. For instance, some of the top: 0 rulesets should be cascadable from .carousel.vertical .carousel-inner>.item and apply to the next three subsequent selectors.

Also, I believe I was able to remove this block without any adverse affects. Are there really elements with class .right and .left or are those for the left/right controls that aren't being used?

.carousel.vertical .carousel-inner>.next,
.carousel.vertical .carousel-inner>.prev,
.carousel.vertical .carousel-inner>.right {
  transform: translate3d(0, 100%, 0);
  top: 0;
}

Suggestions

It would likely help to define that value for the duration as a constant (and if you are using then use const) at the top of the code, so it can be easily modified in the future if necessary.


While there are only a couple redundant DOM queries (e.g. $(#myCarousel)) it would be wise to cache those in a variable.


The event handler functions are very redundant, and violate the Don't Repeat Yourself principle. The only thing that appears to change is the value assigned to the CSS style pointer-events. If the approach mentioned above with the .toggleClass() method doesn't work, the common code in the two callback functions could be abstracted into a single function that accepts the value for that CSS style, and then Partially applied functions could be created with Function.bind() to fix the value to apply to the style.

var carouselCollection = $('#myCarousel');

function setPointerEventsOnListItems(value, e) {
  var $indicator = carouselCollection.find('.carousel-indicators>li');
  $indicator.css('pointer-events', value);
}
// While a transition is in progress (slide event), do this
carouselCollection.on('slide.bs.carousel', setPointerEventsOnListItems.bind(null, 'none'));

// While a transition has finished (slid event), do this
carouselCollection.on('slid.bs.carousel', setPointerEventsOnListItems.bind(null, 'auto'));

As you likely know, the jQuery DOM ready function would be good to use so as to avoid accidentally accessing the DOM before it is ready (though that isn't a problem as much nowdays with modern browsers and adding the script tags to the end of the <body>). An IIFE could also be used instead of that jQuery DOM ready callback.


Rewritten code

The code below utilizes the advice above, though utilizes the .toggleClass() method.

var DURATION = 1000; //could use const instead of var if supporting es-6
$(function() { //DOM ready callback
  $.fn.carousel.Constructor.TRANSITION_DURATION = DURATION;
  var carouselCollection = $('#myCarousel');
  // While a transition is in progress (slide event), do this
  carouselCollection.on('slide.bs.carousel slid.bs.carousel', carouselCollection.toggleClass.bind(carouselCollection, 'sliding'));
});
.carousel.vertical {
  position: relative;
}

.carousel.vertical .carousel-inner {
  height: 100%;
  width: auto;
}

.carousel.vertical .carousel-inner>.item {
  width: auto;
  transition: 1s ease-in-out;
  transform: translate3d(0, 100%, 0);
  top: 0;
}

.carousel.vertical .carousel-inner>.left,
.carousel.vertical .carousel-inner>.prev.right,
.carousel.vertical .carousel-inner>.next.left,
.carousel.vertical .carousel-inner>.active {
  transform: translate3d(0, 0, 0);
}

.carousel.vertical .carousel-inner>.active.right,
.carousel.vertical .carousel-inner>.active.left {
  transform: translate3d(0, -100%, 0);
}

.carousel.vertical .carousel-indicators {
  display: inline-block;
  width: auto;
  padding: 0;
  margin: 0;
  left: auto;
  right: 10px;
  bottom: 2px;
  z-index: 9;
  font-size: 0;
}

.carousel.vertical .carousel-indicators li {
  border: none;
  cursor: pointer;
  display: inline-block;
  width: 18px;
  height: 18px;
  text-indent: -9999px;
  background: url("https://grgs.ro/1/i/sprite.png") no-repeat -528px -502px;
}

.carousel.vertical.sliding .carousel-indicators li {
  pointer-events: none;
}
.carousel.vertical .carousel-indicators li.active {
  background-position: -528px -524px;
}
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>


<div class="container">
  <div id="myCarousel" class="carousel vertical slide" data-ride="carousel" data-interval="9000">
    <!-- Wrapper for slides -->
    <div class="carousel-inner">
      <div class="item active">
        <img src="https://picsum.photos/1200/300/?gravity=east" alt="">
      </div>

      <div class="item">
        <img src="https://picsum.photos/1200/300/?gravity=south" alt="">
      </div>

      <div class="item">
        <img src="https://picsum.photos/1200/300/?gravity=west" alt="">
      </div>
    </div>

    <!-- Indicators -->
    <ol class="carousel-indicators">
      <li data-target="#myCarousel" data-slide-to="0" class="active"></li>
      <li data-target="#myCarousel" data-slide-to="1"></li>
      <li data-target="#myCarousel" data-slide-to="2"></li>
    </ol>
  </div>
</div>

\$\endgroup\$
  • \$\begingroup\$ Thank a lot for the feedback and the effort to rewrite the code. Can you help with the queuing mechanism or some other solution that would make the slider more user-friendly, meaning the the user would not have to click (temporarily) inactive bullets? \$\endgroup\$ – Razvan Zamfir Nov 27 '18 at 11:04
  • \$\begingroup\$ Please provide an answer to the topic An improvable use case of the Swiper slider. Thanks! \$\endgroup\$ – Razvan Zamfir Nov 27 '18 at 11:10
  • \$\begingroup\$ I hope you don’t get the wrong impression about this site, though maybe you have, since many answers offer a rewrite- especially easy with questions involving JS because snippets make it simple for demonstrating. Requests for code implementations are off-topic for this site. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 27 '18 at 11:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.