[clug-progsig] Javascript Woes...

[Shawn]

Nick Wiltshire wrote:
> I have the following HTML:
> 
> <div id='PageList' style='background:#3F3F3F'>
> 	<ul class='Pages'>
> 		<li page_id='1' page_name='About'>removed unnecessary stuff</li>
> 		<ul class='Pages'>
> 			<li page_id='2' page_name='Sub Page'>removed unnecessary stuff</li>
> 		</ul>
> 	<li page_id='0' page_name='Home Page'>removed unnecessary stuff</li>
> 	</ul>
> 	<fieldset section_id='1' section_name='section'>
> 		<legend>section</legend>
> 		<ul class='Pages'>
> 			<li page_id='3' page_name='page'>removed unnecessary stuff</li>
> 		</ul>
> 	</fieldset>
> </div>
> 
> So, all I want to do is go through the (potentially nested) unordered lists 
> and fieldsets and rearrange them into a nice little menu that is similar in 
> structure. 
> 
> The first problem I found is IE thinks the second <ul> is a child of the first 
> <li>...clearly it isn't but ok I think I can get around it by traversing into 
> the <li> elements to dig it out.

This is because your second UL is not properly nested.  A list must 
contain list items only.  Same as a table can contain TRs and THs only 
(which in turn can contain TDs).  They way you have it set up:

<ul>
<li></li>
<ul>
   <li></li>
</ul>
</ul>

is not correct.  Change this to be like so:

<ul>
<li>
   <ul>
     <li></li>
   </ul>
</li>
</ul>

- include your nested list INSIDE a list item - even if that's the only 
content for your list item.



> 
> My (very ugly) test code:
> 
> function PageMenu(ev, mode, page_id, page_name, script_url)
> 	{
> 	//mode is either 'move' or 'copy'
> 	ev = ev || window.event;
> 	var event_position = EventPosition(ev);
> 	
> 	if(mode == "move")
> 		{
> 		menu_div.innerHTML = "Move " + page_name + " to:<br /><br />";
> 		}
> 	
> 	menu_div.innerHTML += ParsePageList(document.getElementById('PageList'), 
> page_id, script_url);
> 	
> 	menu_div.style.top = (event_position.y - 5) + 'px';
> 	
> 	menu_div.style.display = 'block';
> 	
> 	menu_div.style.left = (event_position.x - menu_div.offsetWidth + 5) + 'px';
> 	}
> 	
> function ParsePageList(element, page_id, script_url)
> 	{
> 	var html = "<ul style='padding-left:6px; margin-left:0px; 
> list-style:none'>\n";
> 	
> 	for(i in element.childNodes)
> 		{
> 		if((element.childNodes[i].nodeType) == "1")
> 			{
> 			if(element.childNodes[i].nodeName == "UL")
> 				{
> 				html += ParsePageList(element.childNodes[i], page_id, script_url);
> 				}
> 			if(element.childNodes[i].nodeName == "LI" && 
> element.childNodes[i].getAttribute('page_id') != page_id)
> 				{
> 				html += "<li><a href='" + script_url + "&amp;page_id=" + page_id 
> + "&amp;new_parent=page_" + element.childNodes[i].getAttribute('page_id') 
> + "'>" + element.childNodes[i].getAttribute('page_name') + "</a></li>";
> 				alert(element.childNodes[i].nodeName);
> 			//	html += ParsePageList(element.childNodes[i], page_id, script_url);
> 				}
> 			if(element.childNodes[i].nodeName == "FIELDSET")
> 				{
> 				html += "<li><a href='" + script_url + "&amp;page_id=" + page_id 
> + "&amp;new_parent=section_" + 
> element.childNodes[i].getAttribute('section_id') + "'>" + 
> element.childNodes[i].getAttribute('section_name');
> 				html += "</a></li>";
> 				html += ParsePageList(element.childNodes[i], page_id, script_url);
> 				}
> 			}
> 		}
> 	
> 	html += "</ul>\n\n";
> 	return html;
> 	}
> 
> Note the commented out line...
> 
> //html += ParsePageList(element.childNodes[i], page_id, script_url);
> 
> IE chokes on that line saying element.childNodes[i] is not an object even 
> though it used the same object 2 lines up in the if() !! *expletive*
> 
> Also, in the resulting list (if the line above is commented out so it doesn't 
> choke) the fieldset is traversed twice.
> 
> It works perfectly in FF, Opera, Konqueror...
> 
> anyone more enlightened care to take a stab? Feel free to mock the code :) I'm 
> just trying to make it work before I pretty it up.

