3
\$\begingroup\$

I am a beginner in JavaScript and starting to understand jQuery. I've written a toolbar that uses jQuery. I've also put comments based on what I understand of how the code works, though through the help of the book, it helps. But I would really appreciate if someone can review the code as well as the comments, so that I will know if what I'm doing is correct.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>JQUERY TOOLBAR</title>
    <!-- all stylesheet files -->
    <style>
        .tabStrip 
{
    background-color: #E4E2D5;
    padding: 3px;
    height: 22px;
}

.tabStrip-tab
{
    float: left;
    font: 14px arial;
    cursor: pointer;
    padding: 2px;

    border: 1px solid transparent;
}

.tabStrip-tab-hover
{
    border-color: #316AC5;
    background-color: #f9e391;
}


    </style>
</head>
<body>

<div class="tabStrip">
<div id="tabStrip-tab-1" class="tabStrip-tab">Tab 1</div>
<div id="tabStrip-tab-2" class="tabStrip-tab">Tab 2</div>
<div id="tabStrip-tab-3" class="tabStrip-tab">Tab 3</div>
</div>
<div id="descContainer"></div>

<!-- all script files -->
<script src="assets/js/jq.js"></script>
<script>



$(function() {

// keep track of the active tab's number

var tabActiveNum = 0;

// named function that will catch the event to occur
function handleEvent(e)
{
    // create referencing for both IE and W3C Dom
    var el = $(e.target);

    // Determine if the mouse was moved over or out of an element
    if ( e.type == "mouseover" || e.type == "mouseout" )
    {
        // 1. make sure to toggle correctly the tabstrip-tab-hover
        // 2. make sure that we don't add the tabstrip-tab-click, whenever
        // we hover to the tab 
        if ( el.hasClass("tabStrip-tab") && !el.hasClass("tabStrip-tab-click")  )
        {
                // make sure that tabStrip-tab-hover class toggled correctly
                el.toggleClass("tabStrip-tab-hover");
        }
    }

    // Determine when we click the mouse on the tab
    if ( e.type == "click" )
    {
        if ( el.hasClass("tabStrip-tab-hover") )
        {
            // make sure to track the tab index,
            var id = e.target.id;
            var num = id.substr(id.lastIndexOf("-") + 1);

            // make sure that the tab index is the same
            // as the tab content, if its true
            if ( tabActiveNum != num )
            {
                deactivateTab();

                // remove the tabstrip-tab-hover css
                // and add the tabstrip-tab-click to the element
                el.toggleClass("tabStrip-tab-hover")
                    .toggleClass("tabStrip-tab-click");

                // show the current number and tab description
                showDescription(num);
                // the num result set it to tabActiveNum
                tabActiveNum = num;

            }
        }
    }
}

// this function will show the description
    // of the current active tab
    function showDescription(num)
    {
        // create a div element
        var div = $(document.createElement("div"));

        // and append it to the div element with an id
        // of descContainer
        $("#descContainer").append
        (
            div.attr("id", "tabStrip-desc-" + num)
                .text("Description for tab" + num)
        );
    }

    function deactivateTab()
    {   
        // ensure we won't try to remove a nonexistent
        // object from DOM.
        var descEl = $("#tabStrip-desc-" + tabActiveNum);

        // if an element was found, use the jQuery's remove()
        // to remove the element fronm the DOM as follows
        if ( descEl.length > 0)
        {   

            descEl.remove();

            // remove the tabStrip-tab-click css
            $("#tabStrip-tab-" + tabActiveNum).toggleClass("tabStrip-tab-click");

        }
    }


$(document).bind("click mouseover mouseout", handleEvent);


});


</script>
</body>
</html>
\$\endgroup\$
1
\$\begingroup\$

Binding click mouseover mouseout to a single handleEvent function and then using if-else to distinguish the events is just silly. Split the handling to multiple functions, for example handleClick and handleMouseOverOrOut, without if-else, and bind separately:

$(document).bind("click", handleClick);
$(document).bind("mouseover mouseout", handleMouseOverOrOut);

The result will be simpler and better.

The indentation is a bit messy, and you have a couple of empty lines at odd places.

| improve this answer | |
\$\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.