David Golightly wrote:
[...]
Quote:
Quote:
if(!e) var e = window.event;
This doesn't (or shouldn't) work. Instead use:
There's nothing technically wrong with it - if nothing is passed to the
e parameter, it will be undefined and hence evaluate to false.
However...
Quote:
e = e || window.event;
Is preferred. An old version of Safari requires:
var e = e || window.event;
There is nothing harmful about using var with an already declared
variable, though it should only be used when it is first declared.
Quote:
Why? Well, you're creating a new var e within the scope of that if
statement that won't necessarily be around outside that scope.
Javascript doesn't have block-level scope. A variable declared
anywhere in a function is available anywhere else in the function, but
not outside it. Importantly, functions can access variables declared
in "outer" functions but not "inner" functions:
function foo(){
function bar(){ ... }
}
bar has access to all the variables of foo, but foo has no access to
those in bar. Declaring a local variable in bar with the same name as
one in foo will mask the one in foo.
Quote:
I'm not
100% sure if that's the case here, since you don't use braces, but in
general it's bad practice and you should avoid declaring variables
using the "var" keyword within "if" statements like this (because it'll
get you into trouble when you use it within braces).
It may get you into trouble in other languages, but not javascript. It
is generally considered good practice to declare variables outside
blocks using var. Some like to declare them all at the start of the
function, others immediately before the block they'll be used in.
Different strokes... :-)
Quote:
You already have
a variable identified by "e" - passed in as an argument. So you don't
need to declare it again using "var".
>
Quote:
var trueTarget = (window.event) ? e.srcElement : e.target;
This will work, but more concisely:
>
var trueTarget = e.target || e.srcElement;
>
Quote:
switch(trueTarget.getAttribute('id')) //switch statement gets the id
Ok, I think this might be where your problem comes in. IE sometimes
doesn't return an attribute "id" and you have to use trueTarget.id, but
then Mozilla (correctly) doesn't recognize the "id" property, so you
have to use both:
Are you sure? I've never encountered a case where Mozilla (or any
other browser) barfed on element.id.
The OP's use of switch is totally unnecessary, the element id could be
extracted using a regular expression and placed directly into the call
to dropDownMenuShow. Similarly, that function doesn't need a switch
statement either. The whole concept of hard-coding the IDs is bad
practice and makes the whole thing a maintenance nightmare.
Quote:
I also noticed a lot of "copy-and-paste" code, such as code to make
elements visibly or invisible, repeated in multiple places. Why not
write a function that does this and call it? In fact a lot of your
code, while thorough, can be refactored for reuse and concision.
Anywhere you find yourself wanting to copy and paste some code, ask
yourself "Can I write a function that does this? What variables would
that have?" It's worth writing even one-line functions for
commonly-used statements. Imagine the work you'll have to do - and the
bugs you might introduce - if you have to change that statement in the
future!
Full agreement. The OP should look at pure CSS dropdowns, or at least
ones that use considerably less code.
--
Rob