RJcreatives
RJcreatives

Reputation: 45

Javascript onclick issue?

I am having a problem with some code which I am trying to use as a basic image slider in google Chrome.

My HTML is as follows -

<!DOCTYPE html>
<html>
<head>
<title>Gallery</title>
    <link rel="stylesheet" href="css/gallery.css">
</head>
<body>

    <div class="wrapper">


    <div id="slider">
        <Img id="1" src="images/slide1.jpg">
        <Img id="2" src="images/slide 2.jpg">
        <Img id="3" src="images/slide 3.jpg">
    </div>

    <a href="#" class="left" onclick="prev(); return false;">Previous</a>
    <a href="#" class="right" onclick="next(); return false;">Next</a>


    </div>



    <script src="js/jquery.js"></script>
    <script src="js/gallery.js"></script>

</body>
</html>

and this is my Javascript -

sliderInt=1;
sliderNext=2;

$(document).ready(function(){
    $("#slider>img#1").fadeIn(300);
    startSlider();

});

function startSlider(){
    count=$("#slider>Img").size();

    loop=setInterval(function(){

        if(sliderNext>count){
            sliderNext=1;
            sliderInt=1;
        }

        $("#slider>Img").fadeOut(300);
        $("#slider>Img#" + sliderNext).fadeIn(300);

        sliderInt=sliderNext;
        sliderNext=sliderNext + 1;

    },3000);

}

function prev(){
    newSlide=sliderInt-1;
    showSlide(newSlide);
}

function next(){
    newSlide=sliderInt + 1;
    showSlide(newSlide);
}

function showSlide(){
    if(id>count){
        id=1;
    }else if(id<1){
        id=count;
    }

    $("#slider>Img").fadeOut(300);
    $("#slider>Img#" + id).fadeIn(300);

    sliderInt=id;
    sliderNext=id + 1;

}

However when I click the next/previous text, nothing happens.

Any advice?

PS. this is my first html and java attempt.

Many Thanks

Upvotes: 0

Views: 546

Answers (3)

NotJustin
NotJustin

Reputation: 529

setInterval does not get count variable. The startSlider runs, inits the setInterval and ends. The count is not saved in more global postion eg outside the function. It is destroyed as the funciont ends. So instead above code, you should either:

var count = 0;

function startSlider(){
}

OR

setInterval(function(count){

        if(sliderNext>count){
            sliderNext=1;
            sliderInt=1;
        }

        $("#slider>Img").fadeOut(300);
        $("#slider>Img#" + sliderNext).fadeIn(300);

        sliderInt=sliderNext;
        sliderNext=sliderNext + 1;

    },3000, count);

If it still does not work, you have problem with you might have a problem with your logic.

Upvotes: 0

Mykyta Shyrin
Mykyta Shyrin

Reputation: 322

Open Google Chrome console (ctrl+shift+I), you'll see javascript error's.

  1. It's bad idea, to use onclick attribute. You can use jQuery events. Something like that: $(".left").on("click", prev);

  2. $("#slider>Img").fadeOut(300); $("#slider>Img#" + sliderNext).fadeIn(300);

It's wrong, because, second line will start before the first finished Use this code:

$("#slider>Img").fadeOut(300);
setTimeout(function() {
    $("#slider>Img#" + sliderNext).fadeIn(300);
}, 300);

or

$.when( $("#slider>Img").fadeOut(300); )
 .done( $("#slider>Img#" + sliderNext).fadeIn(300); );

Upvotes: 1

Marcus Brunsten
Marcus Brunsten

Reputation: 572

I would remove the attribute for onclick on in the html for the buttons, and add a listner in jquery instead, since i feel its easier to keep all jquery/javascript in one place.

see fiddle here: http://jsfiddle.net/h25z9/

$(".right").click(function(){
   prev(); 
});
 $(".left").click(function(){
   next(); 
})

This doesnt work on jsfiddle since i dont have all resource, but you should see the changes i did to the code.

Upvotes: 0

Related Questions