1
\$\begingroup\$

I am PHP programmer whom only recently started to play with a bit of JS/jQuery. Because of that my code is probably something that most of You hardocding JS programmers would see as disgrace (again I am far from being one of you).

I wanted to create simple but quite customizable "tab > content" player and so I did.

I would like to hear from you about what I did wrong and how can I improve my code to bust performance and become more readable to other users. If on the way you find any bugs it will be grateful if you point me the right way to actual solution.

I never used OOP in JS before so if anyone comes with a nice OOP structure for my script It would be something nice to look in to. I am aware that I am not JS OOP ready yet so if you know nice place to learn OOP in JS, it would be great if you could point me to that spot.

Here is my code at CodePen

and repo on GitHub.

Also if you have an idea for new "must have" functionality please let me know. I might implement it in the close future.

//jQuery plugin tabster
(function($){
    $.tabjQeryPlugin = function(options) { //or use "$.fn.myPlugin" or "$.myPlugin" to call it globaly directly from $.myPlugin();
        var defaults = {
            mainWrapper: "#tabWraper",
            target: "div.box", // select all div.box
            buttons: "ul li a", // select all buttons
            buttonAttrName: "target", // attr that contin div tab box id np #box1
            activeClassName: "tabActive", //class to added to active button
            delayAfterClick: 0, // wait this time before fade
            fadeSpeed: 0, // fade with this speed
            showDefault: 1, // tab number be activated when loaded / false - no tab on load
            allowNone: false, // allow closing tabs when clicked seckound time on same button    
            autoPlay: true, // autoplay tabs
            speedPlay: 3500, // time for col to fade
            onClickStopPlay: true, // [true|false] When tabs chenge onclick stop auto play
            resumePlayAfter: 3500, //[3000] after stoping wait this amount of time (3s) and start to play again
            onContentHoverPause: true, // When hover on content will stop playing
            contentOuterWraper: "#contentOuterWraper", // We might need it to get padding-bot to reaply on to the tab element when allowNone is set to true % (responsive on window resize) not needed for auto or fixed height
            pluginActionOn: 'click mouseenter', // interact with buttons on ... [click | mouseenter | ...]
        };

        options = $.extend(defaults, options);

        function logic(){

            var objectClicked1; // holds element id of user curent interaction
            var objectClicked2; // holds element id of user previous interaction
            var objectClickTemp; // holds tempolary value that is placed in 

            var loadFirstTime = true; // placeholder for first time load check objectClicked2

            var interval; // placeholder for interval to play tabs every x secounds
            var curentTabIterator = 0; // placeholder for curent tab number
            var resumeTimeout = ""; // placeholder for timeout in witch function play() will be resumed

            var blockReshowingContentBox = true; // dont refresh content box if true

            var contentOuterWraperPaddingBottom = $(options.contentOuterWraper)[0].style.paddingBottom; // get padding-bottom value in % not in px
            var active_button_interval = ""; // placeholder for number of active button

            if(options.showDefault !== false) curentTabIterator = options.showDefault;

            function toogleClass(button, classname){

                $(options.buttons).removeClass(classname);
                button.toggleClass(classname);

                if(!state) button.toggleClass(classname);

            }

            function autoPlay(button, classname, box){ // return interval varible

                if(options.autoPlay === true){

                    interval = setInterval(function(){

                        var tabsCount = $(options.buttons).length; // count numbers of all tabs
                        objectClickTemp = objectClicked1 = objectClicked1 = $(options.buttons).eq(curentTabIterator).attr(options.buttonAttrName);

                        if(curentTabIterator > tabsCount - 1){ // reset to 0 if over the number of all tabs

                            curentTabIterator = 0;
                        }

                        if(state === false && options.allowNone === true){ // it will stop closed tab to show on Autoplay or Resume

                            blockReshowingContentBox = true;

                        }else{ // allow to display box;

                            blockReshowingContentBox = false;
                        }

                        $(options.target).hide(0); // hide all content box
                        active_button_interval =  $(options.buttons).eq(curentTabIterator); // get curent tab number
                        toogleClass(active_button_interval, options.activeClassName); // ad remove css class

                        if(blockReshowingContentBox !== false){

                            state = false;

                        }else{

                            $(options.target).eq(curentTabIterator).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
                            state = true;

                        }

                        curentTabIterator++;

                    }, options.speedPlay);

                    return interval;
                }

            }

            interator = autoPlay();

            function stopPlay(){

                clearTimeout(resumeTimeout);
                clearInterval(interval);

            }

            function resumePlayAfter(){

                resumeTimeout = setTimeout(function(){
                    stopPlay();
                    interator = autoPlay();
                    info = "function start";
                }, options.resumePlayAfter);

            }

            $(options.target).hide(0); // hide all boxes

            if(options.showDefault !== false && loadFirstTime === true){ // run only once and and only if default tab is activated

                loadFirstTime = false; // change status
                var showDefault = $(options.target).eq(options.showDefault-1); // select default div to show
                showDefault.fadeIn(options.fadeSpeed); // show default div with fadeInEffect
                var defaultButton = $(options.buttons).eq(options.showDefault-1);
                var startingAttr = $(options.buttons).eq(options.showDefault-1).attr(options.buttonAttrName); //select default tag name to show
                var objectClicked1 = startingAttr; // first selected and marked as clicked once button
                var state = true; // change status
                toogleClass(defaultButton, options.activeClassName); // reamove from all and add class to curent button

            }else{

                var state = false; // change status
                stopPlay();

            }



            $(options.buttons).on(options.pluginActionOn, function(){ // ON CLICK OR MOUSE ENTER

                if(options.onClickStopPlay){

                    stopPlay(); // onclick stop auto play;  

                    if(options.resumeAfter !== false && options.onContentHoverPause !== true){

                        resumePlayAfter(); // onclick stop auto play;

                    } 

                }

                var button = $(this); // clicked element
                curentTabIterator = $(options.buttons).index(this) + 1; // onclick reset interval iterator position

                if(objectClicked1 != undefined){ objectClickTemp = objectClicked1 };

                $(options.target).hide(0); // hide div 
                var object = $(this).attr(options.buttonAttrName);

                if(objectClicked1 === undefined){ // if no object clicked

                    objectClicked1 = $(this).attr(options.buttonAttrName); // set clisked button as curent active button
                    objectClicked2 = $(this).attr(options.buttonAttrName); // set clisked button as curent active button

                }else if(objectClickTemp === undefined){ // if templary placeholder for clicked object is undefined

                    objectClicked1 = $(this).attr(options.buttonAttrName);
                    objectClicked2 = objectClicked1;

                }else{ // if non of the abowe assign corect values to objectClicked1 and objectClicked2 based on user interaction

                    objectClicked1 = $(this).attr(options.buttonAttrName);
                    objectClicked2 = objectClickTemp;

                }

                if(!options.allowNone){ // if settings don't allow for tab to be colsed (no tab open is allowed)

                    $(options.target).hide(0);
                    $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
                    toogleClass(button, options.activeClassName);
                    state = true;

                }else{ // if tabs can be colsed (some tab must be open)

                    if(!state && objectClicked1 === objectClicked2){ // check if tab content is curently closed  (state === false) also check if user interact with same element for seckound time

                        $(options.target).hide(0);
                        $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
                        state = true; // change state to open
                        toogleClass(button, options.activeClassName); // add or remove class to clicked button

                    }else if(state && objectClicked1 === objectClicked2){ // check if tab content is curently open  (state === true) also check if user interact with same element for seckound time

                        $(object).delay(options.delayAfterClick).fadeOut(options.fadeSpeed);
                        state = false; // change state to closed
                        toogleClass(button, options.activeClassName);

                    }else if(state && objectClicked1 !== objectClicked2){ // ... do this is tab content is open and user clicked on different element

                        $(options.target).hide(0);
                        $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
                        state = true;
                        toogleClass(button, options.activeClassName);

                    }else if(!state && objectClicked1 !== objectClicked2){ // ... do this is tab content is closed and user clicked on different element

                        $(options.target).hide(0);
                        $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
                        state = true;
                        toogleClass(button, options.activeClassName);

                    }

                }

                // function that gets contentouterwraper paddig and removes it if state === false (content box is hidden)               
                function onOffStateRemovePaddingFromContentOuterWraper(){

                    if(state === true){

                        $(options.contentOuterWraper).css('padding-bottom', contentOuterWraperPaddingBottom);

                    }else{  

                        $(options.contentOuterWraper).css('padding-bottom', '0px');

                    }

                };

                onOffStateRemovePaddingFromContentOuterWraper();


            });

            if(options.onContentHoverPause){

                var selection = "\""+options.target + ", " + options.buttons+ "\"";

                $(options.mainWrapper).hover(function(){

                    stopPlay();

                }, function(){

                    resumePlayAfter();

                });

            }

        }

        jQuery(document).ready(function($) {
            logic();
        });

    };

})(jQuery);
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Macro Observations

As written, $.myPlugin = ... is a jQuery static method, not a plugin. It seems more appropriate to write it a genuine plugin, $.fn.myPlugin = ... and invoke it on DOM element(s) with $(selector).myPlugin(...). You will then need to purge hard-coded selectors and take great care to ensure the independence of multiple invocations.

Strict mode will help avoid making certain mistakes. You may get error messages in your console.

Try passing the code through jsLint to check for unused/undeclared vars and heaps of other stuff.

Meso Observations

options.buttons is "ul li a", therefore $(options.buttons) will select all ul,li,a elements on the page, not just the ones you are interested in. Try constraining the selection with something like $(options.buttons, options.mainWrapper). Probably similar elsewhere for other selections.

logic() is an initialisation function and would more conventionally be named init().

Default tab: Typically with this kind of functionality, you would write initialisation function devoid of code that sets the initial condition. Then, as a final step when all other initialisation is complete, emulate user interaction by triggering a click event to select the default tab. This approach can save much time and many lines of code.

Micro Observations

state should be declared in the main declaration block with a good explanatory comment.

logic()'s local variable loadFirstTime appears to be unnecessary. At the point where it is tested, it will only ever be true, won't it?

resumeTimeout can be initialised simply as var resumeTimeout;, same as interval.

var active_button_interval is rather oddly named given that it is used for jQuery collection object, not a setInterval() reference.

objectClickTemp = objectClicked1 = objectClicked1 = ...!?

Look for unnecessary operations, such as :

$(options.target).hide(0);
...
if() {
    $(options.target).hide(0); // already done unconditionally several lines earlier
    ...
}

Look for simplifications, such as :

//javascript's `||` can make for some very abbreviated syntax, eg
var curentTabIterator = options.showDefault || 0;

if(state === false && options.allowNone === true) {
    blockReshowingContentBox = true;
} else {
    blockReshowingContentBox = false;
}
// can be written :
blockReshowingContentBox = !state && options.allowNone;

and :

$(options.mainWrapper).hover(function() {
    stopPlay();
}, function() {
    resumePlayAfter();
});
// can be written :
$(options.mainWrapper).hover(stopPlay, resumePlayAfter);

and :

if(objectClicked1) {
    objectClickTemp = objectClicked1;
}
var object = $(this).attr(options.buttonAttrName);
if(!objectClicked1) { // if no object clicked
    objectClicked1 = $(this).attr(options.buttonAttrName);
    objectClicked2 = $(this).attr(options.buttonAttrName);
} else if(!objectClickTemp) {
    objectClicked1 = $(this).attr(options.buttonAttrName);
    objectClicked2 = objectClicked1;
} else {
    objectClicked1 = $(this).attr(options.buttonAttrName);
    objectClicked2 = objectClickTemp;
}
// looks like it might simplify to :
var objectClickTemp = objectClicked1 || null; // local to the click handler
objectClicked1 = $(this).attr(options.buttonAttrName);
objectClicked2 = objectClickTemp || objectClicked1;

and :

if(!state && objectClicked1 === objectClicked2) {
    $(options.target).hide(0);
    $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
    state = true; // change state to open
    toogleClass(button, options.activeClassName);
} else if(state && objectClicked1 === objectClicked2) {
    $(object).delay(options.delayAfterClick).fadeOut(options.fadeSpeed);
    state = false; // change state to closed
    toogleClass(button, options.activeClassName);
} else if(state && objectClicked1 !== objectClicked2) {
    $(options.target).hide(0);
    $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
    state = true;
    toogleClass(button, options.activeClassName);
} else if(!state && objectClicked1 !== objectClicked2) {
    $(options.target).hide(0);
    $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
    state = true;
    toogleClass(button, options.activeClassName);
}
// can be written :
if(state && objectClicked1 === objectClicked2) {
    $(object).delay(options.delayAfterClick).fadeOut(options.fadeSpeed);
} else {
    $(options.target).hide(0);
    $(object).delay(options.delayAfterClick).fadeIn(options.fadeSpeed);
}
state = (objectClicked1 === objectClicked2) ? !state : true;
toogleClass(button, options.activeClassName);

and :

//is :
function toogleClass(button, classname) {
    $(options.buttons).removeClass(classname);
    button.toggleClass(classname);
    if(!state) button.toggleClass(classname);
}
// not the same as :
function toogleClass(button, classname) {
    $(options.buttons).removeClass(classname);
    if(state) button.toggleClass(classname);
}
// ?

and :

// tests such as  :
if(options.autoPlay === true) {...}
// can (often) be written :
if(options.autoPlay) {...}

and :

if(curentTabIterator > tabsCount - 1) {
    curentTabIterator = 0;
}
// can be written :
curentTabIterator = curentTabIterator % tabsCount;

and :

// jQuery objects need not be assigned in order to call their methods.
// In general, only assign if the collection is to be used more than once.
var showDefault = $(options.target).eq(options.showDefault-1);
showDefault.fadeIn(options.fadeSpeed);
// can be written :
$(options.target).eq(options.showDefault-1).fadeIn(options.fadeSpeed);

(Obvious) ... when doing simplifications, test at each stage and don't move on until it's working again.

\$\endgroup\$
2
  • \$\begingroup\$ You have taken some time to rewiev this script. I do appreciate this. Thank You. I will try to implement proposed changes on to my script. If you came up with anything more it would be nice to here from you again. Regards \$\endgroup\$
    – DevWL
    Jan 22, 2016 at 18:55
  • \$\begingroup\$ Just a bunch of thoughts and ideas. Some suggestions of may work, some may not. Please feel free to ask if you need to know more. BTW: try my first suggestion last. Converting from static method to plugin is the hardest of all my suggestions. \$\endgroup\$ Jan 22, 2016 at 20:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.