|
|
Posted by windandwaves on 10/23/05 00:04
Duncan Booth wrote:
> windandwaves wrote:
>
>> <body onload="hoverer('menu');">
>>
>> function hoverer(ulname) {
>> if (document.all && document.getElementById(ulname).currentStyle ) {
>> var navroot = document.getElementById(ulname);
>> var lis=navroot.getElementsByTagName("li");
>> for (i=0; i<lis.length; i++) {
>> var oldClassName = this.className;
>> lis[i].onmouseover=function() {this.className = ulname + "ie";}
>> lis[i].onmouseout=function() {this.className = oldClassName;}
>> }
>> }
>> }
>>
>> any comments / questions greatly appreciated.
>
> You could avoid multiple calls of getElementById by moving the
> declaration of navroot outside the if statement.
>
> You shouldn't use a global variable as the loop counter. This is
> likely to lead to subtle errors.
>
> The line 'var oldClassName = this.className' is a loop invariant, so
> you might as well move it outside the loop. The functions you define
> don't vary either, so those can be moved out as well.
>
> The onmouseout function is setting the li element's classname to the
> body's classname which probably isn't what you intended.
>
> A better way to do this is to use the fact that an element can have
> multiple classes. e.g.
>
> function hoverer(ulname) {
> function mouseOver() {
> this.className += ' hover';
> }
> function mouseOut() {
> this.className = this.className.replace(/ *hover\b/g, '');
> }
> var navroot = document.getElementById &&
> document.getElementById(ulname);
> if (document.all && navroot && navroot.currentStyle ) {
> var navroot = document.getElementById(ulname);
> var lis=navroot.getElementsByTagName("li");
> for (var i=0; i<lis.length; i++) {
> lis[i].onmouseover=mouseOver;
> lis[i].onmouseout=mouseOut;
> }
> }
> }
Thank you for that. That is awesome.
>
> Then your style rules can do things like:
>
> #menu li.hover, #menu li:hover {
> background-color: #123456;
> }
>
> One other point to consider is that you might want to attach other
> mouse over/mouse out event handlers directly to list elements, so it
> might be better to use the 'attachEvent' method to add an additional
> handler and not interfere with existing handlers. If you do this
> though it gets a bit more complicated as you can't just use 'this' to
> access the list element.
Navigation:
[Reply to this message]
|