Can I improve this jQuery code replacement code?

Question

HTML looks like:

<dl> <dt> <img src='Images\Something\expand-close.gif' /> Something 1 </dt> <dd>Something 1 Text</dd> </dl> 

This HTML is repeated 1 or more times, so there may be many instances of HTML on the same page.

The jquery that I use to expand dd and replace the image:

 $("dl").find("dt").click(function() { // toggle the dd $(this).next().toggle(200); // replace the expand / close image var img = $(this).find("img"); var src = img.attr("src"); if (src.lastIndexOf("open.gif") != -1) { img.attr("src", src.replace("open.gif", "closed.gif")); } else { img.attr("src", src.replace("closed.gif", "open.gif")); } }); 

All this works great and does exactly what I need. My question is: can a jQuery function be executed better? I'm relatively new to using jQuery, and this was the snippet that I raked. If this could be done better, explain the changes as I am trying to learn how to write jQuery code. Any other pointers or suggestions would also be appreciated.

Answer 1

This is pretty close. Try:

 $("dl > dt").click(function() { var dd = $(this).next(); if (dd.is(":visible")) { var newimage = "closed.gif"; dd.hide(200); } else { var newimage = "open.gif"; dd.show(200); } var img = $(this).find("img"); img.attr("src", img.attr("src").replace(/(open|closed)\.gif$/, newimage); }); 

The differences are as follows:

• Just use "dl> dt" (or "dl dt" if necessary) instead of the find syntax in this situation;
• This code scans if a thing is visible or not, rather than an src image that could potentially go out of sync, especially considering the delay;
• The regular expression on the last line simply replaces everything that is, what is right, to serve irregularities.

Answer 2

You can set the ID or Class attribute to the image tag, which will clear your code a bit. I would also recommend using a plugin - there are many plugins for this free online version, and all that is required is one of the methods.

HTML:

 <dl> <dt> <img src='Images\Something\expand-close.gif' id='myimage' /> Something 1 </dt> <dd>Something 1 Text</dd> </dl> 

JavaScript (using jQuery):

 $('#myimage').click(function(){ $(this).parent().get(0).toggle(200); var src = $(this).attr('src'); if (src.lastIndexOf("open.gif") != -1) { $(this).attr('src', src.replace("open.gif", "closed.gif"); } else { $(this).attr('src', src.replace("closed.gif", "open.gif"); } }); 

Hope this helps a bit :)

Answer 3

You can play with the settings, but here is a general idea, a combination of CSS and JS ...

 <style> dl dt { background: transparent url(open.gif) no-repeat scroll top left; padding-left: 20px; } .open dt { background-image: url(close.gif); } </style> <dl class="open"> <dt>Something 1</dt> <dd>Something 1 Text</dd> </dl> <script> $('dl dt').click(function() { $(this).next().toggle(200); $(this).parent().toggleClass("open",$('dd:visible',$(this).parent()).length); }); </script> 

Answer 4

 <dl> <dt class="something"> <img class="switch" src='Images\Something\expand-close.gif' /> Something 1 </dt> <dd>Something 1 Text</dd> </dl> $('.something').click(function() { // toggle the dd $(this).next().toggle(200); // replace the expand / close image var img = $(this).find('.switch'); var src = img.attr('src'); if(src.lastIndexOf('open.gif') != -1) img.attr('src', src.replace('open.gif', 'closed.gif')); else img.attr('src', src.replace('closed.gif', 'open.gif')); });