3
\$\begingroup\$

I made a navigation bar on the left side of my site. It works perfectly, the only problem is I'm new to jQuery and I think I'm repeating too much code. Is this the case? If it is please help me out.

HTML

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>DopEYEmine</title>

<!--Favicon-->
<link rel="shortcut icon" href="favicon.ico" type="image/x-icon">

<!--Fonts-->
<link href='http://fonts.googleapis.com/css?family=Special+Elite' rel='stylesheet' type='text/css'>

<!--CSS-->
<link rel="stylesheet" type="text/css" href="css/normalize.css"/>
<link rel="stylesheet" type="text/css" href="css/style.css"/>

</head>

<body>

    <div id="sidebar">
        <ul>
            <li><img class="logo" src="img/whitelogo.svg"><img class="logotwo" src="img/whitelonglogo.svg"></li>
            <li><img class="nav_icons" src="img/about.svg"><a class="about_btn" href="#">ABOUT</a></li>
            <li><img class="nav_icons" src="img/submit.svg"><a class="submit_btn" href="#">SUBMIT</a></li>
            <li><img class="nav_icons" src="img/contact.svg"><a class="contact_btn" href="#">CONTACT</a></li>
        </ul>
    </div>

    <div id="about">
        <div class="menucontent">
            <div class="aboutclose closer">CLOSE</div>
            <div>
                <img src="img/logo.png">
            </div>
            <p>"Dopamine: the molecule behind all our most sinful behaviors and secret cravings."</p>
            <p></p>
        </div>
    </div>

    <div id="submit">
        <div class="menucontent">
            <div class="submitclose closer">CLOSE</div>

            <form action="submitform.php" method="post" enctype="multipart/form-data">
                <input type="text" name="name" placeholder="NAME" />
                <input type="text" name="email" placeholder="E-MAIL">
            </form>

        </div>
    </div>

    <div id="contact">
        <div class="menucontent">
            <div class="contactclose closer">CLOSE</div>
        </div>
    </div>

    <div id="mainarea">

    </div>

<!--Javascript-->
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<script src="js/browser_hack.js" type="text/javascript"></script>
<script src="js/application.js" type="text/javascript"></script>

</body>
</html>

CSS

html, body {
    height: 100%;
    width: 100%;
    overflow: hidden;
}

a{
    text-decoration: none;
}

/*Side Bar Nav----------------------------------*/

.logo {
    position: absolute;
    top: 30px;
    left: 10px;
    width: 75px;
    margin: 0px;
}

.logotwo {
    position: absolute;
    top: 34px;
    left: 110px;
    width: 150px;
    display: none;
}

#sidebar {
    float: left;
    z-index: 10;
    position: relative;
    top: 0;
    left: 0;
    height: 100%;
    width: 95px;
    background: #555;
    overflow: hidden;
    border-right: solid 3px #444;

    -webkit-transition: all 0.5s ease;
    -moz-transition: all 0.5s ease;
    -ms-transition: all 0.5s ease;
    -o-transition: all 0.5s ease;
    transition: all 0.5s ease;
}

#sidebar:hover {
    width: 285px;
    background: #000;
    border-right: none;
}

.nav_icons {
    width: 40px;
    height: 40px;
}

#sidebar li {
    display: block;
    font-family: 'Special Elite', cursive;
}

#sidebar ul li:nth-child(2) {
    position: absolute;
    top: 88px;
    left: 27px;
}

#sidebar ul li:nth-child(3) {
    position: absolute;
    top: 148px;
    left: 27px;
}

#sidebar ul li:nth-child(4) {
    position: absolute;
    top: 208px;
    left: 27px;
}

.about_btn,
.submit_btn,
.contact_btn {
    position: relative;
    top: -25px;
    left: 80px;
    color: #fff;
    display: none;
    padding: 10px 200px 5px 5px;
}

.gecko .about_btn,
.gecko .submit_btn,
.gecko .contact_btn { 
    top: -35px;
}

#sidebar a:hover {
    background-color: #fff;
    color: #000;
}

/*Panel------------------------------------*/

.closer {
    float: right;
    font-weight: 900;
    font-size: .7em;
    padding: 5px;
    cursor: pointer;
}

.closer:hover {
    font-size: .75em;
}

.menucontent {
    width: 500px;
}

/*About------------------------------------*/

#about {
    float: left;
    z-index: 10;
    height: 100%;
    width: 500px;
    background-color: #fff;
    box-shadow: 3px 0px 2px #555;
    opacity: .7;
    display: none;
}

#about p {
    width: 400px;
    margin: 0 auto;
    text-align: left;
    font-family: 'Special Elite', cursive;
}

#about img {
    position: relative;
    top: 25px;
    left: 50px;
    display: block;
    width: 400px;
}

/*Contact------------------------------------*/

#contact {
    float: left;
    z-index: 10;
    height: 100%;
    width: 500px;
    background-color: #fff;
    box-shadow: 3px 0px 2px #555;
    opacity: .7;
    display: none;
}

/*Submit------------------------------------*/

#submit {
    float: left;
    z-index: 10;
    height: 100%;
    width: 500px;
    background-color: #fff;
    box-shadow: 3px 0px 2px #555;
    opacity: .7;
    display: none;
}

/*Main Area--------------------------------*/

#mainarea {
    float:left;
    display:block;
    position:relative;
    top: 0;
    right: 0;
    z-index: 1;
    height: 100%;
    width: 100%;
    padding-left: 100px;

    -webkit-box-sizing: border-box;
    -moz-box-sizing: border-box;
    -ms-box-sizing: border-box;
    -0-box-sizing: border-box;
    box-sizing: border-box;
}

