[Code Review] Form builder

JavaScript
1

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.


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

While I’m heading off to sleepy-bye land, here are some preliminary notes to kick things off with:

[list][]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
[/list]

3

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:


if (type in (oc(input_a))){

You can use this instead:


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:


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:


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.


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.

4

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.


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

This is a link to a working sample of the code

link

6

by the way the

if (type in (oc(input_a)))

checks if an element exists in the array where type is a string coming from this select box:

<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

I tried it and end it up like this:

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

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?

9

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

10

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:


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.


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:


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';
    }
}
11

The remove_field function is now being tidied up, which before was:


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.


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:


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:


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:


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

Or you could just split it at the underscore:


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

Either way, they seem to be better solutions than the series of if/else comparisons that were occurring before.

12

How do you configure jslint to check the code?

13

Typically this one is enabled:
assume a browser

And sometimes this one too:
tolerate missing ‘use strict’ pragma

14

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

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.

16

I changed the function to move the fields from this:

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:

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

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.

18

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?


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

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.

20

I will run it in Jslint and come back with the cleaned up code.