Since Lasse already posted the solution, I will only pay attention to
the things that can be improved.
John McCarthy wrote:
Novice here - using notepad only .....
Notepad is your friend. You should stick to Notepad or use only source
code editors (I would recommend either eclipse with JavaScript Editor
Plugin 0.0.3 or Dreamweaver MX 6.1 using Homesite/Coders layout). And
never trust (JavaScript) source code you do not fully understand. Most
of it, especially on the Web, is badly flawed and gets you more in
trouble. Copy & pray does not pay. (Hey, I like the sound of that one!
:-))
function MenuBarIn(E)
{
var i;
with (document.getElementById(E.id))
Always check objects and properties before you access them:
if (document.getElementById) // Does such a property exist?
{ // if yes
var o = document.getElementById(E.id); // use it
if (o) // if successful
{
// do something
}
// do something more
}
See also http://pointedears.de.vu/scripts/test/whatami
But you do *not* need getElementById(...) here, since you are already
passing the object reference with `E' (`E.id' tells that):
if (E)
{
with (E)
{
// ...
}
}
is sufficient here.
{
style.backgroundColor = "steelblue";
[...]
style.borderLeftColor = "rgb(36,161,232)";
Since you always refer to the `style' property within the block,
you could equally use
if (E && E.style)
{
with (E.style)
{
// ...
}
}
However, if you can avoid it, do not use the `with' statement.
It is likely to produce script errors if not used thoughtfully
and is therefore deprecated in JavaScript 1.5. Reference the
object you intend to use instead:
// if the referenced object has the `style' property
if (E && typeof E.style != "undefined")
{
var s = E.style;
s.backgroundColor = ...
// ...
s.borderLeftColor = ...
}
It is good practice to watch for the case of identifiers. Usually
constructor functions should start with a capital letter while
properties and object references should not. Thus `e' instead of
`E' and `menuBarIn' instead of `MenuBarIn' is recommended here.
}
// global var
if (giSpanCnt < 14 )
Avoid global variables where you can. With this function, you could
have passed the value of `giSpanCnt' as second argument and let the
function return the changed value. You could have also passed a
reference to a container object as second argument and could change
the property of the object instead. In the latter case, you could
have left out the return of the new value.
{
// attempt to add 14 span id's each with an Event
for ( i = 0; i < 14 ; i++ )
{
var eleSpan = document.createElement( "SPAN" ) ;
giSpanCnt = giSpanCnt + 1 ;
eleSpan.innerHTML = goMenuArray[giSpanCnt] ;
eleSpan.id = eleSpan.uniqueID ;
eleSpan.className = gClassNameMenuObjects ;
//alert(eleSpan.id) ;
//eleSpan.id = "IdSpan" + giSpanCnt ;
//var curID = document.getElementById(E.id).lastChild ;
document.getElementById(E.id).appendChild(eleSpan) ;
// ERROR HERE - seems to attach for the First eleSpan but not the rest
// IOW - if the mouse enters the first Span I get back INCREASE - but then
I get back TypeMisMatch error
//eleSpan.attachEvent( "onMouseOver" , IncText() ) ;
Get used to a coding style that is easily readable and editable.
This also helps you from staying away from trouble: Not that much
work to change it, and omitted whitespace reduces file size while
errors with flawed implementations are less likely. Style also
includes consequent indentation of blocks (with space, not with tab),
e.g. the above should read (ignoring what is semantically wrong
with the code for now):
for (i = 0; i < 14; i++)
{
var eleSpan = document.createElement("SPAN");
giSpanCnt = giSpanCnt + 1;
eleSpan.innerHTML = goMenuArray[giSpanCnt];
eleSpan.id = eleSpan.uniqueID;
eleSpan.className = gClassNameMenuObjects;
//alert(eleSpan.id);
//eleSpan.id = "IdSpan" + giSpanCnt;
//var curID = document.getElementById(E.id).lastChild;
document.getElementById(E.id).appendChild(eleSpan) ;
// ...
}
PointedEars
