5
\$\begingroup\$

You are first confronted by three links. Each link triggers divs to slide out.

The divs slide out and up to appear as if unfolding. The slide back in reverse when any other link on the page is clicked. The code works exactly how we want it to. However, the code is very long and needs re-developing into a neater script.

To also help further here is the website this script belongs to demo website

$('#menu').click(function () {
    if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#fatal').css('display') == 'none' ) {
        $('#fatal').animate({ left: 'toggle' }, 300, function() {
            $('#igna, #black').slideToggle("fast");   
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }
    if ($('#bio-line-1').css('display') != 'none') {
        $('#bio-line-2').slideUp("slow");

        window.setTimeout(function () {
            $('#bio-line-1').animate({ width: 'hide' });
        }, 300);
    }

   $('#contact-info').animate({right: 'hide'});

   $('#black-div, #black-credits, #igna-1-div, #igna-1-credits, #igna-2-div, #igna-2-credits, #fatal-div, #fatal-credits').fadeOut('100');

});
$('#igna').click(function () {
    if ($('#igna-2').css('display') == 'none') {
        $('#igna-2').animate({ left: 'show' }, 300, function() {
            $('#igna-1').slideToggle("fast");
        });
    }

    else {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300);   
        });
    }

   $('#bio-line-1').animate({width: 'hide'});
   $('#contact-info').animate({right: 'hide'});
   $('#bio-line-2').slideUp("fast");
   $('#black-div, #black-credits, #igna-1-div, #igna-1-credits, #igna-2-div, #igna-2-credits, #fatal-div, #fatal-credits').fadeOut('100');
});

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

    $('#contact-info').animate({right: 'toggle'});

   if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#igna-1').css('display') == 'none' ) {
          $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, 300);  
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }
    if ($('#bio-line-1').css('display') != 'none') {
        $('#bio-line-2').slideUp("slow");

        window.setTimeout(function () {
            $('#bio-line-1').animate({ width: 'hide' });
        }, 300);
    }
  $('#black-div, #black-credits, #igna-1-div, #igna-1-credits, #igna-2-div, #igna-2-credits, #fatal-div, #fatal-credits').fadeOut('100');


        });

$('#name-a').click(function () {
    if ($('#bio-line-1').css('display') == 'none') {
        $('#bio-line-1').animate({ width: 'toggle' });
        window.setTimeout(function () {
            $('#bio-line-2').slideToggle("slow");
        }, 100);
    }
    else {
        $('#bio-line-2').slideToggle("slow");

        window.setTimeout(function () {
            $('#bio-line-1').animate({ width: 'toggle' });
        }, 300);
    }
    if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#igna-1').css('display') == 'none' ) {
          $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, 300);  
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }


  $('#contact-info').animate({right: 'hide'});

  $('#black-div, #black-credits, #igna-1-div, #igna-1-credits, #igna-2-div, #igna-2-credits, #fatal-div, #fatal-credits').fadeOut('100');
});
$("#black").click(function(){
    if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#igna-1').css('display') == 'none' ) {
          $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, 300);  
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }

  $('#black-div').fadeIn('500');
  $('#black-credits').fadeIn('3000');
  $('#dust-div, #youth-div, #fatal-div, #hurt-div, #igna-1-div, #igna-2-div, #igna-1-credits, #igna-2-credits, #fatal-credits').fadeOut('100');
});

$("#igna-1 a").click(function(){
    if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#igna-1').css('display') == 'none' ) {
          $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, 300);  
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }

  $('#igna-1-div').fadeIn('500');
  $('#igna-1-credits').fadeIn('3000');
  $('#dust-div, #black-div, #fatal-div, #hurt-div, #youth-div, #black-credits, #igna-2-div, #igna-2-credits, #fatal-credits').fadeOut('100');
});
$("#igna-2 a").click(function(){
    if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#igna-1').css('display') == 'none' ) {
          $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, 300);  
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }

  $('#igna-2-div').fadeIn('500');
  $('#igna-2-credits').fadeIn('3000');
  $('#dust-div, #black-div, #fatal-div, #hurt-div, #youth-div, #black-credits, #fatal-credits, #igna-1-div, #igna-1-credits,').fadeOut('100');
});
$("#fatal").click(function(){

   if ( $('#igna-1').css('display') != 'none' ) {
        $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });   
        });
    } else if ( $('#igna-1').css('display') == 'none' ) {
          $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, 300);  
        });

    } else {
        $('#black, #igna').slideUp("fast", function() {
            $('#fatal').animate({ left: 'hide' }, 300);
        });
    }
  $('#fatal-div').fadeIn('500');
   $('#fatal-credits').fadeIn('3000');
  $('#dust-div, #youth-div, #black-div, #hurt-div, #igna-1-div, #igna-2-div, #black-credits, #igna-1-credits, #igna-2-credits,').fadeOut('100');
});
\$\endgroup\$
2
  • 6
    \$\begingroup\$ Not related to your question but you should really cache your selectors. Selecting elements is one of jQuerys slowest operations. So cache it at the start var $igna = $('#igna'); and just use $igna every where else. \$\endgroup\$ Commented May 29, 2015 at 16:37
  • 1
    \$\begingroup\$ Your demo website link is broken. \$\endgroup\$
    – Mast
    Commented Jun 5, 2015 at 11:33

