Naturally, you wouldn't put it all on one line like that. The code probably
looks more like:

> $('div.myclass img[src$=png]').click(function(){
>     $(this).parents('div.myclass').hide('slow');
> });

The first thing I'd ask is why you're doing the .hide('slow'). Sometimes
people use that and .show('slow') just because they're sort of the "default"
jQuery animations. But that 2D resizing can be really annoying when it
causes text to reflow. You can see an example of this on the jQuery home
page, where the Run link animation reflows as it resizes. It would be
cleaner if it used .slideDown('slow').

An even better example is one of the jQuery date picker plugins, which uses
a .show('slow') when you open it - causing the calendar elements to shift
around as the animation changes width. (Apologies to the plugin author - I
don't remember which plugin it was, but it looked great otherwise. Trust me,
try .slideDown/Up instead of .show/hide and it will be a nice improvement.)

Just food for thought. Since I haven't seen what else is in those divs, nor
any of the other context, I'll assume that .hide('slow') is actually what
you want.

I also assume you wouldn't have one div.myclass nested inside another, would
you? You might get some surprising results if you did. If the answer is
"What a silly question. Of course not!", keep in mind that we haven't see
your HTML code. :-)

Next, how many div.myclass elements are on the page and how many PNGs are in
each one? If there are very many of either, you'll be installing a lot of
event handlers. You could speed up the code with event delegation.

In fact, I think this particular code is easier to read with event
delegation. You don't need a delegation plugin or anything, the "bare metal"
version (buffed to a shine with jQuery) happens to read very nicely:

    $('div.myclass').click( function( event ) {
        if( $(event.target).is('img[src$=png]') )
            $(this).hide('slow');
    });

This will perform very well if there aren't too many div.myclass elements,
even if they contain a very large number of PNGs. It does install a click
handler for each div.myclass element, so if there were a large number of
those elements, then I'd put a single click event on some outer container
element. But that code will end up being a lot less elegant than this
version, so unless there's a performance issue, I'd go with this one.
(Assuming it works, of course - this is untested code after all.)

So, what *does* it do, anyway? :-)

-Mike

Reply via email to