3
\$\begingroup\$

I created a slider that fades images automatically but you can also click to see the next or previous image. It already works how it should. But I think my code is really heavy. I am not experienced with JavaScript / jQuery so it would be nice if someone could help me to tidy up my code.

// helper to change the slider background image
var changeImage = function(id, image){
  $(id).css('background-image', 'url('+image+')');
};

// auto change slider elements over time
$(window).load(function() { 
    var images = [  'http://mysite.com/image1',
                    'http://mysite.com/image2', 
                    'http://mysite.com/image3', 
                    'http://mysite.com/image4', 
                    'http://mysite.com/image5',
                    'http://mysite.com/image6'
                ];
    var texts = [
                    '<h1>Text 1</h1>', 
                    '<h1>Text 2</h1>', 
                    '<h1>Text 3</h1>', 
                    '<h1>Text 4</h1>', 
                    '<h1>Text 5</h1>',
                    '<h1>Text 6</h1>'
                ];
    var hrefs = [   'http://mysite.com/link1',
                    'http://mysite.com/link2', 
                    'http://mysite.com/link3', 
                    'http://mysite.com/link4', 
                    'http://mysite.com/link5',
                    'http://mysite.com/link6'
                ];
    var srcs = [    'http://mysite.com/image1',
                    'http://mysite.com/image2', 
                    'http://mysite.com/image3', 
                    'http://mysite.com/image4', 
                    'http://mysite.com/image5',
                    'http://mysite.com/image6'                      
                ]; 
    // counter variables for images, texts and links
    var i = 0;
    var t = 0;
    var h = 0;
    var s = 0;

    var x = 0;   
    var inter = "";
    var again = "";

    var hrefChange = document.getElementById('theHref');   

    // register next slide click 
    document.getElementById("clickablen").onclick = function() {
        clearInterval(inter);
        changeBackgroundNext();
    }

    // register previous slide click    
    document.getElementById("clickablep").onclick = function() {
        clearInterval(inter);
        changeBackgroundPrev();
    }

    // Init sequence, loading the first image
    $("#wrapper_bottom").css("opacity", 0);
    changeImage('#wrapper_bottom', images[i]);
    changeBackground();

    // slider functionality - general function
    function changeBackground() {
        if ( i >= 6 ) { i = 0; }
        if ( t >= 6 ) { t = 0; }
        if ( h >= 6 ) { h = 0; }
        if ( s >= 6 ) { s = 0; }

        x = 0;

        $("#wrapper_bottom").css("opacity", 0);
        changeImage('#wrapper_bottom', images[i]);
        document.getElementById("myspan").innerHTML=texts[t];     

        $('#wrapper_bottom').animate({"opacity": 1}, 1000, function(){
            hrefChange.setAttribute('href', hrefs[h]);        
            changeImage('#wrapper_top', images[i], 1);
            if (++i >= images.length) { i = 0; x = 1} 
            $("#wrapper_bottom").css("opacity", 0);
            changeImage('#wrapper_bottom', images[i]);
            if (++t >= texts.length) { t = 0; } 
            if (++h >= hrefs.length) { h = 0; } 
            if (++s >= srcs.length) { s = 0; } 
        });

        // fade content background image              
        if (srcs[s]) {
            $('#thesrc').fadeOut(300, function(){
                $(this).attr('src',srcs[s]).bind('onreadystatechange load', function(){
                   if (this.complete) $(this).fadeIn(300);
                });
            }); 
        }
    }
    // repeat this function every 10 seconds
    inter = setInterval(changeBackground, 10000);

    // slider functionality - click next function
    function changeBackgroundNext() {  
        clearInterval(again);
        again = setInterval(changeBackground, 10000);

        if ( i == 6 ) { i = 0; }
        if ( t == 6 ) { t = 0; }
        if ( h == 6 ) { h = 0; }
        if ( s == 6 ) { s = 0; }

        x = 0;

        document.getElementById("myspan").innerHTML=texts[t];     

        $('#wrapper_bottom').animate({"opacity": 1}, 0, function(){
            hrefChange.setAttribute('href', hrefs[h]);        
            changeImage('#wrapper_top', images[i], 1);
            $("#wrapper_bottom").css("opacity", 0);
            if (++i >= images.length) { i = 0; x = 1;} 
            changeImage('#wrapper_bottom', images[i]);
            if (++t >= texts.length) { t = 0; } 
            if (++h >= hrefs.length) { h = 0; } 

            if (srcs[s]) {
                $('#thesrc').fadeOut(0, function(){
                    $(this).attr('src',srcs[s]).bind('onreadystatechange load', function(){
                        if (this.complete) $(this).fadeIn(0);
                    });
                });
                if (++s >= srcs.length) { s = 0; } 
            }
        });
    }

    // slider functionality - click previous function
    function changeBackgroundPrev() {  
        clearInterval(again);
        again = setInterval(changeBackground, 10000);

        i=i-2;
        t=t-2;
        h=t-2;
        s=s-2;

        if ( i <= -1 ) { i = 5; }
        if ( t <= -1 ) { t = 5; }
        if ( h <= -1 ) { h = 5; }
        if ( s <= -1 ) { s = 5; }

        if (x == 1) { i = 4; t = 4; h = 4; s = 4; }

        document.getElementById("myspan").innerHTML=texts[t];     
        $('#wrapper_bottom').animate({"opacity": 1}, 0, function(){
            hrefChange.setAttribute('href', hrefs[h]);        
            changeImage('#wrapper_top', images[i], 1);
            $("#wrapper_bottom").css("opacity", 0);
            changeImage('#wrapper_bottom', images[i]);

            if (srcs[s]) {
                $('#thesrc').fadeOut(0, function(){
                    $(this).attr('src',srcs[s]).bind('onreadystatechange load', function(){
                        if (this.complete) $(this).fadeIn(0);
                    });
                });                     
            }

            x = 0;

            ++i;
            ++t;
            ++h;
            ++s;   
        });       
    }                   
});  
\$\endgroup\$
2
\$\begingroup\$

