views:

67

answers:

3

I have 4 blocks of jQuery that look like this:

$('#aSplashBtn1').click(function(){
  $('#divSliderContent div').hide();
  $('#divSplash1').fadeIn('slow');
  return false;
});
$('#aSplashBtn2').click(function(){
  $('#divSliderContent div').hide();
  $('#divSplash2').fadeIn('slow');
  return false;
});
$('#aSplashBtn3').click(function(){
  $('#divSliderContent div').hide();
  $('#divSplash3').fadeIn('slow');
  return false;
});
$('#aSplashBtn4').click(function(){
  $('#divSliderContent div').hide();
  $('#divSplash4').fadeIn('slow');
  return false;
});

I've tried learning more about javascript arrays and for loops but when I try to implement it into this code it only ends up working for the number 1 block. Could someone show me how they would accomplish optimizing this?

+2  A: 

A variation on Sosh's answer

$('#aSplashBtn1').click(hideAndFadeIn('#divSplash1'));
$('#aSplashBtn2').click(hideAndFadeIn('#divSplash2'));
$('#aSplashBtn3').click(hideAndFadeIn('#divSplash3'));


function hideAndFadeIn(splash){
    return function() {
        $('#divSliderContent div').hide();
        $(splash).fadeIn('slow');
        return false;
    };
}
cobbal
Yes, this is better - use this one! :)
UpTheCreek
Wouldn't you also want to accept the `e` parameter on your anonymous function and run `e.preventDefault();` to keep the page from jumping around? (I am assuming the aSplashBtns are `a` tags)
Doug Neiner
good point, I just put in `return false;` though to keep it equivalent to the OP
cobbal
A: 
var $divSlider = $("#divSliderContent div");
$('*[id^=aSplashBtn]').live('click', function(e){
    // Get the number from the id of the clicked element
    var id = this.id.match(/^aSplashBtn(\d+)$/)[1];

    $divSlider.hide();

    $("#divSplash" + id).fadeIn('slow');

    // Preferred as opposed to return false
    e.preventDefault();
});

This will set a single handler that will match every element with an id that starts with aSplashBtn. Your id's could go as high as you wanted (i.e. #aSpashBtn100) and it would still pair with the correct div#divSplash100.

Also, I cached #divSliderContent div in its own variable since you don't want jQuery to 'look it up' again on each click.

Doug Neiner
+1  A: 

If the clickable items are siblings, you can do:

$('#aSplashBtn1').siblings().andSelf().click(function(e){
    $('#divSliderContent div').hide();
    $('#divSplash'+e.target.id.substr(e.target.id.length-1)).fadeIn('slow');
    e.preventDefault();
});
David
`this` would also work in place of `e.target`
Doug Neiner