Unfinished Loop

[2nervous]

I created the following function to change the visibility of nine
different table cells. It appears to only execute the first instead of
looping. How can I get it to loop through?


function toggleSelection(ID) {
var i=1
for (i=1;i<=9;i++)
{
var pID = "cell-" + i + "-id-" + ID;
var cell = document.getElementById(pID);

if(cell.style.visibility == "hidden" || !cell.style.visibility) {
cell.style.visibility = "visible";

}
else if(cell.style.visibility == "visible") {
cell.style.visibility = "hidden";

}
return false;
}


}

 

[Michael Winter]

I created the following function to change the visibility of nine
different table cells. It appears to only execute the first instead of
looping. How can I get it to loop through?

[snip]

Perhaps if you employed a better formatting style, the reason and
solution would become obvious:

function toogleSelection(id) {
for (var i = 1; i <= 9; ++i) {
var pID = 'cell-' + i + '-id-' + id,
cell = document.getElementById(pID);

if ((cell.style.visibility == 'hidden')
|| !cell.style.visibility) {
cell.style.visibility = 'visible';
} else if (cell.style.visibility == 'visible') {
cell.style.visibility = 'hidden';
}
return false;
}
}

That if statement could be greatly shortened to:

if (cell.style.visibility == 'visible') {
cell.style.visibility = 'hidden';
} else {
cell.style.visibility = 'visible';
}

or:

cell.style.visibility = (cell.style.visibility == 'visible')
? 'hidden' : 'visible';

Some feature detection wouldn't go amiss.

Mike

 

[2nervous]

Thanks for the reply, however I get the same result; the first toggles,
but the rest do not fire.

I created the following function to change the visibility of nine
different table cells. It appears to only execute the first instead of
looping. How can I get it to loop through?

[snip]

Perhaps if you employed a better formatting style, the reason and
solution would become obvious:

function toogleSelection(id) {
for (var i = 1; i <= 9; ++i) {
var pID = 'cell-' + i + '-id-' + id,
cell = document.getElementById(pID);

if ((cell.style.visibility == 'hidden')
|| !cell.style.visibility) {
cell.style.visibility = 'visible';
} else if (cell.style.visibility == 'visible') {
cell.style.visibility = 'hidden';
}
return false;
}
}

That if statement could be greatly shortened to:

if (cell.style.visibility == 'visible') {
cell.style.visibility = 'hidden';
} else {
cell.style.visibility = 'visible';
}

or:

cell.style.visibility = (cell.style.visibility == 'visible')
? 'hidden' : 'visible';

Some feature detection wouldn't go amiss.

Mike

 

[2nervous]

I had noticed before my post that when I removed " return false; " that
a 4 of the cells would toggle before the error.

The problem was there was not a 5th cell. The error was because of the
missing object. My bad! Renaming and adjusting my code solved the
problem.

Thanks for the reply, however I get the same result; the first toggles,
but the rest do not fire.

I created the following function to change the visibility of nine
different table cells. It appears to only execute the first instead of
looping. How can I get it to loop through?

[snip]

Perhaps if you employed a better formatting style, the reason and
solution would become obvious:

function toogleSelection(id) {
for (var i = 1; i <= 9; ++i) {
var pID = 'cell-' + i + '-id-' + id,
cell = document.getElementById(pID);

if ((cell.style.visibility == 'hidden')
|| !cell.style.visibility) {
cell.style.visibility = 'visible';
} else if (cell.style.visibility == 'visible') {
cell.style.visibility = 'hidden';
}
return false;
}
}

That if statement could be greatly shortened to:

if (cell.style.visibility == 'visible') {
cell.style.visibility = 'hidden';
} else {
cell.style.visibility = 'visible';
}

or:

cell.style.visibility = (cell.style.visibility == 'visible')
? 'hidden' : 'visible';

Some feature detection wouldn't go amiss.

Mike

 

[J R Stockton]

cell.style.visibility = (cell.style.visibility == 'visible')
? 'hidden' : 'visible';

It's a pity that the property uses a "word-string" rather than something
which will pass for a Boolean. With F.X0 being an <Input type=text>,
consider

Z = ! (F.X0.value ^= 1)

which, without repetition in the code, toggles F.X0.value between 0 & 1
and sets Z alternately to true or false. Presumably it would work with
any other "argument".

 

[Michael Winter]

It's a pity that the property uses a "word-string" rather than something
which will pass for a Boolean.

If there were only two values for the property, perhaps. As it is, there
are four ('collapse' and 'inherit' are the other two).

[snip]

Mike