jQueryTools plugin custom effect for caching ctabs

Question

This is a custom effect for jQuery tools that enable caching for ajaxed tabs. Please advice on anything that I can improve. Note that I'm not sure where to put the loading indicator, so, at least have a look at that.

$.tools.tabs.addEffect("ajax_cache", function(tabIndex, done) {
 //check if content already loaded
 //if yes, display it, and hide the rest
 //if no, send ajax request
 var panes_cont = this.getPanes().eq(0);
 var dest_pane = panes_cont.find("#tabindex_"+tabIndex);
 if(dest_pane.length > 0){
 panes_cont.children().hide();
 dest_pane.show();
 } else {
 panes_cont.children().hide();
 panes_cont.append("<img id='tab_loading' src='/graphics/loading.gif' />");
 var new_pane = $('<div>').attr('id', 'tabindex_'+tabIndex).load(this.getTabs().eq(tabIndex).attr("href"),
 function(){
 panes_cont.find("#tab_loading").remove();
 panes_cont.append(new_pane.show());
 });
 }
 done.call();
});

• Bill Barry: Instead of giving the loading image an id and then traversing for it, you should make it a cached jquery object.

Answer 1

From a once over :

• Whether the content is loaded or not, you call panes_cont.children().hide();, you might as well centralize that 1 call
• There is too much going on in the creation of new_pane, it ought to be split up.
• lowerCamelCasing is good for you, also write out things. cont keeps reminding me of continue whereas you probably mean content
• I prefer the line of comment on top of the function
• There is no need to check length > 0, you can simply check for length
• As Bill Barry pointed out, caching the loader image is more efficient
• Most recent js standards suggest to put strings in single quotes, whichever way you go, you should be consistent for easy reading.

All the above together gives this:

//check if pane already loaded, if so, display it, otherwise send ajax
$.tools.tabs.addEffect("ajax_cache", function(tabIndex, done)
{
 var allPanes = this.getPanes().eq(0).hide(),
 targetPane = allPanes.find("#tabindex_"+tabIndex);
 if(targetPane.length)
 {
 targetPane.show();
 } 
 else 
 {
 var loaderImage = allPanes.append('<img id="tab_loading" src="/graphics/loading.gif"/>'),
 newPane = $('<div>').attr('id', 'tabindex_'+tabIndex),
 URL = this.getTabs().eq(tabIndex).attr('href');
 newPane.load(URL, function(){
 loaderImage.remove();
 allPanes.append(newPane.show());
 });
 }
 done.call();
});