Random-timed image changer with the possibility of changing them manually

[boris90]

Hi,

this is my first post here. Hope I'll get some help here.
I need someone to look at my code and tell me where I'm wrong. I just started learning JavaScript and I need someone to tell me how to do this (I did it, but it's bugged).

I have the site code where I've used the given images for an automatic slideshow. I managed to make a script which generates a random number between 3000 and 8000, but generates only whole thousands (3000, 4000, 5000, 6000, 7000 and 8000). These numbers represent the miliseconds for the function I use, which is setTimeout(). I then use the randomly generated time as a parameter in the setTimeout() function, which, in a given random time, calls the rotate() function which switches between the two images (does the image change).
Now, my boss told me this is a nicely done thing, except that he couldn't make this to work on his regular website (I think we concluded there was a problem with image names later: his images didn't change at all). Nevertheless, he told me he'll try to fix the implementation, because he saw my code worked on my laptop and didn't on his PC, and to try to make the images change not only by themselves with a random timer, but also when a user clicks on the current image.
I tried doing this by inserting the onclick attribute in the <a> tag, which did the thing he wanted, but the thing is it doesn't work properly: the image changes, but sometimes one of the images appears for too short, which is like it is being skipped. If I try clicking some more, the images start changing faster and faster, until it's unbearable.

My guess is that something must be wrong with the random time generator, a problem which occurs whenever I click on the image (call the rotate() function for changing the images).
What do you suggest?

Thanks.

