1
\$\begingroup\$

I feel like this bit of code could have been written more elegantly, especially with the huge if/else statements. Can somebody help me break this down? It would really help me out in the future.

// Contact
var c = 0;
var contactHeight = $('#contact-keebs').outerHeight();
$('#mail-wrap').click(function (e) {

    e.preventDefault();

    if (c++ % 2 == 0) {

        $('#contact-button').addClass('project-button').css('width', '71px').text('Projects');
        $('.mail-icon').attr('src', site.theme_path + '/img/icon-projects.png');
        $('#contact-button').shuffleLetters({callback:function(){
            if ($(window).scrollTop() != 0) {
                $('html, body').animate({
                    scrollTop : 0
                },200, function() {
                    $('#contact-keebs').fadeIn(200);
                    $('body').css({
                        height   : contactHeight,
                        overflow : 'hidden'
                    });
                    $('#contact-info, #clients').addClass('animated fadeInUp');
                });
            } else {
                $('#contact-keebs').fadeIn(200);
                $('body').css({
                    height   : contactHeight,
                    overflow : 'hidden'
                });
                $('#contact-info, #clients').addClass('animated fadeInUp');
            }
        }
        });

    } else {

        $('#contact-button').removeClass('project-button').css('width', '96px').text('Get in touch');
        $('.mail-icon').attr('src', site.theme_path + '/img/icon-contact.png');
        $('#contact-button').shuffleLetters({callback:function(){
            if ($(window).scrollTop() != 0) {
                $('html, body').animate({
                    scrollTop : 0
                },200, function() {
                    $('#contact-info, #clients').removeClass('animated fadeInUp');
                    $('body').css({
                        height   : '',
                        overflow : ''
                    });
                    $('#contact-keebs').fadeOut(200);
                });
            } else {
                $('#contact-info, #clients').removeClass('animated fadeInUp');
                $('body').css({
                    height   : '',
                    overflow : ''
                });
                $('#contact-keebs').fadeOut(200);
            }
        }
        });
    }
});

HTML

// Button

<div id="mail-wrap">
    <div class="icon-wrap">
        <img class="mail-icon" src="img/icon-contact.png" alt="Contact" height="26" width="24">
    </div>
    <a id="contact-button" href="#">Get in touch</a>
</div>

// Contact section

<div id="contact-keebs">
    <span class="contact-overlay"></span>
    <div id="contact-wrapper">
        <div id="contact-info">
            <h1>Title statement</h1>
            <h2>Email</h2>
        </div>
        <div id="clients">
            <h2>Clients &amp; Partners</h2>
            <ul id="client-list">
                <li>Client 1</li>
                <li>Client 2</li>
                <li>Client 3</li>
                <li>Client 4</li>
                <li>Client 5</li>
                <li>Client 6</li>
                <li>Client 7</li>
                <li>Client 8</li>
            </ul>
        </div>
    </div>
</div>
\$\endgroup\$
1
\$\begingroup\$
// Contact
var contactHeight = $('#contact-keebs').outerHeight();

function in_keebs() {
    $('#contact-keebs').fadeIn(200);
    $('body').css({
        height   : contactHeight,
        overflow : 'hidden'
    });
    $('#contact-info, #clients').addClass('animated fadeInUp');
}

function out_keebs() {
    $('#contact-info, #clients').removeClass('animated fadeInUp');
    $('body').css({
        height   : '',
        overflow : ''
    });
    $('#contact-keebs').fadeOut(200);
}

function show_project_button(e) {
    e.preventDefault();

    $('#contact-button').addClass('project-button').css('width', '71px').text('Projects');
    $('.mail-icon').attr('src', site.theme_path + '/img/icon-projects.png');
    $('#contact-button').shuffleLetters({callback:function(){
        if ($(window).scrollTop() != 0) {
            $('html, body').animate({
                scrollTop : 0
            },200, in_keebs);
        } else {
            in_keebs();
        }
    }
    });

    $('#mail-wrap').click(hide_project_button);
}

function hide_project_button(e) {
    e.preventDefault();

    $('#contact-button').removeClass('project-button').css('width', '96px').text('Get in touch');
    $('.mail-icon').attr('src', site.theme_path + '/img/icon-contact.png');
    $('#contact-button').shuffleLetters({callback:function(){
        if ($(window).scrollTop() != 0) {
            $('html, body').animate({
                scrollTop : 0
            },200, out_keebs);
        } else {
            out_keebs();
        }
    }
    });

    $('#mail-wrap').click(show_project_button);
}

$('#mail-wrap').click(show_project_button);

First, I'm not at all convinced that these are the right function names, but I needed names and these at least seem somewhat related. Part of the problem is that I don't really understand what you're doing, as I don't know what the underlying HTML is.

The insight to get rid of the outer if/else is that you are simply alternating between them. Instead of using a variable for tracking, we change the onclick behavior at the end of each variant. We pick one and start with it. This gives us the same alternating behavior as the if/else did. We can also get rid of c.

We're still stuck with the inner if/else blocks, but we can remove the repeated code into their own functions.

Note that I haven't tested this, as I don't have enough context to run it.

\$\endgroup\$
  • \$\begingroup\$ Thank you. I'm going over the code now. Also, I updated the original post with the HTML as well as a live example for you if it helps at all. Thanks again, I will try to understand your code. \$\endgroup\$ – Desi Jan 10 '15 at 1:52
  • \$\begingroup\$ One thing I noticed was that when I click on the button once, then click it again, the 'Get in Touch' text is supposed to appear. However, it still stays on 'Projects' and #contact-wrapper doesn't fade out. \$\endgroup\$ – Desi Jan 10 '15 at 2:10
  • \$\begingroup\$ @Desi Do you have a test page where the JavaScript is not minimized? \$\endgroup\$ – Brythan Jan 10 '15 at 3:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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