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

Resizing a sidebar

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

Problem

This is really simple but I would like to improve the efficiency as it's run frequently on the client:

function resizeSidebar() {

    var h_tmp = 0;

    if ($("#internal-holder").length > 0)
    { h_tmp = 250; }
    else
    { h_tmp = 181; }

    var h_var = $(window).height() - h_tmp;
    var h_fixed = $(window).height();

    $("#aside-holder").css("height", h_fixed - 225 + "px");
    $("#main-content").css("height", h_var + "px");
    $("#main-inner").css("height", h_var - 115 + "px");
    $("#main-inner-scroll .viewport").css("height", h_var - 110 + "px");
    $("#aside-holder-scroll .viewport").css("height", h_fixed - 225 + "px");
    $("#main-inner-scroll").tinyscrollbar();
    if (hideAsideScrollBar != "true") {
        $("#aside-holder-scroll").tinyscrollbar();
    }
}

Solution

-
$("#foo") is slow, use the DOM: document.getElementById("foo").

-
$("#foo .bar") is slow, get rid of that by navigating the DOM. I can't say exactly how without seeing the markup, but look at the FIXME comments below.

-
$("#foo").css() is slow, use the DOM: foo.style.height = "24px".

-
Get the reference to the elements once, not every time the function is called.

Other notes:

-
Don't do that weird thing with the braces on your if statements.

-
Use the var keyword as little as possible by combining variable declarations.

-
This appeases linters and reduces minified file size.

-
It also pretty much forces you to declare all your variables at the top of the function, which is generally considered to be a good practice.

-
Avoid the use of _ in variable names; prefer camelCase instead. Also try to use meaningful variable names.

-
Except for variables or properties representing constants. In these cases the UPPER_CASED words are joined by underscores.

-
An exception may be made for variables or properties meant to be considered "private". In these cases the camelCased words are prefixed or suffixed with an underscore.

var internalHolder = document.getElementById("internal-holder"),
    asideHolder = document.getElementById("aside-holder"),
    mainContent = document.getElementById("main-content"),
    mainInner = document.getElementById("main-inner"),
    mainInnerScroll = document.getElementById("main-inner-scroll"),
    asideHolderScroll = document.getElementById("aside-holder-scroll"),
    mainInnerScrollViewport = mainInnerScroll.children[0], // FIXME  
    asideHolderScrollViewport = asideHolderScroll.children[0]; // FIXME  

function resizeSidebar() {

    var tmpHeight = internalHolder.children.length ? 250 : 181,
        height = window.innerHeight,
        innerHeight = height - tmpHeight;

    asideHolder.style.height = height - 225 + "px";
    mainContent.style.height = innerHeight + "px";
    mainInner.style.height = innerHeight - 115 + "px";

    mainInnerScrollViewport.style.height = innerHeight - 110 + "px";
    asideHolderScrollViewport.style.height = height - 225 + "px";

    $(mainInnerScroll).tinyscrollbar();
    if (hideAsideScrollBar != "true") {
        $(asideHolderScroll).tinyscrollbar();
    }
}


I can almost guarantee this will be at least 10 times faster if you fix the FIXMEs.

BUT.

What you are doing feels really wrong. Using javascript to get and set a bunch of heights should be avoidable. Again I can't say exactly what to do without seeing the markup, but you should be able to achieve most (if not all) of the $('#foo').css stuff through plain old HTML/CSS. Try giving things 100% height, try inline-block and vertical-align, hell, try tables, but javascript css styling should, IMO, be kept to an absolute minimum.

Code Snippets

var internalHolder = document.getElementById("internal-holder"),
    asideHolder = document.getElementById("aside-holder"),
    mainContent = document.getElementById("main-content"),
    mainInner = document.getElementById("main-inner"),
    mainInnerScroll = document.getElementById("main-inner-scroll"),
    asideHolderScroll = document.getElementById("aside-holder-scroll"),
    mainInnerScrollViewport = mainInnerScroll.children[0], // FIXME  
    asideHolderScrollViewport = asideHolderScroll.children[0]; // FIXME  

function resizeSidebar() {

    var tmpHeight = internalHolder.children.length ? 250 : 181,
        height = window.innerHeight,
        innerHeight = height - tmpHeight;

    asideHolder.style.height = height - 225 + "px";
    mainContent.style.height = innerHeight + "px";
    mainInner.style.height = innerHeight - 115 + "px";

    mainInnerScrollViewport.style.height = innerHeight - 110 + "px";
    asideHolderScrollViewport.style.height = height - 225 + "px";

    $(mainInnerScroll).tinyscrollbar();
    if (hideAsideScrollBar != "true") {
        $(asideHolderScroll).tinyscrollbar();
    }
}

Context

StackExchange Code Review Q#8819, answer score: 5

Revisions (0)

No revisions yet.