Help with dw_scrollObj.js

[JohnSimoudis]

Hello all. I'm designing a new site and wanted to design a scrolling div with thumbnails that when clicked open to larger.
Everything works with the exception of the scrolling div. I can't get the arrows to scroll left or right.

Can you please take a look at: http://www.simoudis.com/Tage-Site/test.html
and see if there is something I'm doing wrong?

I'm sorry, I'm new at Javascript.

Thank you in advance.

John

[Jesdisciple]

First, note that I don't expect you to implement all this (or even get all of it) in one go. You'd have to be a genius or something for that. I just want you to give all of it your best effort and ask questions.

From Firefox's Error Console (Tools > Error Console):

Error: scrl_out is not defined
Source File: http://www.simoudis.com/Tage-Site/test.html
Line: 1

Error: scrl_over is not defined
Source File: http://www.simoudis.com/Tage-Site/test.html
Line: 1

Error: document.getElementById(img) is null
Source File: http://www.simoudis.com/Tage-Site/js/gallery.js
Line: 17

The first two errors are in your scrolling buttons' onclick attributes because they call non-existent functions; the last is from the onclick attributes which call the selectThumb function. The argument passed to selectThumb always contains 2 digits, whereas the id attributes only contain 1 digit except for the number 10. The argument to document.getElementById must be an exact, character-for-character match for the id of the element you expect to receive from it.

In most cases, including this one, you shouldn't use an event-handler attribute; it makes a mess of the HTML. The same purpose may be accomplished, without mixing layout and behavior, by finding the DOM elements via document.getElementsByName when the page loads and assigning a function to the property of each which is spelled just like the attribute.

I said to use document.getElementsByName rather than document.getElementById because an id is like a first name (e.g. "John") while a name is like a last name (e.g. "Doe"). Multiple elements may be found by the same name, but only one element (the first matching one) may be found by an id. You were using multiple ids, but that introduces clutter when the same thing should happen to all the elements.

And you don't need to use document.getElementById in selectThumb. Get rid of the a element, assign a function to the onclick property of the img, and pass this (the img) to the selectThumb function.

Speaking of document.getElementById... When you get anything from a function and you need to do several things with it, you shouldn't call the function for everything you need to do. That generates unnecessary overhead; assign the return value to a variable and use the variable for every task.

[JohnSimoudis]

Chris:

Thanks for the reply. I must admit I'm more confused than ever. I understood the error with the naming convention which I took care of and the thumbs appear better.

Can we address one thing at a time please so I can fix as well as learn it?

This is unclear to me:
And you don't need to use document.getElementById in selectThumb. Get rid of the a element, assign a function to the onclick property of the img, and pass this (the img) to the selectThumb function.

Can you please give me an example?

Thank you.

John

[Jesdisciple]

First, we need to go over one of my more mundane comments as a foundation:

In most cases, including this one, you shouldn't use an event-handler attribute; it makes a mess of the HTML. The same purpose may be accomplished, without mixing layout and behavior, by finding the DOM elements via document.getElementsByName when the page loads and assigning a function to the property of each which is spelled just like the attribute.

So now we have this HTML... (Note that I'm using the shorter naming convention because the longer one botches #10.)

<a href="javascript:;"><img  id="thumb1" src="images/Aspen_Selections/_MG_5108_thumb.jpg" border="0"></a>

and this JavaScript...

var thumb1 = document.getElementById('thumb1');
thumb1.onclick = function (){
	loadFull('image1');
	selectThumb('thumb1');
};

Do you see the redundancy here? We twice go to all the trouble of finding the first element with id="thumb1". This is like looking up a citizen's unique identification number in a government database twice for two parts of the same task. If we keep it handy we can save time, so change selectThumb's argument to this and change selectThumb to the following. (this refers to whatever object owns the current function, i.e. the object of which the current function is a property. In this case, it's thumb1.)

function selectThumb(img){
	for(a=1;a<75;a++){
		if(document.getElementById('thumb'+a)){
			document.getElementById('thumb'+a).style.opacity = '.5';
			document.getElementById('thumb'+a).style.filter = 'alpha(opacity=50)';
		}	
	}
	img.style.opacity = '1';
	img.style.filter = 'alpha(opacity=100)';
}

Now, to generalize that first script:

var thumbs = [];
var images = [];
for(var i = 1; i <= 10; i++){
	thumbs[i] = document.getElementById('thumb' + i);
	images[i] = document.getElementById('image' + i);
	thumbs[i].onclick = function (){
		loadFull(images[i]);
		selectThumb(thumbs[i]);
	};
};

But hold on... That doesn't work! By the time the thumb can be clicked, the loop has concluded and i is 10, so that code will always show the last image. Bugfix:

var thumbs = [];
for(var i = 1; i <= 10; i++){
	thumbs[i] = document.getElementById('thumb' + i);
	thumbs[i].image = document.getElementById('image' + i);
	thumbs[i].onclick = function (){
		loadFull(this.image);
		selectThumb(this);
	};
};

Can you work document.getElementsByName into that?