You can download the whole demo from the link below (I couldn't post the long code here).
http://www.mediafire.com/?9wnb9gv97cgat79

[xelawho]

Hello Boris90 and welcome,

I will not be downloading your file nor unzipping it to my machine. I hope you understand. If it is really too long to post here, I suspect that it contains a whole lot of stuff that is not relevant to the issue at hand (or else it is ancient and inefficient code that you found somewhere and should probably be rewritten). Often it helps clarify what is going on to make a "stripped down" version of your code, with just the problematic functionality - sometimes then you can see what the problem is. And if not, it should be small enough to post here.

Failing all that, find some free hosting, and put it online for us to look at.

In general, it sounds to me like you are not clearing the timeout when you are clicking on an image, so you have multiple timeouts running which will cause the function to be called multiple times.

But that's just a guess - without further detail it's hard to say.

[boris90]

I'll try clearing the setTimout() function, and I'll post back.
On the other side, I assure you the file is, not 100%, but 200% safe. Who'd even want to infect other people's PC's when he needs help?
Anyway, I'll post back when I test this.

[boris90]

Hey,

I've tried your method, though I'm not sure I did it right.
I wrote a complete new function like this:

function imageClick()
{
clearTimeout();
rotate();
}

The rotate function changes between two images (current and the next one). This is the code:

function rotate() {
//Get the first image
var current = ($('div.rotator ul li.show')? $('div.rotator ul li.show') : $('div.rotator ul li:first'));

if ( current.length == 0 ) current = $('div.rotator ul li:first');

//Get next image, when it reaches the end, rotate it back to the first image
var next = ((current.next().length) ? ((current.next().hasClass('show')) ? $('div.rotator ul li:first') :current.next()) : $('div.rotator ul li:first'));

//Un-comment the 3 lines below to get the images in random order

//var sibs = current.siblings();
//var rndNum = Math.floor(Math.random() * sibs.length );
//var next = $( sibs[ rndNum ] );


//Set the fade in effect for the next image, the show class has higher z-index
next.css({opacity: 0.0})
.addClass('show')
.animate({opacity: 1.0}, 1000);

//Hide the current image
current.animate({opacity: 0.0}, 1000)
.removeClass('show');

var time = getRandom(timeMin, timeMax);
setTimeout("rotate()", time);

};

What do you think?

[boris90]

Yeah, I forgot. It doesn't work.
And I added the onClick attribute in the <a> tags (which make the images into links) which calls the imageClick() function on the image click.

[xelawho]

you have to name your timeout, and it is better to pass it a function reference instead of a string:

to=setTimeout(rotate, time);

then you can do

clearTimeout(to);

but depending on which function gets called first, it would be safer to code it as

if(to){
clearTimeout(to);
}

[boris90]

OK, now my rotate function looks like this.

function rotate() {
//Get the first image
var current = ($('div.rotator ul li.show')? $('div.rotator ul li.show') : $('div.rotator ul li:first'));

if ( current.length == 0 ) current = $('div.rotator ul li:first');

//Get next image, when it reaches the end, rotate it back to the first image
var next = ((current.next().length) ? ((current.next().hasClass('show')) ? $('div.rotator ul li:first') :current.next()) : $('div.rotator ul li:first'));

//Un-comment the 3 lines below to get the images in random order

//var sibs = current.siblings();
//var rndNum = Math.floor(Math.random() * sibs.length );
//var next = $( sibs[ rndNum ] );


//Set the fade in effect for the next image, the show class has higher z-index
next.css({opacity: 0.0})
.addClass('show')
.animate({opacity: 1.0}, 1000);

//Hide the current image
current.animate({opacity: 0.0}, 1000)
.removeClass('show');

var time = getRandom(timeMin, timeMax);
var to = setTimeout(rotate, time);
to;

};

And the imageClick() function, which is activated when you click on an image to change it manually:

function imageClick()
{
rotate();

clearTimeout(to);

}

I also tried writing the imageClick function in the pattern: rotate, clearTimeout and then rotate again.
I get the same skipping effect whenever I click images multiple times (even when I click the current image once, one of the images appears for probably half a second and switches to another one).
The more I click on the images, the faster they switch between each other (sometimes they blink).

How can I fix this?

[boris90]

Also, if I put clearTimeout(to) after the var to = setTimeout(rotate, time);, the image changing doesn't seem to occur. The first image just stands still. Looks like the clearTimout function clears the random time for the setTimout function and images stop changing.

[xelawho]

I suspect we might need to see your entire code, but from what I can see it seems that it should be like this:

var to; //declare it outside a function to make it available to all functions

function rotate() {	
//Get the first image
var current = ($('div.rotator ul li.show')? $('div.rotator ul li.show') : $('div.rotator ul li:first'));

if ( current.length == 0 ) current = $('div.rotator ul li:first');

//Get next image, when it reaches the end, rotate it back to the first image
var next = ((current.next().length) ? ((current.next().hasClass('show')) ? $('div.rotator ul li:first') :current.next()) : $('div.rotator ul li:first'));

//Un-comment the 3 lines below to get the images in random order

//var sibs = current.siblings();
//var rndNum = Math.floor(Math.random() * sibs.length );
//var next = $( sibs[ rndNum ] );


//Set the fade in effect for the next image, the show class has higher z-index
next.css({opacity: 0.0})
.addClass('show')
.animate({opacity: 1.0}, 1000);

//Hide the current image
current.animate({opacity: 0.0}, 1000)
.removeClass('show');

var time = getRandom(timeMin, timeMax);
to = setTimeout(rotate, time); // leave out the var to make it accessible in the global scope to other functions
//to; don't know what this is

};


function imageClick(){
clearTimeout(to); //clear timeout first, then call function
rotate();
}

[boris90]

The to; part was supposed to execute the function stored in the to variable, but I removed it.

I did as you said:

function rotate() {
//Get the first image
var current = ($('div.rotator ul li.show')? $('div.rotator ul li.show') : $('div.rotator ul li:first'));

if ( current.length == 0 ) current = $('div.rotator ul li:first');

//Get next image, when it reaches the end, rotate it back to the first image
var next = ((current.next().length) ? ((current.next().hasClass('show')) ? $('div.rotator ul li:first') :current.next()) : $('div.rotator ul li:first'));

//Un-comment the 3 lines below to get the images in random order

//var sibs = current.siblings();
//var rndNum = Math.floor(Math.random() * sibs.length );
//var next = $( sibs[ rndNum ] );


//Set the fade in effect for the next image, the show class has higher z-index
next.css({opacity: 0.0})
.addClass('show')
.animate({opacity: 1.0}, 1000);

//Hide the current image
current.animate({opacity: 0.0}, 1000)
.removeClass('show');

var time = getRandom(timeMin, timeMax);
to = setTimeout(rotate, time); // leave out the var to make it accessible in the global scope to other functions


};

function imageClick()
{
clearTimeout(to); //clear timeout first, then call function
rotate();
}

But unfortunately, it doesn't work. I mean, the images change automatically AND when I click on them, but the problem with faster and faster image changing still persists.

You might want to download the code I posted earlier and see what I have (that was before I made these changes you just gave me).