Michael Geary wrote:
Rob, I think you left out the return statement that you meant to put in. :-)
(Outstanding explanation, BTW!)
For clarity, it could be:
jQuery.fn.toggleVis = function() {
this.each(function() {
if (this.style.visibility == 'hidden') {
this.style.visibility = 'visible';
} else {
this.style.visibility = 'hidden';
}
});
return this;
};
Or, taking advantage of the fact that "each" returns "this" for convenience:
jQuery.fn.toggleVis = function() {
return this.each(function() {
if (this.style.visibility == 'hidden') {
this.style.visibility = 'visible';
} else {
this.style.visibility = 'hidden';
}
});
};
Mitch, take careful note that "this" means two different things in that
code:
jQuery.fn.toggleVis = function() {
/* Here, "this" is the jQuery object */
return this.each(function() {
/* Here, "this" is a DOM element */
if (this.style.visibility == 'hidden') {
this.style.visibility = 'visible';
} else {
this.style.visibility = 'hidden';
}
});
};
The use of "this" inside an "each" loop is one of the two major design
errors in jQuery (the other being the event system, which I'll get to
another day). It is the source of a great deal of confusion. It would have
been far better if "each" passed the DOM element as an argument to the inner
function, instead of using "this":
/* Non-working code as an example of what could have been */
jQuery.fn.toggleVis = function() {
return this.each( function( element ) {
if (element.style.visibility == 'hidden') {
element.style.visibility = 'visible';
} else {
element.style.visibility = 'hidden';
}
});
};
To me at least, that code is *much* easier to follow.
Now it does turn out that the DOM element is passed as an argument to the
"each" inner function, but it's the second argument, not the first. The
first argument is the array index (0 through n). That's unfortunate, since
you rarely need the index but always need the element. But at least you can
code:
jQuery.fn.toggleVis = function() {
return this.each( function( index, element ) {
if (element.style.visibility == 'hidden') {
element.style.visibility = 'visible';
} else {
element.style.visibility = 'hidden';
}
});
};
That capability didn't exist in the first versions of jQuery - it was added
somewhat later - so you'll often see another approach in existing jQuery
code:
jQuery.fn.toggleVis = function() {
return this.each( function() {
var element = this;
if (element.style.visibility == 'hidden') {
element.style.visibility = 'visible';
} else {
element.style.visibility = 'hidden';
}
});
};
-Mike
From: Rob Desbois
Within the function passed to .each(), 'this' refers to the
actual DOM element we are currently dealing with, so that
part of the code is now alright.
It still won't allow you to chain function calls though, so
one final touch is to return the result of .each(), and we're done:
jQuery.fn.toggleVis = function() {
this.each(function() {
if (this.style.visibility == 'hidden') {
this.style.visibility = 'visible';
} else {
this.style.visibility = 'hidden';
}
});
};
Haha, I just couldn't help myself, let's shorten the code and not type
the same three times:
jQuery.fn.toggleVis = function() {
return this.each(function() {
var visibility = this.style.visibility;
visibility = visibility == 'hidden' ? 'visible' : 'hidden';
});
};
--Klaus
