:) What do you think of the code now?

**Bob90** · 05-22-2007, 12:00 PM

http://www.robertgarford.com/code/ra...1_5_strict.htm

I've been through the code and OO some of it. Will finish the rest after exams.
Rewritten to remove browser detection.
Added fall backs in certain situations, like eventhandlers.
Used mostly DOM2(?) methods.

Is there anything obvious/stupid that I have/haven't done.

Thanks in advance.

**Trinithis** · 05-22-2007, 04:09 PM

I think its pretty cool. Perhaps I missed something, but I'm assuming it's just the radial menu you coded. Though, I found the text on the page background hard to read and had to hit ctr+a. I do not fully understand the red "Here" box floating in the middle of the page in one part. I noticed that when you tell the radial menu to kill itself, it docks there, but I haven't seen it do that elsewhere. If that is intentional, I would suggest that you would be able to move the red docking area with your mouse or something so it does not block the page.

Right now I think I found a bug (BTW im using FF 1.0.7 on Windows). When you tell the menu to kill itself and the code sends it to the red box, you can see a miniturized menu there and you can click on it to bring back the menu. If you select kill menu again, it puts in the mini-menu into the box along with an extra mini-menu picture. Doing this many times yeilds more and more.
(My mouse is hovering over one, but the Print Screen did not capture it.)

One suggestion is to add like a new window or a new tab option for the links in the menu.

**Bob90** · 05-22-2007, 05:21 PM

Hey, thanks for your input.

I had already found that 'bug'. You can see my comment on it in the code. What really interesting is that the bug only affects FF, not IE and if you undock then click somewhere else, then re-dock the menu it doesn't happen! Weird. I will look at my code more thoroughly after my exams.
EDIT: In fact, I may post a new topic on why FF does this... may be a bug!

my code comment

Code:

//Dock to div only show one pic

slightly cryptic

As for opening in a new window. It already supports that

. Well it supports anchor target, so it can target certain frames, new windows or itself.

The reason the red div is floating is just to show that the menu can be docked to any div, anywhere! The user can place the div anywhere on their page.

**Trinithis** · 05-22-2007, 05:32 PM

Well, I have a suggestion, but it might be too much work, but it could be fun. I read that you could insert any number of images into the menu, and if too many are inserted then it would have a "more" button. Well, my idea is that instead, the menu just tacks on more rings and evenly distrubutes options within each ring. If one or two options are in an outer ring, then the outer ring would commandeer at least X options from the previous ring to make the outer have at least three in its ring, triangulating them, etc.

**Bob90** · 05-22-2007, 05:51 PM

Firstly, you've caught me out. That was left on from version 1.0, which I consider obselete. V1.0 has that option, but I have considered different ways to fit more items on the menu since then (it has to do with the way the user navigates).

Your idea is great and I still think it a possibilty.

The more I read, the more I think that, in fact, having submenus pop-up over the items would be more advantageous.

I will try and code your idea into version 2, which will be complete by August (Planned)

**blm126** · 05-23-2007, 03:09 AM

First off let me say this, I find the concept interesting, and it seems to work well. However, there are a lot of problems with the code that I see. So in no particular order...

-The menu syntax is weird. What if you want to use the | in the test or image. Sub-arrays would be smarter.

-Misuse of parseInt

Code:

this.index = parseInt(this.parent.items.length,10);

The above code doesn't need to be converted to a number, it should already be one, and even if it does need converted just use the Number constructor.

-IE only code?

Code:

			if(document.attachEvent)
				{
				temp_obj = document.getElementById("id_a"+x);
				//temp_obj.attachEvent("onmouseover",function(){rollover('id'+randId,picDir+tempAr[3]);highlight('id'+randId+'');goTo=tempAr[0];} );
				//temp_obj.attachEvent("onmouseout",function(){rollover('id'+randId,picDir+tempAr[2]);lowlight('id'+randId+'');goTo='';} );
				temp_obj.onmouseover = function(){rollOver(this);goTo=tempAr[0];};
				temp_obj.onmouseout = function(){rollOver(this);goTo='';};
				}

In the above code you seem to test for IE's attachEvent(and maybe Opera,can't remember), but then you use the old style of events, which is fine. But this either outdated code, or browser sniffing. Both are bad.

-Browser Sniffing

Code:

	var ie5 = document.all;

Ok, now that is browser sniffing for sure. Use Conditional Compilation, it's safer.Opera, Firefox, and all versions of IE6-7 actually support document.all. This is only working because Firefox actively lies to you.

-Browser Sniffing(again)

Code:

		if(navigator.appName == "Microsoft Internet Explorer" && parseFloat(navigator.appVersion.split("MSIE")[1]) > 5.5 && el.src.toUpperCase().substring(el.src.length-3, el.src.length) === "PNG") //if ie and png
			{
			el.style.filter = "progid:DXImageTransform.Microsoft.AlphaImageLoader(src='"+el.src+"',sizingMethod='scale')";
			}
		}

Use Conditional Compilation, trust me.
-Nested if-else-if-else staments

Code:

	else
		{
		if(document.attachEvent)
			{

Use else if, it's more readable.
-Why?

Code:

function returnFalse()
	{
	return false;	
	}

As you can see, my biggest complaint is that this isn't really cross-browser. IE support is held together by metaphorical duck tape(document.all). It's liable to break at any time. If you spend the time to make this truly cross-browser, it would be much better. Also, I found the formatting of all the code odd. Consistent spacing makes code more readable, but that is your choice.

**StarrRider** · 05-23-2007, 03:52 AM

Personally - I like it!
Good Job
Lee

**Bob90** · 05-23-2007, 07:59 AM

Thanks for the advice.

BLM126,

Nice catches. TY.

Some Questions/Points

1. Browser sniffing for PNG correction is a must. One of the above is for that reason.

2. why return false - Good Point! I can't remember why.

3. Why doc.attachEvent only to use obj.onmouseover = .... - I had problems dynamically adding eventhandlers to dynamically created elements in IE.
Do you know of the correct way?

4. Sub arrays - yeah I suppose so