2
\$\begingroup\$

This is a follow-up to this question my code has evolved since so I'm reposting my question.

What is the best way to tidy this up? How can this be refined? I am finding it's thrashing the layout quite a lot and the scroll function seems to lag.

;(function ($, window) {
var main = $("#swab"),
    sbContainer = $("#sb-container"),
    sbContainerDiv = sbContainer.find("div"),
    tblcontents = $('#tblcontents'),
    tblcontentstwo = $('#tblcontents_2'),
    button = $('#wrap_return, .thumb, #tblcontents'),
    slideele = $('.site, .menu-panel'),
    slideele2 = $('.site, .menu-panel_2');



function animate() {
    sbContainerDiv.css({
        "transform": "rotate(0deg)",
            "-webkit-transform": "rotate(0deg)",
            "-ms-transform": "rotate(0deg)",
            "-moz-transform": "rotate(0deg)",
            "-o-transform": "rotate(0deg)"
    });

    main.animate({
        "right": "1%",
            "left": "auto",
            "top": "4px"
    }, "slow");
}

function toggletwo() {

    var toggleStatetwo = true;
    $(tblcontents).on("click", function () {
        if (toggleStatetwo) {
            animate({
                "top": "15%"
            });
            main.fadeOut(1000);
        } else {
            animate();
            main.fadeIn(1000);
        }
        toggleStatetwo = !toggleStatetwo;
    });

}

function toggleone() {
    $(tblcontents).on("click", function () {
        main.stop(true, true).animate();
    });

}

var $window = $(window);
$window.resize(function () {
    if (this.resizeTO) clearTimeout(this.resizeTO);

    this.resizeTO = setTimeout(function () {
        $(this).trigger('resizeEnd');
    }, 500);
});

$window.on('resizeEnd', function () {
    var winWidth = $window.width();

    if (winWidth < 960) {
        toggletwo();
        animate();
    } else {
        toggleone();
    }
});

$(document).scroll(function () {
    animate();
});
var toggleState = true;
$('#toggle_div').on("click", function () {
    if (toggleState) {
        main.animate({
            "right": "50%"
        }, "slow");
    } else {
        main.animate({
            "right": "1%",
                "top": "4px"
        }, "slow");
        main.css("left", "auto");
    }
    toggleState = !toggleState;
});

$(tblcontents).effect("pulsate", {
    times: 100
}, 2000).on('click', function () {
    $(this).effect().stop(true, true);
    $(this).animate({
        "opacity": "1"
    }, "fast");
});
$(button).on("click", function () {
    animate({
        "top": "4px"
    }, "slow");
});
main.draggable()

$(function () {

    sbContainer.swatchbook({
        center: 6,
        angleInc: 20,
        speed: 700,
        easing: 'ease',
        proximity: 120,
        neighbor: 20,
        onLoadAnim: true,
        initclosed: true,
        closeIdx: 11,
        openAt: -1

    });
});

var toggleStatemenu2 = true;
$(tblcontentstwo).on("click", function () {
    if (toggleStatemenu2) {
        $(slideele2).animate({
            top: '+=171'
        }, 458, 'swing', function () {});
    } else {
        $(slideele2).animate({
            top: '-=171'
        }, 458, 'swing', function () {});
    }
    toggleStatemenu2 = !toggleStatemenu2;
});
var toggleStatemenu = true;
$(tblcontents).on("click", function () {
    if (toggleStatemenu) {
        $(slideele).animate({
            left: '+=240'
        }, 458, 'swing', function () {});
    } else {
        $(slideele).animate({
            left: '-=240'
        }, 458, 'swing', function () {});
    }
    toggleStatemenu = !toggleStatemenu;
});

})(jQuery, window, document);
\$\endgroup\$
1
\$\begingroup\$

Interesting question:

Naming

  • JavaScript uses lowerCamelCase variable and function names. ( tblcontentstwo -> TblContentsTwo or TabelContents2
  • Some of your prefixes like sb are not adding anything. I would drop those prefixes.
  • Names like slideele2 are unfortunate, ideally the name should reflect what is being slided (slid?)

Comments

  • You have no comments whatsoever, fortunately the code reads well
  • Still, in some places, like sbContainer.swatchbook(, I have no clue what you are trying to accomplish

JSHint.com

  • Your code passes all checks, except for 1 semicolon, well done ;)

Readability

  • You don't use newlines consistently, more specifically you need more newlines in a few spots
  • Also your indentation is off in places, I would use a site like http://jsbeautifier.org/

Don't Repeat Yourself

  • This looks like copy pasted code:

    var toggleStatemenu2 = true;
    $(tblcontentstwo).on("click", function () {
    if (toggleStatemenu2) {
        $(slideele2).animate({
        top: '+=171'
        }, 458, 'swing', function () {});
    } else {
        $(slideele2).animate({
        top: '-=171'
        }, 458, 'swing', function () {});
    }
    toggleStatemenu2 = !toggleStatemenu2;
    });
    var toggleStatemenu = true;
    $(tblcontents).on("click", function () {
    if (toggleStatemenu) {
        $(slideele).animate({
        left: '+=240'
        }, 458, 'swing', function () {});
    } else {
        $(slideele).animate({
        left: '-=240'
        }, 458, 'swing', function () {});
    }
    toggleStatemenu = !toggleStatemenu;
    });
    

    you could try to make a helper function there.

    var toggleStates = {};
    
    function createSlider( button, slider, animation1, animation2 ){
      toggleStates[button.id] = true;
      $(button).on("click", function () {
        if (toggleStates[button.id]) {
          $(slider).animate(animation, 458, 'swing', function () {});
        } else {
          $(slider).animate(animation2, 458, 'swing', function () {});
        }
        toggleStates[button.id] = !toggleStates[button.id];
      }
    }
    createSlider(tblcontentstwo, slideele2, {top: '+=171'}, {top: '-=171'} );
    createSlider(tblcontents, slideele, {left: '+=240'}, {left: '-=240'} );
    
\$\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.