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 Answer 1

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\$
3
  • \$\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, 2015 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, 2015 at 2:10
  • \$\begingroup\$ @Desi Do you have a test page where the JavaScript is not minimized? \$\endgroup\$
    – Brythan
    Jan 10, 2015 at 3:14

Your Answer

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

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