So on my new website in google chrome, I am trying to make the images switch every 5 seconds using a setInterval function. This seems to work, however I have the problem cannot set property src of null.
var images = new Array();
images[0] = "/images/grilled-chicken-large.png";
images[1] = "/images/sizzly-steak.png";
var img = document.getElementById("img");
function changeImage() {
for (var i = 0; i < 3; i++) {
img.src = images[i];
if (i == 2){i = i - 2;};
}
}
window.onload=setInterval("changeImage()", 5000);
I know the problem is that I am trying to get the element with an id of img before the page is done loading, but I've already got a window.onload, so i don't know what to do. Also, if i make an init() function containing the setInterval and the img variable, the page freezes.
Any ideas?
So on my new website in google chrome, I am trying to make the images switch every 5 seconds using a setInterval function. This seems to work, however I have the problem cannot set property src of null.
var images = new Array();
images[0] = "/images/grilled-chicken-large.png";
images[1] = "/images/sizzly-steak.png";
var img = document.getElementById("img");
function changeImage() {
for (var i = 0; i < 3; i++) {
img.src = images[i];
if (i == 2){i = i - 2;};
}
}
window.onload=setInterval("changeImage()", 5000);
I know the problem is that I am trying to get the element with an id of img before the page is done loading, but I've already got a window.onload, so i don't know what to do. Also, if i make an init() function containing the setInterval and the img variable, the page freezes.
Any ideas?
Share Improve this question asked Jul 30, 2012 at 21:29 Ilan BialaIlan Biala 3,4195 gold badges37 silver badges45 bronze badges 4-
3
You only have
images[0]
andimages[1]
, but you're trying to accessimages[2]
. – bfavaretto Commented Jul 30, 2012 at 21:34 -
2
Side note: don't pass strings to
setInterval
, it useseval
. Pass functions:setInterval(changeImage, 5000);
. Also,window.onload = setInterval(changeImage, 5000);
doesn't do what you think it does. – gen_Eric Commented Jul 30, 2012 at 21:39 - yea just realized that bfavaretto, thanks. Rocket, do you mean just pass it as changeImage() instead of "changeImage()"? And what do you mean by the last line not doing what I think it does? – Ilan Biala Commented Jul 31, 2012 at 0:03
-
@Ilan: Bit too much for ments, please create separate questions for them. The last line executes
setInterval
, immediately then assigns the result of that method (it's an integer used as ID) towindow.onload
, which does not make sense. See my anwer, I've added theonload
stuff. – Yogu Commented Jul 31, 2012 at 20:00
4 Answers
Reset to default 3The problem is that you access element 2
of an array with only two elements (0 and 1).
But there is also a logical error in your script: Where do you store the index of the currently visible image? You need a global variable which holds that index.
var currentIndex = 0;
function changeImage() {
// Switch currentIndex in a loop
currentIndex++;
if (currentIndex >= images.length)
currentIndex = 0;
var img = document.getElementById("img");
img.src = images[currentIndex];
}
document.onload = function() {
setInterval(changeImage, 5000);
}
I've moved the img
variable into the function so that it is assigned after five seconds when the document has loaded.
Just to clear things up: JavaScript is an event-based programming language. That means that the slideshow-change code is executed every time the interval fires, does some work (switch to the next slide) and then is done, so that the browser can render the page. If you iterate through the slides in a loop, there is no way for the browser to show the new slide because the script is still executing between the slides. The solution is what I've posted: Define a global variable that replaces the loop variable, and increment it every time the next slide is to be shown.
Here:
window.onload = function () {
var images = [
'http://placekitten./100/200',
'http://placekitten./200/100',
];
var img = document.getElementById( 'img' );
setInterval(function () {
img.src = img.src === images[0] ? images[1] : images[0];
}, 1000 );
};
Live demo: http://jsfiddle/wkqpr/
If you have more than two images, you can do this:
window.onload = function () {
var images = [
'http://placekitten./100/200',
'http://placekitten./200/100',
'http://placekitten./200/150',
'http://placekitten./150/300'
];
images.current = 0;
var img = document.getElementById( 'img' );
setInterval(function () {
img.src = images[ images.current++ ];
if ( images.current === images.length ) images.current = 0;
}, 1000 );
};
Live demo: http://jsfiddle/wkqpr/1/
Your for loop looks odd. Try that loop:
function changeImage(){
if(!img.attributes["index"]){img.attributes["index"]=0;return;};
img.attributes["index"]++;
img.src=images[img.attributes["index"]];
}
window.onload=function(){
window.img=document.getElementById("img");
setInterval("changeImage()",5000)
}
Should work.
You'll notice that I store the image index in an attribute of the image, as opposed to
a global variable. I think the global is slightly faster, but storing it in the image
means you can easily expand this to multiple images if needed.
Also, the problem with your browser freezing was that the following code:
for(var i=0;i<3;i++){
//Some more stuff
if(i==2){i-=2};
}
This loop is infinite, because whenever i
reaches 2
, i==2
bees true, which means that iteration will reset i
to 0
and start it all over again. The only way to prevent that would be a break;
somewhere, which I don't see anywhere.
Tip: It is generally a bad idea to tamper with the index variable in for loops.
function changeImage()
{
for (var i = 0; i < 2; i++)
{
img.src = images[i];
}
}
window.onload=function()
{
setInterval(changeImage,5000);
}
Your last line in the for loop is pletely useless and just plicate things, Also, Your way of setting a window onload event is not standard!
EDIT: the src
property is null because , obviously, your array size is only 2!!