With regards to your element.childNodes[i] part not working the second 
time, you might be better off to store this into a temp variable (you're 
using it more than once, so there's some optimization benefits, AND ease 
of typing, AND you lock down what element you are pointing at.  (i.e. 
var myelement = element.childNodes[i];)  This will probably take care of 
your second use of it not working.  WHY it's not working is a little up 
in the air, but I suspect it's in how you are using innerHTML.

innerHTML gets rendered as soon as it has a value.  Therefore, if you 
happen to be changing the contents of the list via innerHTML, the object 
pointed to by element.childNodes[i], can change.  Similar to trying to 
refer to options by index, after removing one or more...

Instead, this problem seems to be demanding the use of DOM coding.  You 
are even doing so already to some extent.  For instance If I wanted to 
create a list I can do so like this:

var mylist = document.createElement("ul");
var li1 = document.createElement("li");
var li2 = document.createElement("li");

li1.appendChild(document.createTextNode("List Item 1"));

var myanchor = document.createElement("a");
myanchor.href = "http://www.google.ca";
myanchor.appendChild(document.createTextNode("Google"));
li2.appendChild(myanchor);

mylist.appendChild(li1);
mylist.appendChild(li2);
document.appendChild(mylist);

The code should be more or less self explanitory...  Create a few 
elements, assign text through the use of document.createTextNode(), and 
when you're done, append the elements to their respective parents.

The key here is that absolutely no rendering is done until the very last 
line where we append the list to the document (or div, or span, or para, 
or whatever element we can get a reference to).  In addition, you have 
much tighter control over elements in memory - you already have a 
variable reference to them.

A couple of notes though:
1)  To set the CSS class for an item created like this, you use the 
.className property.  Using .class will not work.
2)  If you later want to refer to the text element of the item, you do 
so like this:

var text1 = li1.childNodes[0].nodeValue
var text2 = li2.childNodes[0].childNodes[0].nodevalue;

- For li1, which only contains text, we want to talk to the text element 
(think in XML terms here), which is a sub element of the li element.
- for li2, we have an anchor tag, so we have to get reference to the 
anchor tag, and THEN to the text of that tag.  The first childNodes[0] 
points to the anchor, the second to the text node.
- If you know your item only has a single child, you could substitue 
.firstChild for .childNodes[0].  If seen discussion saying the 
.childNodes[] approach is better and more cross browser, but I've also 
seen the .firstChild work fine in both IE and FF.

OK, and now that we've covered all the hard parts....
WHY ON EARTH DO YOU NEED JAVASCRIPT TO CREATE A MENU FROM THE LISTS?????
Why not just use CSS?  Check out the page at 
http://family.open2space.com/files/layoutSample1.htm to see a sample of 
this.  The menu across the top is defined as an unordered list.  (view 
source will help)  Or a google search for "css list menu" will lead to a 
few helpful sites.  (assuming you are not already familiar with this 
approach).

I'm assuming there is some other business need for the JS approach?  Or 
are you needing the drop down menus too (which don't work in IE).  IF 
that's the case, check out http://www.milonic.com - not free, but a hell 
of a lot cheaper than trying to write your own dynamic menu system. 
There's free alternatives as well.

Shawn