[thelist] Logic problem with a JavaScript image toggle...

Simon Willison cs1spw at bath.ac.uk
Fri Nov 7 12:14:52 CST 2003


Chris George wrote:
> function flip(curArrow) {
> 
> if (flipBack && (oldArrow != curArrow)) {
>  	flipBack="";
> }
> alert("curArrow:"+curArrow+", oldArrow:"+oldArrow);
>     if (document.images) 
> 		if (!flipBack) {
>         	document.images['image'+curArrow].src =
> 'images/menuarrow_off.gif';			
> 			flipBack="1";
> 			oldArrow=curArrow;
> 		} else {
> 			document.images['image'+curArrow].src =
> 'images/menuarrow_on.gif';
> 			flipBack="";
> 		}
> 		alert("flipBack:"+flipBack);
> 
> }

When working with nested if statements it really helps to use a 
consistent indentation style - and to avoid any if statements that don't 
use brackets around the body of the statenebt as these can lead to 
subtle, hard to spot bugs. Here's your function with the indentation fixed:

function flip(curArrow) {
     if (flipBack && (oldArrow != curArrow)) {
         flipBack="";
     }
     alert("curArrow:"+curArrow+", oldArrow:"+oldArrow);
     if (document.images) {
         if (!flipBack) {
             document.images['image'+curArrow].src = 
'images/menuarrow_off.gif';			
             flipBack="1";
             oldArrow=curArrow;
         } else {
             document.images['image'+curArrow].src = 
'images/menuarrow_on.gif';
             flipBack="";
         }
     }
     alert("flipBack:"+flipBack);
}

I think your actual problem is being caused by a reliance on global 
variables that cause inteference between the first and second times the 
function is called. One alternative would be to do something like this:

<a href="#" onclick="flip('image1'); return false;"><img
src="images/menuarrow_on.gif" id="image1" alt="+"></a>

function flip(imageID) {
     var img = document.getElementById(imageID);
     if (img.src == 'images/menuarrow_off.gif') {
         img.src = 'images/menuarrow_on.gif'
     } else {
         img.src = 'images/menuarrow_off.gif'
     }
}

I haven't tested the above code, but it demonstrates a different method 
whereby instead of using global variables the SRC attribute of the image 
element is used to track which image is currently being displayed.

Hope that helps,

Simon




More information about the thelist mailing list