3
\$\begingroup\$

I wanted to create the most shortest JavaScript code for an endless rotation of elements, without any controls. The elements can be images or SVG-elements (only the Vanilla JS version will work here) or text-container. It doesn't matter. I came up with this:

HTML

<div id="container" class="container">
    <img src="http://placehold.it/300&text=1" alt="">
    <img src="http://placehold.it/300&text=2" alt="">
    <img class="active" src="http://placehold.it/300&text=3" alt="">
    <img src="http://placehold.it/300&text=4" alt="">
</div>

CSS

.container > img {
    opacity: 0;
    position: absolute;
     -webkit-transition: all 1s linear;
    transition: all 1.5s linear;
}

.container > img.active {
    opacity: 1;
}

Vanilla JS

var e = document.getElementById('container');
if (e) {
    setInterval(function() {
        var current = container.getElementsByClassName('active')[0];
        current.classList.remove('active');
        var next = current.nextElementSibling || container.firstElementChild;
        next.classList.add('active');
    }, 2000);
}

Try it here

jQuery

var images = $('.container > img');

if (images.length) {
    setInterval(function() {
        var current = images.filter('.active');    
        current.removeClass('active');
        var next = ( current.next().length ? current.next() : images.eq(0) );
        next.addClass('active');    
    }, 2000);
}

Try it here

Is there anything that can be improved? I'm especially curious about the first line within each interval which involve a request (getElementsByClassName, Vanilla JS) or a loop (.filter, jQuery).

\$\endgroup\$
6
\$\begingroup\$

Eliminating functional differences between Vanilla JS and jQuery versions

Your vanilla script and your jQuery script are not functionally equivalent. Your vanilla is selecting the container by its ID, while the jQuery is selecting the container by its class and then selecting the images. Your vanilla script would more closely resemble the jQuery by using document.querySelector() and/or document.querySelectorAll().

e.g.: var images = document.querySelectorAll('.container > img');

Making it work for multiple containers

Your code will function strangely if more than one image rotator container is on the page: the jQuery will grab image elements from all the containers and only set one to active at a time, while the vanilla javascript will only change elements in the first container it grabs.

(EDIT: As you noted in the comments, you can work around this by selecting the different containers by ID before passing them to your function, but that approach requires manually specifying IDs both in the HTML and in the JavaScript. Ideally, you'd be able to just add more elements with the appropriate classes in the HTML to take advantage of the existing image rotator functionality.)

With a little tweaking, you can ensure your code processes each container separately (allowing each container to have its own active image).

var containers = document.querySelectorAll('.container');
for (var i = 0; i < containers.length; i++) {
  beginRotation(containers[i]);
}
function beginRotation(container){
    setInterval(function() {
        var current = container.querySelector('.active');
        current.classList.remove('active');
        var next = current.nextElementSibling || container.firstElementChild;
        next.classList.add('active');
    }, 2000);
}

The above code first grabs all the elements with the container class, then sets the interval to run on each of them.

Minor nitpick(s)

You don't need that semicolon at the end of your if(e){ ... } statement in your vanilla js example.

Supporting older browsers (if you have to)

IE9 and below do not support classList, so if you want to support them you'll have to use the className property.

To remove a class, something like this would work:

current.className = current.className.replace("active", "").trim();

and its complement to add a class:

next.className = (next.className + " active").trim();

Note that this only really works because we know what to expect in the class names. This would break, for example, if you had an element with a class my-active-class because it would replace the substring active. A better (but more tedious) approach would be to split className into an array (using element.className.split(" ")) and check each array element for the desired class.

You'd also want to be aware that although IE9 supports CSS opacity, it doesn't support CSS transitions. The end result is that the images will transition promptly instead of fading in and out for IE9 users.

Working Example

var containers = document.querySelectorAll('.container');
for (var i = 0; i < containers.length; i++) {
  beginRotation(containers[i]);
}

function beginRotation(container) {
  setInterval(function() {
    var current = container.querySelector('.active');
    current.className = current.className.replace("active", "").trim();
    var next = current.nextElementSibling || container.firstElementChild;
    next.className = (next.className + " active").trim();
  }, 2000);
}
.container > img {
  opacity: 0;
  position: absolute;
  -webkit-transition: all 1s linear;
  transition: all 1.5s linear;
}
.container > img.active {
  opacity: 1;
}
<div class="container">
  <img src="http://placehold.it/300x100&text=1" alt="" />
  <img src="http://placehold.it/300x100&text=2" alt="" />
  <img class="active" src="http://placehold.it/300x100&text=3" alt="" />
  <img src="http://placehold.it/300x100&text=4" alt="" />
</div>
<div style="height:100px">&nbsp;</div>
<div class="container">
  <img class="active" src="http://placehold.it/300x100&text=1" alt="" />
  <img src="http://placehold.it/300x100&text=2" alt="" />
  <img src="http://placehold.it/300x100&text=3" alt="" />
  <img src="http://placehold.it/300x100&text=4" alt="" />
</div>

EDIT: Efficiency Suggestion(s)

Caching DOM Queries

You can make the code in setInterval more efficient by getting all the necessary DOM operations out of the way beforehand.

If you want to reduce the number of DOM queries (which are typically expensive operations) you can cache an array of references to all the relevant <img> elements and then reference that array in the setInterval function.

I know you're hoping for the "shortest and most efficient way", but this would be for efficiency at the expense of brevity, since you'd need to set up an extra variable to track the index of the active image. Using this index would save your code from having to read the current classLists of the images, requery the container element to get the active image, and traverse the DOM for the next sibling or first child.

var containers = document.querySelectorAll('.container');
for (var i = 0; i < containers.length; i++) {
    beginRotation(containers[i], containers[i].querySelectorAll('img'));
}
function beginRotation(container, imgs) {
    var i = getIndexOf(container.querySelector('.active'), imgs);
    setInterval(function () {
        imgs[i++].classList.remove('active');
        imgs[(i == imgs.length ? i = 0 : i)].classList.add('active');
    }, 2000);
}
function getIndexOf(target, arr) {
    var len = arr.length;
    while (len--) {
        if (target == arr[len]) {
            return len;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ The difference between both: I'm aware that both behave differently and that the selector by id will be faster than the querySelectorAll. The code to watch is actually the one within the interval. But thanks to point that out for other visitors. \$\endgroup\$ – insertusernamehere Apr 30 '15 at 9:00
  • \$\begingroup\$ The multi usage: As you can see in my first fiddle I already encapsulated the logic into a function, so it will work with multiple elements. Here's a forked fiddle. As this wasn't relevant for the question I didn't include it. :) \$\endgroup\$ – insertusernamehere Apr 30 '15 at 9:01
  • \$\begingroup\$ Older browser: As I didn't say anything about browser support, that's a nice addition, should have mentioned it. I assumed to use the latest versions of each browser. The opacity is also only an example of a transition to make the question more clear. :) \$\endgroup\$ – insertusernamehere Apr 30 '15 at 9:02
  • \$\begingroup\$ What's left is still the question whether the actual logic could be improved. \$\endgroup\$ – insertusernamehere Apr 30 '15 at 9:02
  • \$\begingroup\$ PS: You're right and sorry about the semicolon - it was left from removing some code. Good catch though. :) \$\endgroup\$ – insertusernamehere Apr 30 '15 at 9:05

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.