2 Answers 2

11
+100
\$\begingroup\$

Gary Storey's comment that selecting elements every time you need them is probably the slowest part of your code is right but given that everything involves timed animations you probably won't notice any change from replacing them.

The main issue that stands out is that there is a lot of repeated code.

This bit appears 20 times:

$('#black, #igna').slideUp("fast", function() {
      $('#fatal').animate({ left: 'hide' }, 300);  
});

You could replace this with a single function which would make your code easier to read and that would help with the process of finding other opportunities for simplification. If you feel like you might in future want to tweak the animation times for different elements you could do something like this:

function showBlackAndIgna(T){
    $('#black, #igna').slideUp("fast", function() {
          $('#fatal').animate({ left: 'hide' }, T);  
    });
}

Some of your if statements have unnecessary else if conditions. For example:

if ( $('#igna-1').css('display') != 'none' ) {
    $('#igna-1').slideToggle("fast", function() {
        $('#igna-2').animate({ left: 'hide' }, 300, function() {
            showBlackAndIgna(300) 
        });   
    });
} else if ( $('#igna-1').css('display') == 'none' ) {
    showBlackAndIgna(300)

} else {
    showBlackAndIgna(300)
}

In the above example I replaced the previous block of code with showBlackAndIgna(300) and it is now much easier to see that the else if is redundant.

You have several blocks that look something like this:

$('#black-div').fadeIn('500');
$('#black-credits').fadeIn('3000');
$('#dust-div, #youth-div, #fatal-div, #hurt-div, #igna-1-div, #igna-2-div, #igna-1-credits, #igna-2-credits, #fatal-credits').fadeOut('100');

Assuming the same elements are involved each time and are either fading in or out, you might be able to replace them with a function something like this:

function fadeMany(in1,in2){
    var elementList=['#black-div','#black-credits','#dust-div'];
    $(in1).fadeOut(500);
    $(in2).fadeOut(3000);
    elementList.forEach(function(elSelector){
        if(elSelector!=in1 && elSelector!=in2) $(elSelector).fadeOut('100');
    });
}

You would need to work out which elements are involved and extend the elementList array to include everything that is always fading out if not fading in.

There were a few other minor points unrelated your code:

  1. The contact email address didn't display properly, but using CONTACT<b>@</b>XXXXXXXXXX.COM seemed to fix it and look more or less the same.
  2. I found the 'Work' sub menu disappearing when one of its items were clicked a bit annoying. Personally I would leave it visible to make navigation a little less tedious.
  3. That there was extra content off the bottom of the screen wasn't immediately obvious. It was only when I noticed that a scroll bar had appeared did I realise that scrolling down reveals more pictures.

I'm not sure exactly what is causing the problem in point 1. but if you were hoping to hide the email address from bots you will need to remove the href and use JS to add it when the page loads. Point 3 almost is certainly due to my screen size only fitting exactly one picture at at time, something you may not be aware of if you have a larger screen.

\$\endgroup\$
1
  • \$\begingroup\$ Very good. Needless code replication is a major maintainability problem. \$\endgroup\$
    – Mast
    Commented Jun 5, 2015 at 11:36
7
\$\begingroup\$

To ignore having nested if ....elseifs you can use a registry/resolver container (for example an array of objects that have a function to check conditions and a function that does the actual action) then you can ask for the first object that meets the current conditions and ask it to do its job, something like this:

var actionResolvers=[
{ 
   condition=function (){return $('#igna-1').css('display') != 'none' ;},
   action=function(){
           $('#igna-1').slideToggle("fast", function() {
            $('#igna-2').animate({ left: 'hide' }, 300, function() {
                $('#black, #igna').slideUp("fast", function() {
                    $('#fatal').animate({ left: 'hide' }, 300);
                });
            });  
   }
},
{ 
     //other objects with same members
}
];

Then you can do something like this:

for(var i=0;i<actionResolvers;i++)
{
    if(actionResolvers[i].condition())
    {
       actionResolvers[i].action();
       break;

    }
}

It's needless to say that you can have named functions instead of anonymous ones and thus you can DRY. Besides this code complies with SOLID open/closed principle because it is open to improvement/extension (adding a new action resolver to the array and closed to modification (adding another if at the end of nested ifs).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.