A few thoughts :

  • It's hard to see why you need four congruant arrays. Why not one array containing objects, each with properties .image, .text, .href, .src?
  • Probably better to increment/decrement the indices "on demand" not in advance. That way you only ever need to adjust by +1 or -1. Currently, -2 is only necessary because +1 was previously predicted.
  • Use the module operator % to acheve the "wrap-around" effect. Javascript doesn't naturally do modulo of negative numbers properly, but a simple solution is available.
  • Don't hard-code 6. Use images.length etc all through.
  • jQuery's .bind() is deprecated in favour of .on(), but you should probably be using .one() to prevent the build-up of event handlers.

Also :

  • Similar functions changeBackground(), changeBackgroundNext() and changeBackgroundPrev() could be combined into one, with the detailed behaviour determined by parameters; it seems that direction 1|-1 and a boolean indicating manual|auto will suffice.
  • Two interval vars, inter and again seem unnecessary. One should suffice.

Without seeing a demo, it's hard to visualise the interplay between #wrapper_top and #wrapper_bottom and the differences betweeen a manual and auto so those aspects may be wrong, but the general pattern might be as follows :

// Auto change slider elements over time
$(window).load(function() {
    // *** Outer vars ***
    var data = [
        {
            'image': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg', 
            'text': '<h1>Text 1</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg'
        },
        {
            'image': 'http://www.heise.de/imgs/18/1/4/9/2/9/0/1/urn-newsml-dpa-com-20090101-150420-99-02936_large_4_3-fc997534cde9c449.jpeg',
            'text': '<h1>Text 2</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://www.heise.de/imgs/18/1/4/9/2/9/0/1/urn-newsml-dpa-com-20090101-150420-99-02936_large_4_3-fc997534cde9c449.jpeg'
        },
        {
            'image': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg',
            'text': '<h1>Text 3</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg'
        },
        {
            'image': 'http://www.heise.de/imgs/18/1/4/9/2/9/0/1/urn-newsml-dpa-com-20090101-150420-99-02936_large_4_3-fc997534cde9c449.jpeg',
            'text': '<h1>Text 4</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://www.heise.de/imgs/18/1/4/9/2/9/0/1/urn-newsml-dpa-com-20090101-150420-99-02936_large_4_3-fc997534cde9c449.jpeg'
        },
        {
            'image': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg',
            'text': '<h1>Text 5</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg'
        },
        {
            'image': 'http://www.heise.de/imgs/18/1/4/9/2/9/0/1/urn-newsml-dpa-com-20090101-150420-99-02936_large_4_3-fc997534cde9c449.jpeg',
            'text': '<h1>Text 6</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://www.heise.de/imgs/18/1/4/9/2/9/0/1/urn-newsml-dpa-com-20090101-150420-99-02936_large_4_3-fc997534cde9c449.jpeg', 
        },
        {
            'image': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg',
            'text': '<h1>Text 7</h1><h2>...</h2>',
            'href': 'http://google.de/',
            'src': 'http://p5.focus.de/img/fotos/crop3167630/6282711837-w1200-h627-o-q75-p5/Google.jpg'                     
        }
    ];
    var i = data.length - 1; // counter variable for images, texts, links and srcs
    var inter;
    var hrefChange = document.getElementById('theHref');
    var title = document.getElementById("myspan");

    // *** Worker functions ***
    // Change an element's bg image
    function changeImage(id, image) {
        return $(id).css('background-image', 'url(' + image + ')');
    }
    // Initiate auto-play
    function auto() {
        inter = setInterval(function() {
            changeBackground(1, true);
        }, 10000);
    }
    // Slider functionality - general function
    function changeBackground(dir, isAuto) {
        var t1 = isAuto ? 1000 : 0;
        var t2 = isAuto ? 300 : 0;
        var ii = i; // remember index before incrementing/decrementing

        i = (((i + dir) % data.length) + data.length) % data.length; // increment/decrement index, with wrap-around in both directions.

        title.innerHTML = data[i].text;

        if(isAuto) {
            changeImage('#wrapper_bottom', data[ii].image).css('opacity', 1);
        }

        $('#wrapper_bottom').animate({'opacity': 1}, t1, function() {
            hrefChange.setAttribute('href', data[i].href);
            changeImage('#wrapper_top', data[ii].image);
            changeImage('#wrapper_bottom', data[i].image).css('opacity', 1);
        });

        // fade content background image
        if (data[i].src) {
            $('#thesrc').fadeOut(t2, function() {
                $(this).attr('src', data[i].src).off('onreadystatechange load').one('onreadystatechange load', function() {
                    if (this.complete) $(this).fadeIn(t2);
                });
            });
        }
    }

    // *** Attach event handlers ***
    document.getElementById("clickablen").onclick = function() {
        clearInterval(inter);
        changeBackground(1, false);
        auto();
    };
    document.getElementById("clickablep").onclick = function() {  
        clearInterval(inter);
        changeBackground(-1, false);
        auto();
    };

    // *** Initialize ***
    changeBackground(1, true); // Load first image
    auto(); // Start auto play
});

Auto-play is assumed always to go forward. If it should go in reverse after clickablep, then simple change - auto() will need to accept 1|-1, which it can pass on to changeBackground(1, true);.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your helpful tips. But could you perhaps provide a full solution? Especially point 2 - I do not really have an idea how to do this the right way. That would be great if you could point this out. Thank you in advance. \$\endgroup\$ – Nico Feb 28 '16 at 23:41
  • \$\begingroup\$ Difficult without the ability to run tests. \$\endgroup\$ – Roamer-1888 Feb 29 '16 at 0:21
  • \$\begingroup\$ However, I have had a go - see edit. \$\endgroup\$ – Roamer-1888 Feb 29 '16 at 2:47
  • \$\begingroup\$ Thank you for your time. But it seems not to work for me. So I created a small demo in jsbin with my code to show you how it works. Maybe you could implement your javascript code there? Thank you in advance: jsbin.com/nirafimazu/1/edit?html,css,js,output \$\endgroup\$ – Nico Feb 29 '16 at 12:27
  • \$\begingroup\$ Just needed the right data and a couple of minor bug fixes - edited above. Seems to run now. \$\endgroup\$ – Roamer-1888 Feb 29 '16 at 21:04

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.