jQuery

$(document).ready(function(){

//SIDEBAR-------------------------------------------------------
    $('#sidebar').hover(function(){
        $(".about_btn, .submit_btn, .contact_btn").delay(100).fadeIn(1000);
        $(".logotwo").delay(100).fadeIn(1000);
    },function(){
        $(".about_btn, .submit_btn, .contact_btn").fadeOut(10);
        $(".logotwo").fadeOut(10);
    });


//ABOUT PANEL---------------------------------------------------
    var fadeInAbout = function() {
        $('#about').animate({width: 'toggle'});
    };  
    $(".about_btn").click(function () {
        if($('#submit').css("display") == "block") {
            $('#submit').fadeOut(10);
            fadeInAbout();
        } else if($('#contact').css("display") == "block") {
            $('#contact').fadeOut(10);
            fadeInAbout();
        } else {
            fadeInAbout();
        }
    }); 
    $(".aboutclose").click(function () {
        fadeInAbout();
    });


//SUBMIT PANEL---------------------------------------------------
    var fadeInSubmit = function() {
        $('#submit').animate({width: 'toggle'});
    };
    $(".submit_btn").click(function () {
        if($('#about').css("display") == "block") {
            $('#about').fadeOut(10);
            fadeInSubmit();
        } else if($('#contact').css("display") == "block") {
            $('#contact').fadeOut(10);
            fadeInSubmit();
        } else {
            fadeInSubmit();
        }
    });
    $(".submitclose").click(function () {
        fadeInSubmit();
    });


//CONTACT PANEL---------------------------------------------------
    var fadeInContact = function() {
        $('#contact').animate({width: 'toggle'});
    };

    $(".contact_btn").click(function () {
        if($('#about').css("display") == "block") {
            $('#about').fadeOut(10);
            fadeInContact();
        } else if($('#submit').css("display") == "block") {
            $('#submit').fadeOut(10);
            fadeInContact();
        } else {
            fadeInContact();
        }
    }); 

    $(".contactclose").click(function () {
        fadeInContact();
    });

//---------------------------------------------------------------
});
\$\endgroup\$
  • \$\begingroup\$ try and separate code that is similar and see if you can combine it into one function instead of 2+ \$\endgroup\$ – Geo May 3 '14 at 2:09
  • \$\begingroup\$ @Geo show him, and you have a full-fledged CR-answer... \$\endgroup\$ – Vogel612 May 3 '14 at 12:18
2
\$\begingroup\$

I have a few suggestions.

1. Always cache your jquery objects if you are going to select the same thing more than once.

For example:

if($('#submit').css("display") == "block") {
    $('#submit').fadeOut(10);

Here you are querying the DOM twice to find the #submit element. Instead, save it to a variable and use that variable in both places:

var $submit = $('#submit');

if($submit.css("display") == "block") {
    $submit.fadeOut(10);

In fact, assuming the elements in question are not dynamically created/removed, you can add this variable assignment at the very top of your code (inside your document.ready) and use it throughout your code. This is only 1 single query instead of multiple with every click.

2. For code readability use .is(":visible") to check if an element is visible.

Instead of $submit.css("display") == "block", which I am assuming you are using to check to see if the element is display: none (hidden) vs display: block (visible), you can use $submit.is(":visible"). This is slightly more concise but more importantly better conveys what the code is doing.

3. If a function is called in all possible execution paths, call it after the if

In the following code (and other similar code), the function (fadeInContact in this case) is called in every possible execution path:

if($('#about').css("display") == "block") {
    $('#about').fadeOut(10);
    fadeInContact();
} else if($('#submit').css("display") == "block") {
    $('#submit').fadeOut(10);
    fadeInContact();
} else {
    fadeInContact();
}

In this case, the following is functionally equivalent and doesn't require you to put the function into every if/else branch:

if($('#about').css("display") == "block") {
    $('#about').fadeOut(10);
} else if($('#submit').css("display") == "block") {
    $('#submit').fadeOut(10);
}

fadeInContact();

4. Replace all fadeIn functions with just one.

You have three different functions which are identical with exception of the id of the element. One example:

var fadeInContact = function() {
    $('#contact').animate({width: 'toggle'});
};

All 3 of these could be replace by one with a parameter for the id:

var fadeInElement = function(id) {
    $('#' + id).animate({width: 'toggle'});
};

Then it would be called like this:

fadeInElement("contact");

5. Use chaining and filters to consolidate code.

While not 100% equivalent, making the assumption that only one of your elements is visible at a time, you can replace the following:

if($('#about').css("display") == "block") {
    $('#about').fadeOut(10);
} else if($('#submit').css("display") == "block") {
    $('#submit').fadeOut(10);
}

with

$("#about, #submit").filter(":visible").fadeOut(10);

In fact, fadeOut will implicitly do a check for visibility and only fade out if the element is visible. As a result, the filter can be excluded making it simply:

$("#about, #submit").fadeOut(10);

With all changes above incorporated, the following:

$(".about_btn").click(function () {
    if($('#submit').css("display") == "block") {
        $('#submit').fadeOut(10);
        fadeInAbout();
    } else if($('#contact').css("display") == "block") {
        $('#contact').fadeOut(10);
        fadeInAbout();
    } else {
        fadeInAbout();
    }
}); 

Becomes:

$(".about_btn").click(function () {
    $("#contact, #submit").fadeOut(10);

    fadeInElement("about");
}); 
\$\endgroup\$

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.