SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 28
  1. #1
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)

    [Code Review] Form builder

    I am looking for some feedback on my code, it is not done but most of the functionality is already there however the code seems to be long and wanted to see how I could improve and shorten it.

    Code JavaScript:
    function oc(a){
    	var o = {};
    	for(var i=0;i<a.length;i++){
    		o[a[i]]='';
    		}
    	return o;
    	}
    var fields_array = new Array();
    var fields_array_counter = 0;
    var input_a = new Array('button', 'checkbox', 'file', 'hidden', 'password', 'radio', 'reset', 'submit', 'text')
    var last_created_1 = '';
    var last_created_2 = '';
    var last_created_3 = '';
    var last_created_4 = '';
    var last_created_5 = '';
    var last_created_counter = null;
    var groupname_a = new Array();
    var groupname_counter = 0;
    var form_id = 'form_preview';
    var form_action = '(No action has been set)';
    var form_method = '(no method has been set)';
    function form_create_input(){
    	var label = document.getElementById('label').value;
    	var label_loc = document.getElementById('label_loc').value;
    	var groupname = document.getElementById('groupname').value;
    	var groupnamelegend = document.getElementById('groupnamelegend').value;
    	var field_name = document.getElementById('field_name').value;
    	var type = document.getElementById('type').value;
    	var text_value = document.getElementById('text_value').value;
    	var text_rows = document.getElementById('text_rows').value;
    	var text_cols = document.getElementById('text_cols').value;
    	var showinfo = document.getElementById('showinfo').value;
    	var sel_check_opt = document.getElementById('sel_check_opt').value.split(', ');
    	var sel_check_opt_counter = sel_check_opt.length - 1;
    	var form_preview = document.getElementById(form_id).innerHTML;	
    	//var button_remover = '<a id="' + field_name + 'button_remove" class="button_remove buttons" href="#" onclick="remove_field(' + "'" + field_name + "'" + ')" ><span >Remove field</span></a>';
    	var button_remover = '<a class="button_remove buttons" href="#" onclick="remove_field(this);return false;" ><span >Remove field</span></a>';
    	var button_up = '<a class="button_up buttons" href="#" onclick="move_field(this, ' + "'previous'" + ');return false;" ><span >Move up</span></a>';
    	var button_down = '<a class="button_down buttons" href="#" onclick="move_field(this, ' + "'next'" + ');return false;" ><span >Move down</span></a>';
    	last_created_5 = last_created_4;
    	last_created_4 = last_created_3;
    	last_created_3 = last_created_2;
    	last_created_2 = last_created_1;
    	last_created_1 = form_preview;
    	if (showinfo != ''){
    		showinfo = '<span>' + showinfo + '</span>';
    		}
    	if (groupname != ''){
    		if (groupname in (oc(groupname_a))){}
    		else{
    			groupname_a[groupname_counter] = document.getElementById('groupname').value;
    			groupname_counter ++;
    			}
    		}
    	var newlabel = '<label id="' + field_name + 'l" >';
    	if (type in (oc(input_a))){
    		var newfield = '<input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />';
    		if (label_loc == '1'){
    			newfield = newlabel + label + showinfo + newfield + button_remover + button_up + button_down + '</label>';
    			}
    		else {newfield = newlabel + newfield + label + showinfo + button_remover + button_up + button_down + '</label>';}
    		}
    	else if(type == 'textarea'){
    		var newfield = '<textarea id="' + field_name + '" rows="' + text_rows + '" cols="' + text_cols + '">' + text_value + '</textarea>';
    		if (newlabel != ''){
    			if (label_loc == 1){
    				newfield = newlabel + newfield;
    				}
    			}
    		}
    	else if(type == 'select'){
    		var newfield = '<select id="' + field_name + '" >';
    		var count_opt = 0;
    		var options = '';
    		while (sel_check_opt_counter >= count_opt){
    			options = options + '<option value="' + sel_check_opt[count_opt] + '">' + sel_check_opt[count_opt] + '</option>';
    			count_opt ++;
    			}
    		newfield = newfield + options + '</select>';
    		if (newlabel != ''){
    			if (label_loc == 1){
    				newfield = newlabel + newfield;
    				}
    			}
    		}
    	fields_array[fields_array_counter] = new Array(groupname, groupnamelegend, newfield);
    	fields_array.sort();
    	fields_array_counter ++;
    	last_created_counter = 1;
    	this_fieldset = '';
    	document.getElementById(form_id).innerHTML = form_preview + newfield;
    	}	
    function revert_change(){
    	if(last_created_counter == null){
    		alert('Sorry canīt undo, you have not done any changes.');
    		}
    	else if (last_created_counter == 1){
    		document.getElementById(form_id).innerHTML = last_created_1;
    		last_created_counter ++;
    		}
    	else if (last_created_counter == 2){
    		document.getElementById(form_id).innerHTML = last_created_2;
    		last_created_counter ++;
    		}
    	else if (last_created_counter == 3){
    		document.getElementById(form_id).innerHTML = last_created_3;
    		last_created_counter ++;
    		}
    	else if (last_created_counter == 4){
    		document.getElementById(form_id).innerHTML = last_created_4;
    		last_created_counter ++;
    		}
    	else if (last_created_counter == 5){
    		document.getElementById(form_id).innerHTML = last_created_5;
    		last_created_counter ++;
    		}
    	else {
    		alert('Sorry canīt undo, use the red x on the field that you like to delete.');
    		}
    	}
    function remove_field(id){
    	var nodetoremove = id.parentNode;
    	var parent = document.getElementById(form_id);
    	parent.removeChild(nodetoremove);
    	}
    function form_configurator(form_option){
    	var new_value = document.getElementById(form_option).value;
    	var the_form = document.getElementById(form_id);
    	if (form_option == 'form_id'){
    		the_form.id = new_value;
    		form_id = new_value;
    		}
    	else if (form_option == 'form_action'){
    		the_form.action = new_value;
    		form_action = new_value;
    		}
    	else if (form_option == 'form_method'){
    		the_form.method = new_value;
    		form_method = new_value;
    		}
    	var the_info = 'Current form id is "' + form_id + '" current action is "' + form_action + '", current method is "' + form_method + '"';
    	document.getElementById('form_info').innerHTML = the_info;
    	}
    var warningsent = '1';
    function move_field(node, direction) {
    	var nodetomove = node.parentNode;
    	if (direction == 'previous'){
    		if (nodetomove.previousSibling.id == null){
    			if(warningsent == '2'){}
    			else{alert('Nowhere to move!');warningsent = '2';}
    			}
    		else {
    			var thenode = nodetomove.previousSibling;
    			document.getElementById(form_id).insertBefore(nodetomove, thenode);
    			warningsent = '1';
    			}
    		}
    	else{
    		if (nodetomove.nextSibling == null){
    			if(warningsent == '2'){}
    			else{alert('Nowhere to move!');warningsent = '2';}
    			}
    		else{
    			var thenode = nodetomove.nextSibling;
    			thenode = thenode.nextSibling;
    			document.getElementById(form_id).insertBefore(nodetomove, thenode);
    			warningsent = '1';
    			}
    		}
    	}

  2. #2
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    While I'm heading off to sleepy-bye land, here are some preliminary notes to kick things off with:

    • The oc function seems to work differntly from other object creation functions that are out there.
      The standard technique is for the methods to be passed as an object instead of an array. The object is then looped through using for...in and checking hasOwnProperty
    • [] instead of new Array()
    • use array instead of multiple variations of last_created
    • last_created_counter ??? use array length instead
    • one var statement instead of multiple
    • remove newfield duplication
    • confusing if/else structures. Move code blocks out to functions, and simplify
    • use decorator pattern for label_loc
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  3. #3
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    I am looking for some feedback on my code, it is not done but most of the functionality is already there however the code seems to be long and wanted to see how I could improve and shorten it.
    I did mention the oc function before, but until I find out what you intend to use a for, which currently seems to be to empty properties of the object.

    Ahh, looking further in to the rest of the code, that oc function is used only to work out if an array item already exists in an array, for which there is a built-in array.indexOf() method that should be used instead.

    You do need to cater for web browsers such as Internet Explorer that haven't yet reached support for JavaScript 1.6, so the page also gives you compatibility code to help those browsers out.

    So instead of the following code:

    Code:
    if (type in (oc(input_a))){
    You can use this instead:

    Code javascript:
    if (input_a.indexOf(type) > -1) {

    Next though, about the variables. Are they global variables? They are fundamentally at risk of being clobbered by any other code on the page that also sets global variables (not just by your code, but by other code that you cannot trust). We can try and resolve that problem by putting them and the functions within a single formBuilder object, but that can occur later on once the functions are much more encapsulated.

    Counters

    The sel_check_opt_counter variable is not needed and can be easily removed. Instead of using the following while loop:

    Code:
    while (sel_check_opt_counter >= count_opt){
        options = options + '<option value="' + sel_check_opt[count_opt] + '">' + sel_check_opt[count_opt] + '</option>';
        count_opt ++;
    }
    All you need to do instead is to loop through the sel_check_opt array:

    Code javascript:
    for (i = 0; i < sel_check_opt.length; i += 1) {
        options = options + '<option value="' + sel_check_opt[i] + '">' + sel_check_opt[i] + '</option>';
    }

    Similar work can be performed to remove other counters, such as the last_created one. That whole mess can be replaced with pushing and popping values from the array.

    We can even get your special message in for if no changes have been made, by tracking whether it has been used or not.

    Code javascript:
    last_created.push(form_preview);
    ...
    if (last_created.length === 0) {
        if (!last_created.used) {
            alert('Sorry canīt undo, you have not done any changes.');
        } else {
            alert('Sorry canīt undo, use the red x on the field that you like to delete.');
    } else {
        document.getElementById(form_id).innerHTML = last_created.pop();
        last_created.used = true;
    }

    There's a lot more to be done, but I'll hold off until there is some kind of testing harness, or even a test page where the script can be exercised to ensure that through the code changes that the behaviour remains consistent and as expected.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  4. #4
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I am checking your post right now, but I read your first message and did some changes and this is how the code looks right now, and I will go through your new post and make more changes to it, thank you.
    Code JavaScript:
    var input_a = new Array('button', 'checkbox', 'file', 'hidden', 'password', 'radio', 'reset', 'submit', 'text');
    var form_values = new Array('form_preview', 'No action has been set', 'no method has been set');
    var undo_last = [];
    var warningsent = false;
    var button_remover = '<a class="button_remove buttons" href="#" onclick="remove_field(this);return false;" ><span >Remove field</span></a>';
    var button_up = '<a class="button_up buttons" href="#" onclick="move_field(this, ' + "'previous'" + ');return false;" ><span >Move up</span></a>';
    var button_down = '<a class="button_down buttons" href="#" onclick="move_field(this, ' + "'next'" + ');return false;" ><span >Move down</span></a>';
    var select_field = '<span class="checkbox checkbox_u" ref="#"  value="1" onmousedown="checked_(this);return false;" /></span>';
    function oc(a){
    	var o = {};
    	for(var i=0;i<a.length;i++){
    		o[a[i]]='';
    		}
    	return o;
    	}
    function form_create_input(){
    	var newfield;
    	var label = document.getElementById('label').value;
    	var label_loc = document.getElementById('label_loc').value;
    	var groupname = document.getElementById('groupname').value;
    	var groupnamelegend = document.getElementById('groupnamelegend').value;
    	var field_name = document.getElementById('field_name').value;
    	var type = document.getElementById('type').value;
    	var text_value = document.getElementById('text_value').value;
    	var text_rows = document.getElementById('text_rows').value;
    	var text_cols = document.getElementById('text_cols').value;
    	var showinfo = document.getElementById('showinfo').value;
    	var sel_check_opt = document.getElementById('sel_check_opt').value.split(', ');
    	var form_preview = document.getElementById(form_values[0]).innerHTML;	
    	undo_last.unshift(form_preview);
    	showinfo = '<span>' + showinfo + '</span>';
    	var newlabel = '<label id="' + field_name + 'l" >';
    	if (type in (oc(input_a))){
    		newfield = '<input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />';
    		}
    	else if(type == 'textarea'){
    		newfield = '<textarea id="' + field_name + '" rows="' + text_rows + '" cols="' + text_cols + '">' + text_value + '</textarea>';
    		if (newlabel != ''){
    			if (label_loc == 1){
    				newfield = newlabel + newfield;
    				}
    			}
    		}
    	else if(type == 'select'){
    		var options = '';
    		for ( keyVar in sel_check_opt ) {
    			options = options + '<option value="' + sel_check_opt[keyVar] + '">' + sel_check_opt[keyVar] + '</option>';
    			}
    		newfield = '<select id="' + field_name + '" >' + options + '</select>';
    		}
    	if (label_loc == '1'){
    			newfield = newlabel + label + showinfo + newfield + button_remover + button_up + button_down + select_field + '</label>';
    			}
    	else {newfield = newlabel + newfield + label + showinfo + button_remover + button_up + button_down + select_field + '</label>';}
    	document.getElementById(form_values[0]).innerHTML = form_preview + newfield;
    	}	
    function revert_change(){
    	if(undo_last[0] != undefined){
    		document.getElementById(form_values[0]).innerHTML = undo_last[0];
    		undo_last.splice(0, 1);
    		}
    	else{alert('Nothing to undo.');}
    	}
    function remove_field(id){
    	var nodetoremove = id.parentNode;
    	var parent = document.getElementById(form_values[0]);
    	parent.removeChild(nodetoremove);
    	}
    function form_configurator(form_option){
    	var new_value = document.getElementById(form_option).value;
    	var the_form = document.getElementById(form_values[0]);
    	if (form_option == 'form_id'){
    		the_form.id = new_value;
    		form_values[0] = new_value;
    		}
    	else if (form_option == 'form_action'){
    		the_form.action = new_value;
    		form_values[1] = new_value;
    		}
    	else if (form_option == 'form_method'){
    		the_form.method = new_value;
    		form_values[2] = new_value;
    		}
    	document.getElementById('form_info').innerHTML = 'Current form id: <span class="form_info_hl" >"' + form_values[0] + '"</span><br/>Current action: <span class="form_info_hl" >"' + form_values[1] + '"</span><br/>Current method: <span class="form_info_hl" >"' + form_values[2] + '"</span>';
    	}
    function move_field(node, direction) {
    	var nodetomove = node.parentNode;
    	if (direction == 'previous'){
    		if (nodetomove.previousSibling.id == null){
    			if(warningsent == true){}
    			else{alert('Nowhere to move!');warningsent = true;}
    			}
    		else {
    			var thenode = nodetomove.previousSibling;
    			document.getElementById(form_values[0]).insertBefore(nodetomove, thenode);
    			warningsent = false;
    			}
    		}
    	else{
    		if (nodetomove.nextSibling == null){
    			if(warningsent == true){}
    			else{alert('Nowhere to move!');warningsent = true;}
    			}
    		else{
    			var thenode = nodetomove.nextSibling;
    			thenode = thenode.nextSibling;
    			document.getElementById(form_values[0]).insertBefore(nodetomove, thenode);
    			warningsent = false;
    			}
    		}
    	}
    function checked_(checked){
    	if(checked.value == '1'){
    		checked.className = 'checkbox checkbox_c';
    		checked.value = '2'
    		}
    	else{
    		checked.className = 'checkbox checkbox_u';
    		checked.value = '1'
    		}
    	}

    By the way the variables outside the functions are global, I will also upload the file to my host so you can view what the code is doing

  5. #5
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    This is a link to a working sample of the code

    link

  6. #6
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    by the way the
    Code JavaScript:
    if (type in (oc(input_a)))
    checks if an element exists in the array where type is a string coming from this select box:

    Code HTML4Strict:
    <select id="type" >
    <option value="button" >button</option>
    <option value="checkbox" >checkbox</option>
    <option value="file" >file</option>
    <option value="hidden" >hidden</option>
    <option value="password" >password</option>
    <option value="radio" >radio</option>
    <option value="reset" >reset</option>
    <option value="select" >select</option>
    <option value="submit" >submit</option>
    <option value="text" >text</option>
    <option value="textarea" >textarea</option>
    </select>

  7. #7
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by paul_wilkins View Post
    So instead of the following code:

    Code:
    if (type in (oc(input_a))){
    You can use this instead:

    Code javascript:
    if (input_a.indexOf(type) > -1) {
    I tried it and end it up like this:

    Code JavaScript:
    if (type in (oc(input_a))){
    		newfield = '<input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />';
    		}
    	/*if(input_a.indexOf(type) > -1){
    		newfield = '<input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />';
    		}*/

    The first if works in IE the second one does not, both work in FF, you can view the error here it says the object does not accept the property or method

  8. #8
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    The first if works in IE the second one does not, both work in FF, you can view the error here it says the object does not accept the property or method
    Did you also read about how it works on modern web browsers, but others like Internet Explorer require compatibility code to support that version 1.6 feature?
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  9. #9
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by paul_wilkins View Post
    Did you also read about how it works on modern web browsers, but others like Internet Explorer require compatibility code to support that version 1.6 feature?

    Oh, I guess I missed that part, it is working now and I removed the oc() function, thank you

  10. #10
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    Oh, I guess I missed that part, it is working now and I removed the oc() function, thank you
    I passed the script code through jslint.com to help deal with most of the obvious issues.

    After that, I found that the only indexOf that you were doing was in that one area. Since it's a list of input types that won't be varying, a more appropriate way to achieve the desired end result here is with a switch statement, which also allows you to get rid of the compatibility code.

    Then I updated the form_create_input() function by moving the new field part out to a separate function, where a switch statement is used to determine what to do based on the type:

    Code javascript:
    switch (type) {
    case 'button' || 'file' || 'hidden' || 'password' || 'radio' || 'reset' || 'submit' || 'text':
        newfield = '<input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />';
        break;
    case 'textarea':
        newfield = '<textarea id="' + field_name + '" >' + text_value + '</textarea>';
        break;
    case 'select':
        ...
    }

    I was also wondering what the unshift() part was for, then when seeing where you splice an item out, realised that it's where you're adding and removing undo states from stack. That part has been tidied up too so that it uses push() and pop() which more people find to be a more understandable way to manage such stacks.

    Code javascript:
    undo_last.push(form_preview);
    ...
    if (undo_last.length > 0) {
        document.getElementById(form_values[0]).innerHTML = undo_last.pop();
    } else {

    So with a few other minor tweaks, what we have now is a good starting point from which other improvements can be made:

    Code javascript:
    var form_values = ['form_preview', 'No action has been set', 'no method has been set'],
        undo_last = [],
        warningsent = false,
        button_remover = '<a class="button_remove buttons" href="#" onclick="remove_field(this);return false;" ><span >Remove field</span></a>',
        button_up = '<a class="button_up buttons" href="#" onclick="move_field(this, ' + "'previous'" + ');return false;" ><span >Move up</span></a>',
        button_down = '<a class="button_down buttons" href="#" onclick="move_field(this, ' + "'next'" + ');return false;" ><span >Move down</span></a>',
        select_field = '<span class="checkbox checkbox_u" onmousedown="toggle_checked(this);" /></span>';
     
    function newField(type) {
        var newfield = '',
            options = '',
            cb = '',
            cb_value = '',
            check = '',
            field_name = document.getElementById('field_name').value,
            text_value = document.getElementById('text_value').value,
            text_rows = document.getElementById('text_rows').value,
            text_cols = document.getElementById('text_cols').value,
            cb_groupname = document.getElementById('cb_groupname').value,
            sel_opt = document.getElementById('sel_opt').value.split(', '),
            cb_opt = document.getElementById('cb_opt').value.split(','),
            i;
        switch (type) {
        case 'button' || 'file' || 'hidden' || 'password' || 'radio' || 'reset' || 'submit' || 'text':
            newfield = '<input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />';
            break;
        case 'textarea':
            newfield = '<textarea id="' + field_name + '" >' + text_value + '</textarea>';
            break;
        case 'select':
            options = '';
            for (i = 0; i < sel_opt.length; i += 1) {
                options = options + '<option value="' + sel_opt[i] + '">' + sel_opt[i] + '</option>';
            }
            newfield = '<select id="' + field_name + '" >' + options + '</select>';
            break;
        case 'checkbox':
            for (i = 0; i < cb_opt.length; i += 1) {
                cb_value = cb_opt[i].split(':');
                check = '';
                if (cb_value[2] === 'c') {
                    check = 'checked="checked" ';
                }
                cb = cb + '<input type="checkbox" name="' + cb_groupname + '" value="' + cb_value[0] + '" ' + check + '/><span>' + cb_value[1] + '</span>';
            }
            newfield = '<p id="' + field_name + '" >' + cb + '</p>';
            break;
        default:
            // do nothing
        }
        return newfield;
    }
    function form_create_input() {
        var newfield = '',
            newlabel = '',
            label = document.getElementById('label').value,
            groupname = document.getElementById('groupname').value,
            groupnamelegend = document.getElementById('groupnamelegend').value,
            field_name = document.getElementById('field_name').value,
            type = document.getElementById('type').value,
            showinfo = document.getElementById('showinfo').value,
            form_preview = document.getElementById(form_values[0]).innerHTML;
     
        undo_last.push(form_preview);
        newlabel = '<label id="' + field_name + 'l" >';
        showinfo = '<span>' + showinfo + '</span>';
        newfield = newlabel + label + showinfo + newField(type) + button_remover + button_up + button_down + select_field + '</label>';
        document.getElementById(form_values[0]).innerHTML = form_preview + newfield;
    }
     
    function revert_change() {
        if (undo_last.length > 0) {
            document.getElementById(form_values[0]).innerHTML = undo_last.pop();
        } else {
            window.alert('Nothing to undo.');
        }
    }
     
    function remove_field(id) {
        var nodetoremove = id.parentNode,
            parent = document.getElementById(form_values[0]);
        parent.removeChild(nodetoremove);
        if ("undefined" !== typeof (event)) {
            event.returnValue = false;
        }
        return false;
    }
     
    function form_configurator(form_option) {
        var new_value = document.getElementById(form_option).value,
            the_form = document.getElementById(form_values[0]);
        if (form_option === 'form_id') {
            the_form.id = new_value;
            form_values[0] = new_value;
        } else if (form_option === 'form_action') {
            the_form.action = new_value;
            form_values[1] = new_value;
        } else if (form_option === 'form_method') {
            the_form.method = new_value;
            form_values[2] = new_value;
        }
        document.getElementById('form_info').innerHTML = 'Current form id: <span class="form_info_hl" >"' + form_values[0] + '"</span><br/>Current action: <span class="form_info_hl" >"' + form_values[1] + '"</span><br/>Current method: <span class="form_info_hl" >"' + form_values[2] + '"</span>';
    }
     
    function move_field(node, direction) {
        var nodetomove = node.parentNode,
            thenode;
        if (direction === 'previous') {
            if (!nodetomove.previousSibling.id) {
                if (!warningsent) {
                    window.alert('Nowhere to move!');
                    warningsent = true;
                }
            } else {
                thenode = nodetomove.previousSibling;
                document.getElementById(form_values[0]).insertBefore(nodetomove, thenode);
                warningsent = false;
            }
        } else {
            if (!nodetomove.nextSibling) {
                if (!warningsent) {
                    window.alert('Nowhere to move!');
                    warningsent = true;
                }
            } else {
                thenode = nodetomove.nextSibling;
                thenode = thenode.nextSibling;
                document.getElementById(form_values[0]).insertBefore(nodetomove, thenode);
                warningsent = false;
            }
        }
    }
     
    function toggle_checked(checked) {
        if (checked.className === 'checkbox checkbox_u') {
            checked.className = 'checkbox checkbox_c';
        } else {
            checked.className = 'checkbox checkbox_u';
        }
    }
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  11. #11
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    The remove_field function is now being tidied up, which before was:

    Code javascript:
    function remove_field(id) {
        var nodetoremove = id.parentNode,
            parent = document.getElementById(form_values[0]);
        parent.removeChild(nodetoremove);
        if ("undefined" !== typeof (event)) {
            event.returnValue = false;
        }
        return false;
    }

    There's no need to retrieve the parent from the global array, since the parent is available from the parentNode property. What's worse though is that if the nodeToRemove is not a direct descendant of the parent from your array, that will result in a script-killing error occurring.

    If the parent is obtained directly from the node that you want to remove, no such problems occur.

    The returnValue part of the script has also been removed, because that's only for Internet Explorer and you already have a return false, which achieves the same result.

    Code javascript:
    function remove_field(elem) {
        var nodetoremove = elem.parentNode,
            parent = nodetoremove.parentNode;
        parent.removeChild(nodetoremove);
        return false;
    }

    Next up is to deal to the horrible form_values[0] notation. Arrays are supposed to contain the same semantic items, but this breaks that idea. By making it an object with properties instead, the code becomes much easier to follow:

    Code javascript:
    var form_values = {
        id: 'form_preview',
        action: 'No action has been set',
        method: 'no method has been set'
    },

    You can then use form_values.id or form_values.action or form_values.method throughout your code, and other parts of your code become much shorter too:

    Code javascript:
    function form_configurator(form_option) {
        var the_form = document.getElementById(form_values.id),
            prop = form_option.substr(5),
            new_value = document.getElementById(form_option).value;
     
        the_form[prop] = new_value;
        form_values[prop] = new_value;
        document.getElementById('form_info').innerHTML = 'Current form id: <span class="form_info_hl" >"' + form_values.id + '"</span><br/>Current action: <span class="form_info_hl" >"' + form_values.action + '"</span><br/>Current method: <span class="form_info_hl" >"' + form_values.method + '"</span>';
    }

    Or, if you want to use a regular expression to get the property, you could do it like this:

    Code javascript:
    ...
    match = form_option.match(/form_(\w+)/),
    prop = match[1],
    ...

    Or you could just split it at the underscore:

    Code javascript:
    ...
    prop = form_option.split('_')[1]
    ...

    Either way, they seem to be better solutions than the series of if/else comparisons that were occurring before.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  12. #12
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    How do you configure jslint to check the code?

  13. #13
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    How do you configure jslint to check the code?
    Typically this one is enabled:
    [x] assume a browser

    And sometimes this one too:
    [x] tolerate missing 'use strict' pragma
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  14. #14
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I was asking because this: alert("Nowhere to move!");

    gives me an error: Problem at line 7 character 17: 'alert' was used before it was defined.

    alert("Nowhere to move!");

  15. #15
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    I was asking because this: alert("Nowhere to move!");

    gives me an error: Problem at line 7 character 17: 'alert' was used before it was defined.

    alert("Nowhere to move!");
    I prefer to use window.alert() which removes all doubt about the situation.

    If you prefer though, you can specify window as a predefined variable near the bottom of the page.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  16. #16
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I changed the function to move the fields from this:

    Code JavaScript:
    function move_field(node, direction) {
    	var nodetomove = node.parentNode;
    	var thenode = node.parentNode.previousSibling;
    	if (direction === 'previous') {
    		if (thenode.id === null) {
    			if (warningsent === false) {
    				window.alert('Nowhere to move!');
    				warningsent = true;
    			}
    		} else {
    			document.getElementById(form_values[0]).insertBefore(nodetomove, thenode);
    			warningsent = false;
    		}
    	} else {
    		if (node.parentNode.nextSibling === null) {
    			if(warningsent === true){}
    			else {window.alert('Nowhere to move!');warningsent = true;}
    		} else {
    			thenode = node.parentNode.nextSibling;
    			thenode = thenode.nextSibling;
    			document.getElementById(form_values[0]).insertBefore(nodetomove, thenode);
    			warningsent = false;
    		}
    	}
    }

    to this:

    Code JavaScript:
    function move_field(node, direction) {
         document.getElementById(form_values[0]).insertBefore(node, direction);
    }

    it does work, however assuming I have 3 fields, when I click the move up link in the top field twice it sends it at the bottom, and when I click the move down link on the bottom field it gives me a nextSibling null error, I tried catching the exemption with an if but I was not able to do it, how could I check if the nextSibling is null as this:

    if(nextSibling != null)

    did not work for me?

  17. #17
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    and when I click the move down link on the bottom field it gives me a nextSibling null error
    That's odd, because the insertBefore() method is designed to allow a null value as the second parameter, in which case it moves the element to the end of the list.

    Here's what it says on the Node.insertBefore page:

    If referenceElement is null, newElement is inserted at the end of the list of child nodes.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  18. #18
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I have made many changes based on your recommendations and here is what the code looks like right now, can you tell me what you think?

    Code JavaScript:
    var form_values = new Array('form_preview', 'No action has been set', 'no method has been set');
    var undo_last = [];
    var button_remover = '<a class="button_remove buttons" href="#" onclick="remove_field(this);return false;" ><span >Remove field</span></a>';
    var button_up = '<a class="button_up buttons" href="#" onclick="move_field(this.parentNode, this.parentNode.previousSibling);return false;" ><span >Move up</span></a>';
    var button_down = '<a class="button_down buttons" href="#" onclick="move_field(this.parentNode, this.parentNode.nextSibling.nextSibling);return false;" ><span >Move down</span></a>';
    var select_field = '<span class="checkbox checkbox_u" onmousedown="checked_box(this);" /></span>';
    function form_create_input(){
    	var newfield;
    	var label = document.getElementById('label').value;
    	var groupname = document.getElementById('groupname').value;
    	var groupnamelegend = document.getElementById('groupnamelegend').value;
    	var field_name = document.getElementById('field_name').value;
    	var type = document.getElementById('type').value;
    	var text_value = document.getElementById('text_value').value;
    	var text_rows = document.getElementById('text_rows').value;
    	var text_cols = document.getElementById('text_cols').value;
    	var showinfo = document.getElementById('showinfo').value;
    	var cb_groupname = document.getElementById('cb_groupname').value;
    	var sel_opt = document.getElementById('sel_opt').value.split(', ');
    	var cb_opt = document.getElementById('cb_opt').value.split(',');
    	var form_preview = document.getElementById(form_values[0]).innerHTML;	
    	undo_last.unshift(form_preview);
    	showinfo = '<span>' + showinfo + '</span>';
    	var newlabel = '<label id="' + field_name + 'l" >';
    	var options = '';
    	for ( keyVar in sel_opt ) {
    		options = options + '<option value="' + sel_opt[keyVar] + '">' + sel_opt[keyVar] + '</option>';
    	}
    	var cb = '<label><input type="checkbox" name="' + cb_groupname + '" value="selected" hidden="hidden" /></label>';		
    	for ( keyVar in cb_opt) {
    		var cb_value = cb_opt[keyVar].split(':')
    		var check = '';
    		if(cb_value[2] == 'c'){
    			var check = 'checked="checked" ';
    		}			
    		cb = cb + '<label><input type="checkbox" name="' + cb_groupname + '" value="' + cb_value[0] + '" ' + check + '/>' + cb_value[1] + '</label>';
    	}
    	switch (type) {
    		case 'text' || 'password':document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" >' + label + '<span>' + showinfo + '</span><input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />' + button_remover + button_up + button_down + select_field + '</label>';break;
    		case 'textarea':document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" >' + label + '<span>' + showinfo + '</span><textarea id="' + field_name + '" >' + text_value + '</textarea>' + button_remover + button_up + button_down + select_field + '</label>';break;
    		case 'select':document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" >' + label + '<span>' + showinfo + '</span><select id="' + field_name + '" >' + options + '</select>' + button_remover + button_up + button_down + select_field + '</label>';break;
    		case 'checkbox':document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" >' + label + '<span>' + showinfo + '</span><p id="' + field_name + '" >' + cb + '</p>' + button_remover + button_up + button_down + select_field + '</label>';break;
    	}
    	}	
    function revert_change() {
    	if (undo_last[0] != undefined) {
    		document.getElementById(form_values[0]).innerHTML = undo_last[0];
    		undo_last.splice(0, 1);
    	}
    }
    function remove_field(id){
    	document.getElementById(form_values[0]).removeChild(id.parentNode);
    	}
    function form_configurator(value, integer){
    	switch (integer){
    		case 0:document.getElementById(form_values[0]).id = value.value;break;
    		case 1:document.getElementById(form_values[0]).action = value.value;break;
    		case 2:document.getElementById(form_values[0]).method = value.value;break;
    	}
    	form_values[integer] = value.value;
    }
    function move_field(node, direction) {
    	document.getElementById(form_values[0]).insertBefore(node, direction);
    }
    function checked_box(checked) {
    	if (checked.className === 'checkbox checkbox_u') {
    		checked.className = 'checkbox checkbox_c';
    	} else {
    		checked.className = 'checkbox checkbox_u';
    	}
    }

  19. #19
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    I have made many changes based on your recommendations and here is what the code looks like right now, can you tell me what you think?
    Do you mind if I take a look at it after you have cleaned it up with jslint? Many of them will be "minor", but a lot of minor issues can still add up to a large problem.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  20. #20
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by paul_wilkins View Post
    Do you mind if I take a look at it after you have cleaned it up with jslint? Many of them will be "minor", but a lot of minor issues can still add up to a large problem.
    I will run it in Jslint and come back with the cleaned up code.

  21. #21
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I ran it through Jslint

    • Assume a browser
    • Tolerate missing 'use strict' pragma
    • Tolerate many var statements per function


    and this is the result

    Code JavaScript:
    var form_values = ['form_preview', '', ''];
    var undo_last = [];
    var button_remover = '<a class="button_remove buttons" href="#" onclick="remove_field(this);return false;" >Remove field</a>';
    var button_up = '<a class="button_up buttons" href="#" onclick="move_field(this.parentNode, this.parentNode.previousSibling);return false;" >Move up</a>';
    var button_down = '<a class="button_down buttons" href="#" onclick="move_field(this.parentNode, this.parentNode.nextSibling.nextSibling);return false;" >Move down</a>';
    var select_field = '<span class="checkbox checkbox_u" onmousedown="checked_box(this);" /></span>';
    function form_create_input() {
    	var newfield;
    	var label = document.getElementById('label').value;
    	var groupname = document.getElementById('groupname').value;
    	var groupnamelegend = document.getElementById('groupnamelegend').value;
    	var field_name = document.getElementById('field_name').value;
    	var type = document.getElementById('type').value;
    	var text_value = document.getElementById('text_value').value;
    	var text_rows = document.getElementById('text_rows').value;
    	var text_cols = document.getElementById('text_cols').value;
    	var showinfo = document.getElementById('showinfo').value;
    	var cb_groupname = document.getElementById('cb_groupname').value;
    	var sel_opt = document.getElementById('sel_opt').value.split(', ');
    	var cb_opt = document.getElementById('cb_opt').value.split(',');
    	var form_preview = document.getElementById(form_values[0]).innerHTML;	
    	var newlabel = '<label id="' + field_name + 'l" >';
    	var options = '';	
    	var cb = '<label><input type="checkbox" name="' + cb_groupname + '" value="selected" hidden="hidden" /></label>';
    	var check = '';
    	undo_last.unshift(form_preview);
    	showinfo = '<span>' + showinfo + '</span>';
    	switch (type) {
    	case 'text' || 'password':
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	case 'textarea':
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><textarea id="' + field_name + '" >' + text_value + '</textarea>' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	case 'select':
    		for (i = 0; i < sel_opt.length; i++) { 
    			options = options + '<option value="' + sel_opt[i] + '">' + sel_opt[i] + '</option>';
    		} 
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><select id="' + field_name + '" >' + options + '</select>' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	case 'checkbox':
    		for (i = 0; i < cb_opt.length; i++) {
    			var cb_value = cb_opt[i].split(':');
    			check = '';
    			if (cb_value[2] === 'c') {
    				check = 'checked="checked" ';
    			}			
    			cb = cb + '<label><input type="checkbox" name="' + cb_groupname + '" value="' + cb_value[0] + '" ' + check + '/>' + cb_value[1] + '</label>';
    		}
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><p id="' + field_name + '" >' + cb + '</p>' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	}
    }	
    function revert_change() {
    	if (undo_last[0] !== undefined) {
    		document.getElementById(form_values[0]).innerHTML = undo_last[0];
    		undo_last.splice(0, 1);
    	}
    }
    function remove_field(id) {
    	document.getElementById(form_values[0]).removeChild(id.parentNode);
    }
    function form_configurator(value, integer) {
    	switch (integer) {
    	case 0: 
    		document.getElementById(form_values[0]).id = value.value;
    		break;
    	case 1: 
    		document.getElementById(form_values[0]).action = value.value;
    		break;
    	case 2: 
    		document.getElementById(form_values[0]).method = value.value;
    		break;
    	}
    	form_values[integer] = value.value;
    }
    function move_field(node, direction) {
    	document.getElementById(form_values[0]).insertBefore(node, direction);
    }
    function checked_box(checked) {
    	if (checked.className === 'checkbox checkbox_u') {
    		checked.className = 'checkbox checkbox_c';
    	} else {
    		checked.className = 'checkbox checkbox_u';
    	}
    }

  22. #22
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I removed the for loop and replaced it for array.map

    Code JavaScript:
    var form_values = ['form_preview', '', ''];
    var undo_last = [];
    var button_remover = '<a class="button_remove buttons" href="#" onclick="remove_field(this);return false;" >Remove field</a>';
    var button_up = '<a class="button_up buttons" href="#" onclick="move_field(this.parentNode, this.parentNode.previousSibling);return false;" >Move up</a>';
    var button_down = '<a class="button_down buttons" href="#" onclick="move_field(this.parentNode, this.parentNode.nextSibling.nextSibling);return false;" >Move down</a>';
    var select_field = '<span class="checkbox checkbox_u" onmousedown="checked_box(this);" /></span>';
    function form_create_input() {
    	var newfield;
    	var label = document.getElementById('label').value;
    	var groupname = document.getElementById('groupname').value;
    	var groupnamelegend = document.getElementById('groupnamelegend').value;
    	var field_name = document.getElementById('field_name').value;
    	var type = document.getElementById('type').value;
    	var text_value = document.getElementById('text_value').value;
    	var text_rows = document.getElementById('text_rows').value;
    	var text_cols = document.getElementById('text_cols').value;
    	var showinfo = document.getElementById('showinfo').value;
    	var cb_groupname = document.getElementById('cb_groupname').value;
    	var sel_opt = document.getElementById('sel_opt').value.split(', ');
    	var cb_opt = document.getElementById('cb_opt').value.split(',');
    	var form_preview = document.getElementById(form_values[0]).innerHTML;	
    	var newlabel = '<label id="' + field_name + 'l" >';
    	var options = '';	
    	var cb = '<label><input type="checkbox" name="' + cb_groupname + '" value="selected" hidden="hidden" /></label>';
    	var check = '';
    	undo_last.unshift(form_preview);
    	showinfo = '<span>' + showinfo + '</span>';
    	switch (type) {
    	case 'text' || 'password':
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><input type="' + type + '" id="' + field_name + '" name="' + field_name + '" value="' + text_value + '" />' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	case 'textarea':
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><textarea id="' + field_name + '" >' + text_value + '</textarea>' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	case 'select':
    		sel_opt.map(function (i) { 
    			options = options + '<option value="' + i + '">' + i + '</option>';
    		}); 
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><select id="' + field_name + '" >' + options + '</select>' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	case 'checkbox':
    		cb_opt.map(function (i) {
    			var cb_value = i.split(':');
    			check = '';
    			if (cb_value[2] === 'c') {
    				check = 'checked="checked" ';
    			}			
    			cb = cb + '<label><input type="checkbox" name="' + cb_groupname + '" value="' + cb_value[0] + '" ' + check + '/>' + cb_value[1] + '</label>';
    		});
    		document.getElementById(form_values[0]).innerHTML = form_preview + '<label id="' + field_name + 'l" ><span>' + label + '<p>' + showinfo + '</p></span><p id="' + field_name + '" >' + cb + '</p>' + button_remover + button_up + button_down + select_field + '</label>';
    		break;
    	}
    }	
    function revert_change() {
    	if (undo_last[0] !== undefined) {
    		document.getElementById(form_values[0]).innerHTML = undo_last[0];
    		undo_last.splice(0, 1);
    	}
    }
    function remove_field(id) {
    	document.getElementById(form_values[0]).removeChild(id.parentNode);
    }
    function form_configurator(value, integer) {
    	switch (integer) {
    	case 0: 
    		document.getElementById(form_values[0]).id = value.value;
    		break;
    	case 1: 
    		document.getElementById(form_values[0]).action = value.value;
    		break;
    	case 2: 
    		document.getElementById(form_values[0]).method = value.value;
    		break;
    	}
    	form_values[integer] = value.value;
    }
    function move_field(node, direction) {
    	document.getElementById(form_values[0]).insertBefore(node, direction);
    }
    function checked_box(checked) {
    	if (checked.className === 'checkbox checkbox_u') {
    		checked.className = 'checkbox checkbox_c';
    	} else {
    		checked.className = 'checkbox checkbox_u';
    	}
    }

  23. #23
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    I removed the for loop and replaced it for array.map
    But if you still want Internet Explorer to also handle the code, you'll need to allow for it only supporting up to version 1.3 of JavaScript.
    Since array.map() is from version 1.6, the documentation page provides compatibility code to simulate its behaviour.

    With the map code, you shouldn't use i as the variable, because that implies that it will be a numerical indexing number. A variable called value should be more appropriate in that case there.
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript

  24. #24
    SitePoint Addict tlacaelelrl's Avatar
    Join Date
    Apr 2011
    Location
    Mexico city, Mexico
    Posts
    353
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I have made many changes since I first posted this thread and would like to get, if possible, a new opinion on the code.

    1. I ran it in Jslint and got no errors
      • Assume a browser
      • Allow multiple vars per function
      • Tolerate missing 'strict
    2. Removed as many variables as I could from within the function.
    3. Ended up with only one global variable.
    4. Added the compatibility mode for
      • !Function.prototype.bind
      • !Array.prototype.map
      • !Array.prototype.indexOf


    And this is the final code, I removed the compatibility code from here but I do have it in my script

    Code JavaScript:
    var form_values = ['form_preview', '', ''];
    function getTheData(id, property) {
    	return document.getElementById(id)[property];
    }
    function fieldBeginningData(i) {
    	if (i === 0) { 
    		return getTheData(form_values[0], 'innerHTML') + '<div id="' + getTheData('field_name', 'value') + 'l" class="basic_input_div" ><label for="' + getTheData('field_name', 'value') + '">' + getTheData('label', 'value') + '<\/label><span>' + getTheData('showinfo', 'value') + '<\/span>';
    	} else if (i === 1) {
    		return getTheData(form_values[0], 'innerHTML') + '<div id="' + getTheData('field_name', 'value') + 'l" class="';
    	}
    }
    function fieldEndData() {
    	return '" >' + document.getElementById('confbuttons').innerHTML + '<\/div><\/div>';
    }
    function validateBeforeAdding(adding) {
    	if (adding === 0) {
    		if (form_values.indexOf(getTheData('field_name', 'value')) === -1) {
    			form_values.push(getTheData('field_name', 'value'));
    			document.getElementById('form_javascript_data').lang = getTheData('field_name', 'value');
    			document.getElementById('form_javascript_data').innerHTML = '<div id="confbuttons" ><a onclick="removeElement(' + "'" + getTheData('field_name', 'value') + "'" + ');return false;" >Remove<\/a><a onclick="moveElement(' + "'" + getTheData('field_name', 'value') + "l', 'up'" + ');return false;" >Move up<\/a><a onclick="moveElement(' + "'" + getTheData('field_name', 'value') + "l', 'down'" + ');return false;" >Move down<\/a><a class="checkbox checkbox_u" onclick="checkedClassChange(this);" /><\/a><\/div>';
    		} else {
    			window.alert('The id of every field should be unique, please verify it and try again, id ' + getTheData('field_name', 'value') + ' is already in use');
    			document.getElementById('form_javascript_data').className = 'append_element_true';
    		}
    	}
    }
    function formCreateInput() {
    	var options = '';	
    	var cb = '<div id="' + getTheData('field_name', 'value') + 'l" class="radio_input_div"><p class="radio_checkbox_p" >' + getTheData('label', 'value') + '<\/p><span class="radio_checkbox_span" >' + getTheData('showinfo', 'value') + '<\/span>	<div class="radio_input_insidediv" >';
    	var radio = '<div id="' + getTheData('field_name', 'value') + 'l" class="radio_input_div"><p class="radio_checkbox_p" >' + getTheData('label', 'value') + '<\/p><span class="radio_checkbox_span" >' + getTheData('showinfo', 'value') + '<\/span><div class="radio_input_insidediv" >';
    	var check = '';	
    	validateBeforeAdding(0);
    	if (getTheData('form_javascript_data', 'className') === 'append_element_true') {
    		document.getElementById('form_javascript_data').className = 'append_element_false';
    		return false;		
    	}
    	switch (getTheData('type', 'value')) {
    	case 'button':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(1) + 'button_field_div" ><button type="submit" id="' + getTheData('field_name', 'value') + '" >' + getTheData('label', 'value') + '<\/button><div class="button_conf button_button_conf' + fieldEndData();
    		break;
    	case 'checkbox':/**/
    		getTheData('cb_opt', 'value').split(',').map(function (value) {
    			var cb_value = value.split(':');
    			check = '';
    			if (cb_value[2] === 'c') {
    				check = 'checked="checked" ';
    			}			
    			cb = cb + '<input id="' + cb_value[0] + '" type="checkbox" name="' + getTheData('cb_groupname', 'value') + '" value="' + cb_value[0] + '" ' + check + '/><label for="' + cb_value[0] + '" >' + cb_value[1] + '<\/label><br/>';
    		});
    		document.getElementById(form_values[0]).innerHTML = getTheData(form_values[0], 'innerHTML') + cb + '<\/div><div class="button_conf checkbox_button_conf' + fieldEndData();
    		break;
    	case 'file':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(0) + '<input type="file" id="' + getTheData('field_name', 'value') + '" /><div class="button_conf file_button_conf' + fieldEndData();
    		break;
    	case 'hidden':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(1) + 'basic_input_div" ><label for="' + getTheData('field_name', 'value') + '">Hidden field<\/label><span>With ID: ' + getTheData('field_name', 'value') + '<\/span><span class="hidden_filler" ><\/span><input value="' + getTheData('text_value', 'value') + '" name="' + getTheData('field_name', 'value') + '" type="hidden" id="' + getTheData('field_name', 'value') + '" /><div class="button_conf' + fieldEndData();
    		break;	
    	case 'password':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(0) + '<input type="password" id="' + getTheData('field_name', 'value') + '" value="' + getTheData('text_value', 'value') + '" /><div class="button_conf' + fieldEndData();
    		break;
    	case 'radio':/**/
    		getTheData('cb_opt', 'value').split(',').map(function (value) {
    			var cb_value = value.split(':');
    			check = '';
    			if (cb_value[2] === 'c') {
    				check = 'checked="checked" ';
    			}			
    			radio = radio + '<input id="' + cb_value[0] + '" type="radio" name="' + getTheData('cb_groupname', 'value') + '" value="' + cb_value[0] + '" ' + check + '/><label for="' + cb_value[0] + '" >' + cb_value[1] + '<\/label><br/>';
    		});
    		document.getElementById(form_values[0]).innerHTML = getTheData(form_values[0], 'innerHTML') + radio + '<\/div><div class=" button_conf radio_button_conf' + fieldEndData();
    		break;
    	case 'reset':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(1) + 'basic_input_div" ><input id="' + getTheData('field_name', 'value') + '" class="basic_input_reset" type="reset" value="' + getTheData('label', 'value') + '" /><div class="button_conf reset_button_conf' + fieldEndData();
    		break;
    	case 'select':/**/
    		getTheData('sel_opt', 'value').split(', ').map(function (value) { 
    			options = options + '<option value="' + value + '">' + value + '<\/option>';
    		}); 
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(0) + '<select id="' + getTheData('field_name', 'value') + '" >' + options + '<\/select><div class="button_conf select_button_conf' + fieldEndData();
    		break;
    	case 'submit': /**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(1) + 'button_field_div" ><button type="submit" id="button" >' + getTheData('field_name', 'value') + '<\/button><div class="button_conf button_button_conf' + fieldEndData();
    		break;	
    	case 'text':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(0) + '<input type="text" id="' + getTheData('field_name', 'value') + '" value="' + getTheData('text_value', 'value') + '" /><div class="button_conf' + fieldEndData();
    		break;
    	case 'textarea':/**/
    		document.getElementById(form_values[0]).innerHTML = fieldBeginningData(0) + '<textarea id="' + getTheData('field_name', 'value') + '" >' + getTheData('text_value', 'value') + '<\/textarea><div class="button_conf textarea_button_conf' + fieldEndData();
    		break;		
    	}
    }
    function removeElement(id) {
    	validateBeforeAdding();
    	if (document.getElementById(id + 'l').parentNode.nodeName === 'FIELDSET' && document.getElementById(id + 'l').parentNode.getElementsByTagName('div').length === 2) {
    		document.getElementById(form_values[0]).removeChild(document.getElementById(id + 'l').parentNode);
    	} else {
    		document.getElementById(id + 'l').parentNode.removeChild(document.getElementById(id + 'l'));
    	}
    	form_values.splice(form_values.indexOf(id), 1);
    }
    function formConfigurator(value, integer) {
    	switch (integer) {
    	case 0: 
    		document.getElementById(form_values[0]).id = value.value;
    		break;
    	case 1: 
    		document.getElementById(form_values[0]).action = value.value;
    		break;
    	case 2: 
    		document.getElementById(form_values[0]).method = value.value;
    		break;
    	}
    	form_values[integer] = value.value;
    }
    function moveElement(node, direction) {
    	validateBeforeAdding();
    	if (direction === 'up') {
    		if (document.getElementById(node).previousSibling) {
    			document.getElementById(node).parentNode.insertBefore(document.getElementById(node), document.getElementById(node).previousSibling);
    		}
    	} else if (direction === 'down') {
    		if (!document.getElementById(node).nextSibling) {
    			return false;
    		}
    		document.getElementById(node).parentNode.insertBefore(document.getElementById(node), document.getElementById(node).nextSibling.nextSibling);
    	}
    }
    function checkedClassChange(checked) {
    	if (checked.className === 'checkbox checkbox_u') {
    		checked.className = 'checkbox checkbox_c';
    	} else {
    		checked.className = 'checkbox checkbox_u';
    	}
    }
    function groupSelectedElements(els) {
    	validateBeforeAdding();
    	els = '<fieldset id="' + getTheData('groupname', 'value') + '" class="preview_div_style" ><legend>' + getTheData('groupnamelegend', 'value') + '<\/legend>';
        Array.prototype.slice.call(document.getElementById(form_values[0]).getElementsByTagName('a')).map(function (value) {        
    		if (value.className === 'checkbox checkbox_c') {
    			value.className = 'checkbox checkbox_u';
    			els = els + '<div id="' + value.parentNode.parentNode.id + '" class="' + value.parentNode.parentNode.className + '" >' + document.getElementById(value.parentNode.parentNode.id).innerHTML + '<\/div>';
    			document.getElementById(form_values[0]).removeChild(document.getElementById(value.parentNode.parentNode.id));
            }
    	});
    	document.getElementById(form_values[0]).innerHTML = els + '<\/fieldset>' + getTheData(form_values[0], 'innerHTML');
    }
    Do you get bothered because I do the same thing every day?
    Do you question why I do it?
    Then find something that you actually like doing!!!

    Stop thinking on what I do.

  25. #25
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,526
    Mentioned
    83 Post(s)
    Tagged
    3 Thread(s)
    Quote Originally Posted by tlacaelelrl View Post
    I have made many changes since I first posted this thread and would like to get, if possible, a new opinion on the code.
    It seems that the HTML structure of the web page has changed too. The old link to the test page doesn't seem to be valid anymore, so do you have a link to an existing test page?
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •