HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Clickable icon for expanding/collapsing content

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
iconexpandingclickableforcontentcollapsing

Problem

The general idea of the code is that I have a + or - icon (".trigger") that is click-able and will expand or collapse the content (".cont") directly following it (there are many of there expand/collapse pairs). I also have a span("#expandAll") that, when clicked, will expand or collapse every block.

Please review my first usage of jQuery and help me put my head into a more jQuery state. Where can the code be improved? Are there any problems with it? Is there a better way to do it?

(To be perfectly honest, I have no idea what the $() function really does.)

var expandAll = function () {
    $(".cont").removeClass("hid");
    $("#expandAll").html("Colapse All");
    $("#expandAll").unbind("click", expandAll);
    $(".trigger").html("-");
    $("#expandAll").click(colapseAll);
}
var colapseAll = function () {
    $(".cont").addClass("hid");
    $("#expandAll").html("Expand All");
    $("#expandAll").unbind("click", colapseAll);
    $(".trigger").html("+");
    $("#expandAll").click(expandAll);
}
$(document).ready(function () {
    $(".trigger").click(function (event) {
        $this = $(this);
        $this.html($this.text() == '+' ? '-' : '+');
        $($this.nextAll(".cont").get(0)).toggleClass("hid");
    });
    $("#expandAll").click(expandAll);
});


Please let me know if you need more/less info.

Solution

I've refactored your code and added comments to explain certain things.

// We can shorten this from document.ready(...) to $(...) 
// Internally, jQuery will add the passed in function to a list
// of handlers that will be invoked when the document is ready.
$(function() {
    // Since the $ function will query the document with the 
    // specified selector we want to cache these.
    // Doing this from within the ready callback allows us 
    // to keep these references hidden.
    // I've also added the 2 function variables to keep them
    // hidden as well.
    var expandAll, colapseAll,
        expandAllElement = $("#expandAll"),
        contElements = $(".cont"),
        triggerElements = $(".trigger");

    expandAll = function() {
        contElements.removeClass("hid");
        triggerElements.html("-");
        // Notice how we can chain these calls together?
        // jQuery was designed with a fluent interface.
        expandAllElement.html("Colapse All").one("click", colapseAll);
    };

    colapseAll = function() {
        contElements.addClass("hid");
        triggerElements.html("+");
        expandAllElement.html("Expand All").one("click", expandAll);
    };

    triggerElements.click(function(event) {
        // Always define a variable so the 'name' 
        // is not defined in the global object,
        var $this = $(this);
        $this.html($this.text() == '+' ? '-' : '+');
        $($this.nextAll(".cont").get(0)).toggleClass("hid");
    });

    // Using the one function allows you to skip 
    // unbinding the event handler for each click.
    expandAllElement.one("click", expandAll);
});

Code Snippets

// We can shorten this from document.ready(...) to $(...) 
// Internally, jQuery will add the passed in function to a list
// of handlers that will be invoked when the document is ready.
$(function() {
    // Since the $ function will query the document with the 
    // specified selector we want to cache these.
    // Doing this from within the ready callback allows us 
    // to keep these references hidden.
    // I've also added the 2 function variables to keep them
    // hidden as well.
    var expandAll, colapseAll,
        expandAllElement = $("#expandAll"),
        contElements = $(".cont"),
        triggerElements = $(".trigger");

    expandAll = function() {
        contElements.removeClass("hid");
        triggerElements.html("-");
        // Notice how we can chain these calls together?
        // jQuery was designed with a fluent interface.
        expandAllElement.html("Colapse All").one("click", colapseAll);
    };

    colapseAll = function() {
        contElements.addClass("hid");
        triggerElements.html("+");
        expandAllElement.html("Expand All").one("click", expandAll);
    };

    triggerElements.click(function(event) {
        // Always define a variable so the 'name' 
        // is not defined in the global object,
        var $this = $(this);
        $this.html($this.text() == '+' ? '-' : '+');
        $($this.nextAll(".cont").get(0)).toggleClass("hid");
    });

    // Using the one function allows you to skip 
    // unbinding the event handler for each click.
    expandAllElement.one("click", expandAll);
});

Context

StackExchange Code Review Q#830, answer score: 15

Revisions (0)

No revisions yet.