开发者

How can I not repeat this code in jquery?

I am repeating an IF ELSE statement like so

 $('#accountLoginButton').click(function() {




     if($('#topSubscribe').is(":visible")) {

         $('#topSubscribe').slideUp(function(){


            if ($('#topLogin').is(":hidden"))
            {
                 $('#topLogin').slideDown("fast");
            } else {
                 $('#topLogin').slideUp("fast");
            }

         });

     } else {

         if ($('#topLogin').is(":hidden"))
            {
                 $('#topLogin').slideDown("fast");
            } else {
                 $('#topLogin').slideUp("fast");
            }

     }




 });



 $('#subscribeTopButton').click(function() {

     if($('#topLogin').is(":visible")) {

         $('#topLogin').slideUp(function(){


            if ($('#topSubscribe').is(":hidden"))
            {
                 $('#topSubscribe').slideDown("fast");开发者_运维技巧
            } else {
                 $('#topSubscribe').slideUp("fast");
            }

         });

     } else {

         if ($('#topSubscribe').is(":hidden"))
            {
                 $('#topSubscribe').slideDown("fast");
            } else {
                 $('#topSubscribe').slideUp("fast");
            }

     }


 });

Basically 2 buttons operating like tabs to show/hide stuff.

As you can see I literally have the same code being repeated in a few different ways a couple times. I have a feeling i could somehow get this down to a few lines of code but my javascript understanding is a bit shady in general.

How do I trim this down the most?


You could use slideToggle instead of checking for visibility and then using slideDown or slideUp.

You can replace:

if ($('#topLogin').is(":hidden"))
{
     $('#topLogin').slideDown("fast");
} else {
     $('#topLogin').slideUp("fast");
}

with:

$('#topLogin').slideToggle("fast");

This should allow you to get rid of a lot of repetition in your code.


You could try something like this:

function handleVisibilityOf( el, otherEl ) {
  if( el.is(':visible') ) {
    el.slideUp( function() {
      otherEl.slideToggle();
    } );
  }
  else {
    otherEl.slideToggle();
  }
}

$('#accountLoginButton').click( function() {
  handleVisibilityOf( $('#topSubscribe'), $('#topLogin') );
} );

$('#subscribeTopButton').click( function() {
  handleVisibilityOf( $('#topLogin'), $('#topSubscribe') );
} );


You can put that code-block into a function and call it in both locations as needed.


Try refactoring it to smaller functions, then you should be able to see duplication more easily.


You're merely running to much code inside your if block. Try this instead:

$('#accountLoginButton').click(function() {
    if($('#topSubscribe').is(":visible")) {
        $('#topSubscribe').slideUp();
    }
    $('#topLogin').slideToggle('fast');
}

Or you can, as several others have proposed, factor out the repeated code into a separate function:

$('#accountLoginButton').click(function() {
    if($('#topSubscribe').is(":visible")) {
        $('#topSubscribe').slideUp(function(){
            $('#topLogin').slideToggle('fast');
        });
    } else {
        $('#topLogin').slideToggle('fast');
    }
});

EDIT: Using .slideToggle() instead of the if block.


function slideBoth(elm1, elm2){
  if(elm1.is(":visible")){
    elm1.slideUp(function(){
      elm2.slideToggle("fast");
    }
  }else{
    elm2.slideToggle("fast");
  }
}


$('#accountLoginButton').click(function() {
  slideBoth($("#topSubscribe"), $("#topLogin"));
}



$('#subscribeTopButton').click(function() {
  slideBoth($("#topLogin"),$("#topSubscribe"));
}
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