Hide/show sidebar

Question

I am using this code for my responsive layout mobile sidebar hide/show. I am not an expert in jQuery and just wonder if I can optimize this code with the same functionality.

$(function() {
    var a = $("#sidepanelpull");
    var l = $("#sidepanelclose");
    side = $(".qa-sidepanel");
    sideHeight = side.height();

    $(l).hide();

    $(a).on("click", function(b) {
        b.preventDefault();
        side.slideToggle("fast");
        l.fadeToggle("fast");
        $(this).text($(this).text() == 'Hide Sidebar' ? 'Show Sidebar' : 'Hide Sidebar');
        $(this).toggleClass('sidebar-state');
    });

    $(l).on("click", function(b) {
        b.preventDefault();
        side.slideToggle("fast");
        $(this).fadeOut("fast");
        $(a).text($(a).text() == 'Hide Sidebar' ? 'Show Sidebar' : 'Hide Sidebar');
        $(a).toggleClass('sidebar-state');
    });

    $(window).resize(function() {
        var b = $(window).width();
        if (b > 320 && side.is(":hidden")) {
            side.removeAttr("style")
            }
    })
});

Little details about the code:

.qa-sidepanel is a main sidebar div
#sidepanelpull is a handler when user click it will slide open the .qa-sidepanel and also fadeToggle #sidepanelclose handler ( which is located at the top )
#sidepanelclose is a text link which will be visible at the top when sidebar is open so user on mobile can close from the top if it is too long.

David Fregoli · Accepted Answer · 2013-05-13 16:08:10Z

style-wise I did some changes, have a look

$(function() {

var $window = $(window),
    $a = $("#sidepanelpull"),
    $l = $("#sidepanelclose"),
    $side = $(".qa-sidepanel"),
    sideHeight = $side.height(); // Declared but never used??

$a.on("click", function() {
    $side.slideToggle("fast");
    $l.fadeToggle("fast");
    $a.text( $a.text() == 'Hide Sidebar' ? 'Show Sidebar' : 'Hide Sidebar' );
    $a.toggleClass('sidebar-state');
    return false;
});

$l.hide().on("click", function() {
    $side.slideToggle("fast");
    $l.fadeOut("fast");
    $a.text( $a.text() == 'Hide Sidebar' ? 'Show Sidebar' : 'Hide Sidebar' );
    $a.toggleClass('sidebar-state');
    return false
});

$window.resize(function() {
    var b = $window.width();
    if (b > 320 && $side.is(":hidden")) {
        $side.removeAttr("style")
    }
});

});

most notably, you are calling jQuery on object that are already jQuery objects ($(l) and so on). every call to $() will have its performance overhead. Inmy code the jQuery DOM querying engine is only triggered in the inital variables declaration statement (notice how I grouped var declarations with commas). Even calling $(this) has its overhead so it's best to use the cached variable.

Also, it's good practice to start variables names that contain jQuery objects with the dollar sign $.

And the sideHeight variable is declared but never used?

Since every jQuery method returns the objects it's been called on, you can chain method calls, so I've chained $l.hide().on() since hide() returns $l itself.

Also removeAttr("style") should be left as a last resort just in case you don't have better methods to change the element's style.

Stack Exchange Network

Hide/show sidebar

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Hide/show sidebar

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions