3
\$\begingroup\$

I'd appreciate a review of at least some of this javascript, if anyone has time. It's really long, but I think it's useful. It's designed to let you create forms and do javascripty things by only modifying html tags and attributes.

Take a look at what it's supposed to do here: https://docs.google.com/document/d/1T08AJr4OGR2ohXLeSK3WjKn8zG-sSLi3Vz1Ya5tLkQY/edit?usp=sharing

//Configuration variables
var config = jQuery("edfconfig");

function getConfigBoolean(attr) {
    if (config != null)
    return jQuery(config).attr(attr) != undefined;
}
function getConfigValue(attr) {
    if (config != null)
    return jQuery(config).attr(attr);
}

var noasterisks         = getConfigBoolean("noasterisks");
var addafter            = getConfigBoolean("addafter");
var doactions           = getConfigBoolean("doactions");
var noappend            = getConfigBoolean("noappend");
var requiredmessage     = getConfigValue("requiredmessage");
var requiredmessageid   = getConfigValue("requiredmessageid");


//eDomForm variables
var attrs = jQuery("edfvars")[0];
var variables = {};
if (attrs != null)
for (i=0;i<attrs.attributes.length;i++) {
    variables[attrs.attributes[i].nodeName] = attrs.attributes[i].nodeValue;
}

//Function because jQuery doesn't have a selector for the name attribute
function getByName(n) {
    return document.getElementsByName(n);
}

//required fields have the class required_field
var required = jQuery(".required_field");
//message div to display global form messages
var message = jQuery("#message");
//reference to the entire form itself
var form = jQuery("#form");

//form data
jQuery(form).data("required_empty",required.length);

//Add next and back buttons
jQuery(".form_page").each(function(i){
    jQuery(this).prepend('<button type="button" class="back">Back</button> <button type="button" class="next">Next</button>');
});

//Next and Back buttons
var pages = jQuery(".form_page");
var backButtons = jQuery(".back");
var nextButtons = jQuery(".next");
for (i=0;i<pages.length;i++) {
    if (i != pages.length-1) {
        nextButtons[i].onclick = function() {
            jQuery(this).closest("div").fadeOut();
            jQuery(this).closest("div").nextAll(":not(.disabled):first").fadeIn();
        };
    } else {
        jQuery(nextButtons[i]).remove();
    }
    if (i != 0) {
        backButtons[i].onclick = function(i) {
            jQuery(this).closest("div").fadeOut();
            jQuery(this).closest("div").prevAll(":not(.disabled):first").fadeIn();
        };
    } else {
        jQuery(backButtons[i]).remove();
    }
}

//Aliases
function getByAlias(a) {
    return jQuery("[alias="+a+"]");
}
function hasClass(element, cls) {
    return (' ' + element.className + ' ').indexOf(' ' + cls + ' ') > -1;
}

var aliases = jQuery(".alias");

//Connect all aliases and original fields to each other
jQuery(".alias").each(function(i){
    var alias = jQuery(this).attr("alias");
    var allaliases = jQuery("[alias="+alias+"]");
    var alloriginals = jQuery("."+alias);
    jQuery(alloriginals).each(function(j){
        var thisOriginal = this;
        jQuery(allaliases).each(function(k) {
            var thisAlias = this;
            if (hasClass(thisOriginal,"required_field")) {
                jQuery(thisAlias).addClass("required_field");
            }
            thisAlias.onchange = aliasChange.bind(null,thisAlias,thisOriginal,thisAlias.onchange);
            thisOriginal.onchange = aliasChange.bind(null,thisOriginal,thisAlias,thisOriginal.onchange);
            jQuery(allaliases).each(function(l){
                var innerAlias = this;
                innerAlias.onchange = aliasChange.bind(null,innerAlias,thisAlias,innerAlias.onchange);
                thisAlias.onchange = aliasChange.bind(null,thisAlias,innerAlias,thisAlias.onchange);
            });
        });
        jQuery(alloriginals).each(function(j){
            var innerOriginal = this;
            innerOriginal.onchange = aliasChange.bind(null,innerOriginal,thisOriginal,innerOriginal.onchange);
            thisOriginal.onchange = aliasChange.bind(null,thisOriginal,innerOriginal,thisOriginal.onchange);
        });
    });
});
function aliasChange(o,a,func) {
    if (func != null)
    func();
    a.value = o.value;
    if (a.onblur != null)
    a.onblur();
}
var radioGroupCounter = 1;
var actions = {};

jQuery(document).ready(function() {
    //Prevent form submission if required fields have not been filled in
    if (form[0] != null)
    form[0].onsubmit = function() {
        var required_fields = document.getElementsByClassName("required_field");
        for (i=0;i<required_fields.length;i++) {
            if (required_fields[i].value == "") {
                jQuery("#"+requiredmessageid).html(requiredmessage);
                return false;
            }
        }
        jQuery("#"+requiredmessageid).html("");
        return true;
    };

    window.activate = function(func) {
        actions[func]();
    };

    //action functions
    window.write = function(text) {
        document.write(text);
    };
    window.writeTo = function(to,text) {
        jQuery("."+to).each(function(){
            jQuery(this).append(text);
        });
    };
    window.writeOver = function(to,text) {
        jQuery("."+to).each(function() {
            jQuery(this).html(text);
        });
    };
    window.hide = function(what) {
        jQuery("."+what).each(function() {
            jQuery(this).hide();
        });
    };
    window.show = function(what) {
        jQuery("."+what).each(function() {
            jQuery(this).show();
        });
    };
    window.set = function(what,val) {
        variables[what] = val;
    };

    //Action tags
    function handleActionTags() {
        jQuery("action").each(function(i) {
            var id = jQuery(this).attr("id");
            var type = jQuery(this).attr("type");
            var args = jQuery(this).attr("arguments").split(" ");
            for (i=0;i<args.length;i++) {
                var name = args[i].match(/%(.+)%/);
                if (name != null) {
                    args[i] = variables[name[1]];
                }
            }
            actions[id] = function() {
                window[type].apply(null,args);
            };
        });
    }
    handleActionTags();

    //Custom Events
    jQuery("[onempty]").each(function() {
        if (jQuery(this).val() == "") {
            jQuery(this).data("empty",true);
        }
        jQuery(this).on("keyup",function() {
            if (jQuery(this).val() == "") {
                jQuery(this).data("empty",true);
                eval(jQuery(this).attr("onempty"));
            }
        });
    });
    jQuery("[onnotempty]").each(function() {
        jQuery(this).on("keyup",function() {
            if (jQuery(this).val() != "" && jQuery(this).data("empty")) {
                jQuery(this).data("empty",false);
                eval(jQuery(this).attr("onnotempty"));
            }
        });
    });
    jQuery("[oncheck]").each(function() {
        jQuery(this).on("change",function() {
            if (jQuery(this).is(":checked")) {
                eval(jQuery(this).attr("oncheck"));
            }
        });
    });
    jQuery("[onuncheck]").each(function() {
        jQuery(this).on("change",function() {
            if (!jQuery(this).is(":checked")) {
                eval(jQuery(this).attr("onuncheck"));
            }
        });
    });

    //Hidden pages
    function handleHiddenPages() {
        jQuery(".revealer").each(function(i){
            var page = jQuery(this).attr("page");
            jQuery(this).click(function(){
                if (jQuery(this).is(":checked")) {
                    jQuery("."+page).removeClass("disabled");
                } else {
                    jQuery("."+page).addClass("disabled");
                }
            });
        });
    }
    handleHiddenPages();

    //Switchers
    function handleSwitchers() {
        jQuery(".switcher").each(function(x){
            var connections = jQuery(this).attr("connections");
            connections = jQuery.parseJSON(connections);
            var connectedSections = {};
            for (var key in connections) {
                //if something like a-b
                if (connections[key].indexOf("-") > -1) {
                    var nums = connections[key].split("-");
                    var resultNums = [];
                    for (i=0;i<nums.length;i++) {
                        nums[i] = parseInt(nums[i]);
                    }
                    for (i=nums[0];i<=nums[nums.length-1];i++) {
                        resultNums.push(i+"");
                        connectedSections[i] = key;
                    }
                } else {
                    if (connections.hasOwnProperty(key))
                    connectedSections[connections[key]] = key;
                }
            }
            jQuery(this).change(function(){
                for (var key in connectedSections) {
                    jQuery("."+connectedSections[key]).hide();
                }
                jQuery("."+connectedSections[jQuery(this).val()]).show();
            });
        });
    }
    handleSwitchers();

    //Displayers/Hiders
    function handleDisplayers() {
        jQuery(".displayer").each(function(x){
            var connected = jQuery(this).attr("display");
            var special = "";
            var connecteds = [];
            if (connected.indexOf(" ") > -1) {
                connecteds = connected.split(" ");
                var special = connecteds[0];
                connected = connecteds[1];
            }
            var name = jQuery(this).attr("name");
            var group = getByName(name);
            jQuery(group).each(function() {
                jQuery(this).on("click",function() {
                var button = this;
                    if (jQuery(this).attr("display") != null) {
                        if (special == "") {
                            jQuery("."+connected).each(function() {
                                jQuery(this).show();
                            });
                        }
                        else if (special == "next") {
                            jQuery("."+connected).each(function() {
                                if (button.compareDocumentPosition(this) == 4) {
                                    jQuery(this).show();
                                    return false;
                                }
                            });
                        }
                        else if (special == "prev") {
                            jQuery(jQuery("."+connected).get().reverse()).each(function(i) {
                            if (button.compareDocumentPosition(this) == 2) {
                                jQuery(this).show();
                                return false;
                            }
                        });
                        }
                    }else {
                        if (special == "") {
                            jQuery("."+connected).each(function() {
                                jQuery(this).hide();
                            });
                        }
                        else if (special == "next")
                        jQuery("."+connected).each(function() {
                            if (button.compareDocumentPosition(this) == 4) {
                                jQuery(this).hide();
                                return false;
                            }
                        });
                        else if (special == "prev")
                        jQuery(jQuery("."+connected).get().reverse()).each(function(i) {
                            if (button.compareDocumentPosition(this) == 2) {
                                jQuery(this).hide();
                                return false;
                            }
                        });
                    }
                });
            });
        });
    }
    handleDisplayers();

    //findNext function from stackoverflow
    /**
     * Find the next element matching a certain selector. Differs from next() in
     *  that it searches outside the current element's parent.
     *  
     * @param selector The selector to search for
     * @param steps (optional) The number of steps to search, the default is 1
     * @param scope (optional) The scope to search in, the default is document wide 
     */
    $.fn.findNext = function(selector, steps, scope)
    {
        // Steps given? Then parse to int 
        if (steps)
        {
            steps = Math.floor(steps);
        }
        else if (steps === 0)
        {
            // Stupid case :)
            return this;
        }
        else
        {
            // Else, try the easy way
            var next = this.next(selector);
            if (next.length)
                return next;
            // Easy way failed, try the hard way :)
            steps = 1;
        }

        // Set scope to document or user-defined
        scope = (scope) ? $(scope) : $(document);

        // Find kids that match selector: used as exclusion filter
        var kids = this.find(selector);

        // Find in parent(s)
        hay = $(this);
        while(hay[0] != scope[0])
        {
            // Move up one level
            hay = hay.parent();     
            // Select all kids of parent
            //  - excluding kids of current element (next != inside),
            //  - add current element (will be added in document order)
            var rs = hay.find(selector).not(kids).add($(this));
            // Move the desired number of steps
            var id = rs.index(this) + steps;
            // Result found? then return
            if (id > -1 && id < rs.length)
                return $(rs[id]);
        }
        // Return empty result
        return $([]);
    }

    //Adding New Sections
    function handleAdds() {
        jQuery(".add").each(function(x){
            var add = jQuery(this).attr("add");
            if (add.indexOf(" ") > -1) {
                add = add.split(" ");
            }
            var to = jQuery(this).attr("to");
            var radiogroup = jQuery(this).attr("radiogroup");
            if (radiogroup != null)
            radiogroup = radiogroup.split(" ");
            var cpy = jQuery("<div />").append(jQuery("."+add).clone()).html();
            if (to == null) {
                jQuery(this).click(function() {
                    var text = cpy;
                    var counter = radioGroupCounter++;
                    if (radiogroup != null)
                    for (i=0;i<radiogroup.length;i++) {
                        var re = new RegExp(radiogroup[i]+"\\[\\d\\]","g");
                        text = text.replace(re,radiogroup[i]+"["+(counter)+"]");
                    }
                    if (addafter)
                    jQuery(this).after(text);
                    else
                    jQuery(this).before(text);

                    handleHiddenPages();
                    handleDisplayers();
                    handleSwitchers();
                });
            } else {
                if (to.indexOf(" ") > -1) {
                    to = to.split(" ");
                }
                jQuery(this).click(function() {
                    var text = cpy;
                    var counter = radioGroupCounter++;
                    if (radiogroup != null)
                    for (i=0;i<radiogroup.length;i++) {
                        var re = new RegExp(radiogroup[i]+"\\[\\d\\]","g");
                        text = text.replace(re,radiogroup[i]+"["+(counter)+"]");
                        console.log(text);
                    }
                    jQuery("#"+to).append(text);

                    handleHiddenPages();
                    handleDisplayers();
                    handleSwitchers();
                });
            }
        });
    }
    handleAdds();

    //Action tags


    function handleAll() {
        handleHiddenPages();
        handleDisplayers();
        handleSwitchers();
        handleAdds();
        handleActionTags();
    }
});

required = jQuery(".required_field");

//Loop through required fields, adding the onblur event
//so that whenever the user deselects a required field,
//if it is blank the asterisk will turn red.
for (i=0;i<required.length;i++) {
    jQuery(required[i]).after("<span>*</span>");
    jQuery(required[i]).data("empty",true);
    required[i].onblur = function() {
        if (this.value == "") {
            jQuery(this).next().css("color","#f00");
        } else {
            jQuery(this).next().css("color","#000");
        }
    };
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Just in the first few lines: config if a jQuery object, but you're still doing jQuery(config), so you're creating another jQuery object. Not only that, but you're doing it every time you call getConfig*. \$\endgroup\$ – mowwwalker Jun 29 '13 at 1:45
3
\$\begingroup\$

After looking through your code I have a few suggestions for you that are mainly best practice type suggestions:

  1. Avoid global variables

You are defining just about all of your code at the global scope which is a bad practice that can lead to conflicts with other scripts. For instance, you have defined config as a global variable. That is a really common variable name and another script may use a global variable named config which could cause problems with both scripts.

Consider at the very least defining all of your code inside a self-executing function:

(function(){/*Your code*/});

Or even better create a namespace and define all of your variables and functions there:

var yourNamespace = {
 YourFunctionName : function(){},
 config : {}
}
  1. Always use brackets after if statements

In several place you are omitting the {} after if statements. This is perfectly valid JavaScript, but it creates some readability issues and can also lead to bugs. Lets say in the future you require two statements after your if statement and you forget to add the braces. Anyway, the point is there is no good reason not to put in the braces.

  1. Use !== and === instead of != and '=='

There is no good reason to use != and ==. When testing for equality or inequality you should be checking for type and value equality. Using the != and == can lead to surprising results that are not at all intuitive.

  1. Use a single var keyword

This is a bit of a style suggestion that most JS developers adhere to but is by no means absolutely necessary. Again, this will make your code look better to many developers but is functionally equivalent to what you have.

var noasterisks         = getConfigBoolean("noasterisks"),
    addafter            = getConfigBoolean("addafter"),
    doactions           = getConfigBoolean("doactions"),
    noappend            = getConfigBoolean("noappend"),
    requiredmessage     = getConfigValue("requiredmessage"),
    requiredmessageid   = getConfigValue("requiredmessageid");

This is not an exhaustive list, but the first three suggestions will make your code more maintainable and easier to debug than it stands.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the suggestions. I'll probably use the majority of them. Especially the first one. \$\endgroup\$ – Samuel Reid Jun 28 '13 at 20:00

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.