PDA

View Full Version : What is wrong with this code?



psilos
05-24-2007, 01:18 PM
function show(){
document.getElementById('main02').style.filter = "none";
}
function hide(){
document.getElementById('main02').style.filter = "gray";
}
window.onload = function() {
//hide();

var menu = document.getElementById('chromemenu');
var menuitems=document.getElementById('chromemenu').getElementsByTagName("a");
for (var i=0;i<menuitems.length;i++){
menuitems[i].onmouseout = hide();
menuitems[i].onmouseover = show();
}

}


Is something wrong with this code?
I want to add mouseover and mouseout events in all "a" tags inside the div with id "chromemenu".

The problem is that instead of adding the events, it executes the hide() function..

Any Help??

Trinithis
05-24-2007, 03:39 PM
I think you just need to take off the parenthesis on the show and hide for these:
menuitems[i].onmouseout = hide(); ---> menuitems[i].onmouseout = hide;
menuitems[i].onmouseover = show(); ---> menuitems[i].onmouseover = show;

Bob90
05-24-2007, 04:19 PM
I think filters only apply to IE as well, so you're missing out on a lot of people.

shachi
05-24-2007, 04:34 PM
Try this:



menuitem[i].onmouseout = function(){
hide();
}
menuitem[i].onmouseover = function(){
show();
}

jscheuer1
05-24-2007, 05:18 PM
I'm not sure about 'gray' but 'none' is not a filter, I don't think 'gray' (in and of itself as you've written it there) is either. And, as noted earlier, filters are for IE only.

Although the other two suggestions given above are good ones as far as general coding practices go, the entire code looks like garbage to me even with those improvements.

Since this is a request for a modification to an existing DD script, it would be better to post a question in the dynamic drive scripts help section and, rather than offering your beginner's attempt at solving your problem, describe in detail, in plain English the effect you want to achieve.

Twey
05-24-2007, 05:38 PM
the other two suggestions given above are good ones as far as general coding practices go

menuitem[i].onmouseout = function(){
hide();
}
menuitem[i].onmouseover = function(){
show();
}This one isn't. There's no need to create separate functions here.

jscheuer1
05-24-2007, 06:48 PM
This one isn't. There's no need to create separate functions here.

Sure it is. Just not in this situation. Ooops, since this situation really isn't a situation, I guess it is up to interpretation. I simply meant they can work if needed in otherwise well written code, but that the whole thing was such a mess that these adjustments (both of them) are relatively meaningless.

I do agree with what you seem to be saying in principal - If the function itself can be assigned, there is no reason to create another function simply to do so. However, there is no need to assign the functions directly either, as it still won't make any difference. Until the correct code to achieve the desired effect (whatever that is) can be determined, there's no way to be certain of the best way to assign it to events. I do prefer Trinithis' idea in those situations where it will do the trick. There are cases where the assignment needs to be determined by a test though (perhaps whether to use filters or some other method, depending upon availability). In some of these cases shachi's approach might be ideal.

psilos
05-29-2007, 07:08 AM
Ok thanx for your answers everybody...

Sorry for late answering.. But i was a bit offended by jscheuer1 posts..

He has helped many times in the past and i wasn't expecting such answers..

Anyway, i guess the problem was that i have accidentaly used the word chromemenu in my code...

My example is irrelevant with DD scripts. I post it here because it is javascript problem.

I know filters are only for IE. If you look at http://www.w3schools.com/dhtml/dhtml_css.asp you will see that there is a gray filter like that i use inside my "garbage" code..

My problem was to attach mouse events in all A tags inside a Div tag..

That's it. Nothing more, nothing else...

OK, and if u know any way to have a grayscale effect in a Div conpatible with all browsers i'll be happy to learn..

jscheuer1
05-29-2007, 07:58 AM
You've gathered all the a tags but the events you are assigning to them only act directly on the element with the id of 'main02'.

There still is no such filter as 'none'. The gray filter would be better applied using its formal name, if used at all. Images may be made gray scale in an image program and swapped with their color counterparts, then all browsers will see them as gray/colored depending upon their 'state'. The style color (and background-color) gray (and various shades of gray) are available for any text.

Without seeing the markup, it is hard to be more specific.

Bob90
05-29-2007, 07:58 AM
Hey,

Unless you are a great writer, sometimes your writing is going to come across as rude; thats the reason most fights start on msn. Obviously jscheuer didn't mean to offend, but he knows code.



function filterSwap(id,type){
document.getElementById(id).style.filter = type;
}

window.onload = function() {
var menuitems = document.getElementById('chromemenu').getElementsByTagName("a");
var i;
for (i=0; i<menuitems.length; i++){
if(document.attachEvent){
menuitems[i].attachEvent('onmouseover',filterSwap('main02','none'));
menuitems[i].attachEvent('onmouseout',filterSwap('main02','gray'));
}
}
}

jscheuer1
05-29-2007, 09:12 AM
I still think the code is poor. Filters just aren't the way to go with this. I did some experimenting and the 'none' string seems to work at cancelling out the gray filter, even though it shouldn't. But, in IE 7, using any filter, including 'none' causes any text to lose its anti-aliasing quality - making it look ragged and faint. Also, the gray filter will not apply if the element it is being applied to doesn't have 'layout'.

psilos
05-29-2007, 10:49 AM
Ok i know he didn't mean to offend.

I know the best way is to make 2nd grayscaled image and swap them...

i tried to avoid it because inside div "main02" i load images 330x490px
which can have some kb size... So i didn't want to load an extra image with some extra ..kb size.

The concept is something like that:

In every section there loads a different image(330x490px) in the same position..
And when users will try to leave that Section and go to another, by hovering over the menu (A tags), image will become grayscaled.

Something like when u log out of an OS..;)

So i tried to solve it with just few lines reusable code instead of duplicating all images and assigning to them different swap states.

So i guess the best solution for me would be if i find script that applies the grayscale effect in all major browsers.