Learning JS, trying to figure out createelement

[Ben Hallert]

Hi there,

I haven't done a lot of Javascript programming, so I'm not having great
luck debugging something I put together, and I was hoping someone here
could give it a quick lookover and maybe see the obvious mistake I'm
making.

My challenge is, I've been asked to make a change to an internal tool.
It has a screen with a tree control and a series of buttons at the top.
The buttons at the top control which form is submitted, and are
enabled/disabled based on what type of things are selected in the tree
below.

It currently uses radio buttons, but I've changed the items that are
parents to be checkboxes instead so I can have multiselect. To
maintain compatibillity with the other forms that expect a single
variable (named testclass_id) to be posted, I've made the following
action to be executed on submit:

function dealWithMultiSe lect ()
{
/*This function should parse through all tcl set checkboxes to see if
there are more then one
//selected. If so, it needs to create a new hidden field with the
same name for each so we can
//send multiple instances of testclass_id to fulfillments review.
//Suggested flow, go through all of the checkboxes and see if there
are more then one active.
//In a sub loop, for each found, add a new hidden field.
//We'll modify set_val to make sure that there isn't a combination of
tcs and tcls set*/
num = document.tc.id. length;
for (var i = 0; i < num; i++)
{
if (document.tc.id[i].checked == true && document.tc.id[i].type ==
"checkbox")
{
//if it's not the FIRST tcl selected, then create a new hidden
element named testclass_id
if (TotalTCLSelect ed)
{
var hidden = document.create Element('input' );
hidden.setAttri bute('type','hi dden');
hidden.setAttri bute('value',do cument.tc.id[i].value);
hidden.setAttri bute('name','te stclass_id');
document.getEle mentById(docume nt.appendChild( hidden));
}
TotalTCLSelecte d++;
}
}
}

As you can see, I'm trying to use createelement to add more hidden
variables of the same name (in this case, 'testclass_id') so that I'll
be emulating a multi-select GET on the next screen. The intention is
that it loops through all the inputs in the form and, if the item is
checked AND is of the type 'checkbox', it adds a new hidden input with
the value of that item. End result, it should submit multiple
instances of 'testclass_id'. I increment TotalTCLSelecte d so that it
doesn't start doing that until the second one it encounters on purpose,
that's also to maintain compatibillity with other forms.

Can anyone offer some suggestions? My main theory is that the problem
has to do with one or more of the following:
1. Can I really keep redefining the var hidden in the loop? And does
it get cleared each time?
2. Am I messing up the object inheritance by doing
document.create element instead of something like
document.tc.cre ateelement? (where tc is the name of the form being
parsed)
3. I didn't define TotalTCLSelecte d anywhere. It isn't erroring out,
but could this be silently killing it?
4. Other?

Thanks!

[Martin Honnen]

Ben Hallert wrote:

var hidden = document.create Element('input' );
hidden.setAttri bute('type','hi dden');
hidden.setAttri bute('value',do cument.tc.id[i].value);
hidden.setAttri bute('name','te stclass_id');
document.getEle mentById(docume nt.appendChild( hidden));

The last line is somehow nonsense, you should call appendChild on the
element you want to append the input to, probably the form element or a
descendant of the form e.g.
document.forms. formName.append Child(hidden);
The document.getEle mentById is not needed at all.
Also consider to set value and defaultValue of the input, otherwise some
browsers will not have the value set for the newly created input:
hidden.value = hidden.defaultV alue =
document.tc.id[i].value;

--

Martin Honnen
http://JavaScript.FAQTs.com/

[RobG]

Ben Hallert wrote:
[...]

num = document.tc.id. length;
If this is intended to get all the input elements of a form with
id="tc", you should consider getting all the form elements and then
use that:

var ele = document.forms['tc'].elements;

and then you could change:
for (var i = 0; i < num; i++)
{
if (document.tc.id[i].checked == true && document.tc.id[i].type ==
"checkbox")
to

for (var i = 0; i < ele.length; i++) {
if (ele[i].type == "checkbox" && ele[i].checked ) {

If you test if it's checked first, then the second test is more or less
redundant since only a checkbox can be checked. But you also depend on
non-checkboxes returning evaluating to false, which probably should
happen but may not.
{
//if it's not the FIRST tcl selected, then create a new hidden
element named testclass_id
if (TotalTCLSelect ed)
You haven't declared TotalTCLSelecte d. Do so near the start of your
script (before the for loop), and change the if statement:

var TotalTCLSelecte d = 0; // set to zero
...
if (TotalTCLSelect ed == 0) // or <1 or !> 0 whatever
{ ... }
TotalTCLSelecte d++;

If you are not going to use the value of TotalTCLSelecte d, make it a
boolean:

var TotalTCLSelecte d = false;
if (TotalTCLSelect ed == 0)
{ ... }
TotalTCLSelecte d = true;

Of course you may then want to change the name to 'pastFirstTCL' or
similar.
{
var hidden = document.create Element('input' );
hidden.setAttri bute('type','hi dden');
hidden.setAttri bute('value',do cument.tc.id[i].value);
You really need better names for variables, change "hidden" the
variable name to hiddenInput or something otherwise it is very
confusing for anyone trying to maintain your code.

Why not (if you've used my suggestion above and renamed 'hidden')

hiddenInput.val ue = ele[i].value;
hidden.setAttri bute('name','te stclass_id');
This will give all your hidden inputs the same name, which whilst
invalid HTML will likely not cause any problems at the client end
(unless you want to refer to them by id). Consider:

hiddenInput.nam e = 'testclass_id' + i;
document.getEle mentById(docume nt.appendChild( hidden));
}
TotalTCLSelecte d++;
}
I think your appendChild error has been well covered. To make life
much easier, why not include the hidden inputs in the HTML?

[...] 1. Can I really keep redefining the var hidden in the loop? And does
it get cleared each time?
It's OK.
2. Am I messing up the object inheritance by doing
document.create element instead of something like
document.tc.cre ateelement? (where tc is the name of the form being
parsed)
No. You create the element, then add it to a collection.
3. I didn't define TotalTCLSelecte d anywhere. It isn't erroring out,
but could this be silently killing it?
Very likely (it doesn't work for me). Just declare it before using it
in the if statement.
4. Other?

Of course I haven't tested any of the above very much since I only have
your snippet and no HTML. If you have any issues post your modified
code & I'll have another go.

--
Rob

[RobG]

RobG wrote:
[...]

if (TotalTCLSelect ed == 0) // or <1 or !> 0 whatever

That should have been:

if (TotalTCLSelect ed != 0) // or <1 or !> 0 whatever
--
Rob