Jeff Voss
Jeff Voss

Reputation: 3695

Elegant jQuery functions

I am relatively new to jQuery and I was wondering if the below function(s) I did were produced the the most elegant manner possible? any info is greatly appreciated! thank you

 $(function(){
        $('#enter').click(function() {
            $('#enter').fadeOut(80, function () {
            $('#enter').hide( function () { 
            $('#overlay').fadeIn(1500, function () {
            $("#loading").show().delay(500);
            $("#loading").hide( function(){ $("#s5").show(); });
            return false;

        });
    });
    });
        $('#slideTop').animate({ 
            marginTop: '-=230'   
        }, 500, function() {
    });
        $('#slideBottom').animate({ 
            marginBottom: '-=333',
        }, 500, function() {
    });

        });
    });

should I have it begin something like this:

$(function(){
var cintro = function(){
    $('#box').click(function(){
        $(this).slideUp(
            {duration:1000}//2000,easing:"easeOutBounce"
        );
    setTimeout(function(){
        ('#box').slideUp(
            {duration:1000}//2000
        );}, 6000);
    //$('#box').css({'display': 'block'}).click(function(){$(this).css('display', 'none');
});
    $('#slidenav').slideDown({duration:2000,easing:"easeOutBounce"});
    $('#slider1').data('AnythingSlider').startStop(true);
}
$('#enter').click(cintro);
});

Upvotes: 0

Views: 203

Answers (2)

gnarf
gnarf

Reputation: 106392

$(function(){
  // cache some selectors that won't ever change
  var enter = $( '#enter' ),
      overlay = $( '#overlay' ),
      loading = $( '#loading' ),
      s5 = $( '#s5' ),
      slideTop = $( '#slideTop' ),
      slideBottom = $( '#slideBottom' );

  enter.click(function() {
    enter.fadeOut(80, function() {
      // i don't think you really need this .hide() but you might...
      enter.hide();
      overlay.fadeIn(1500, function() {
        loading.show().delay(500).hide('slow', function() {
          $("#s5").show();
        })
      });
    });
    return false;
  });

  slideTop.animate({ 
    marginTop: '-=230'   
  }, 500);
  slideBottom.animate({
    marginBottom: '-=333',
  }, 500);
});

Upvotes: 1

RSG
RSG

Reputation: 7123

I'm not a JQuery expert, but there are a few things I would change about this.

If I'm interacting with the same object from the DOM over and over I usually initialize it in a constant rather than select it every time:

var $ENTER_BUTTON = null;
...[other elements]...

$(document).ready(function(){
  $ENTER_BUTTON = $('#enter');
  ... [etc] ...
});

You also don't need to follow fadeOut with hide AFAIK. That's redundant.

If other animations could interfere with the transition use stop() appropriately.

Lastly, if any of these transitions can be invoked from other parts of the page, or if you want to make your code a bit more readable I would define a function and reference the function:

function enterAnimation(){
  $ENTER_BUTTON.fadeOut(80,function(){
  ... etc
}    

$(document).ready(function(){
  $ENTER_BUTTON = $('#enter');
  ... [etc] ...
  $ENTER_BUTTON.click(enterAnimation);
});    

Upvotes: 0

Related Questions