Thanks again, all of you for the comments, I have further updated the
page:
http://www.sunnysideup.co.nz/j/imageChange/
Nicolaas
See comments.
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "
http://www.w3.org/
TR/html4/strict.dtd">
<html>
<head>
<title>Sunny Side Up Webdesign - ImageChange</title>
<meta name="robots" content="all">
<script type="text/javascript">
/* <![CDATA[ */
You still don't need this in an HTML document.
var htmlSnippets = new Array();
Use [] instead of new Array(). I don't know why it is better, but
JSLint always complains when I do this.
var exOwner;
var exEl;
Every time I see these, I think of ex-owner, as in the previous
owner. The names currentOwner and currentEl would be clearer.
function showMe(optionNumber, owner) {
if(checkFunctions()) {
The name of this function doesn't really convey what it does. It
checks if a specific element exists, whether it has a style property,
etc. Furthermore, such a generic name is likely to collide with
another script in the future.
var elName = "option" + optionNumber;
var el = document.getElementById(elName);
You didn't check if the element exists or if it had a style property.
The latter is not strictly necessary as you checked the container
element's style property and I think if one element has it, the rest
follow.
el.style.display = "block";
You really shouldn't be using the display property for this (see the
other thread that Randy mentioned.)
owner.className ="highlight";
if(exOwner) {
exOwner.className = "";
exEl.style.display = "none";
}
exEl = el;
exOwner = owner;
return true;
No. You do not want to return true here. Your links reference
anchors, so you have no need to return false and doing so will break
the application when scripting is enabled and style isn't.
}
}
function checkFunctions() {
if(document.getElementById) {
var el = document.getElementById("imagePlaceholder");
if(el) {
if(el.className && el.style) {
return true;
}
return (el && el.className && el.style)
However, I don't think you need to worry about el.className.
}
}
}
/* ]]> */
As mentioned above, you can get rid of this now.
</script>
<style type="text/css" media="all">
body {margin: auto;width: 400px; position: absolute; top: 13%; left:
13%;}
I've read these body rules a few times now and they still don't make
sense to me. What are you trying to do here?
.highlight:link, .highlight:visited {color: white; background-color:
green;}
.menu a {display: block; width: 3em; height: 3em; background-color:
#ddd; float: left; text-align: center; }
#imagePlaceholder {clear: both; height: 300px; overflow: hidden;}
#imagePlaceholder img {display: block;}
#imagePlaceholder p {margin: 0; padding: 0; font-style: italic;
margin-bottom: 30px;}
</style>
</head>
<body>
<div class="menu">
<a href="#pic1" onclick="return !showMe(1, this);">1</a>
<a href="#pic2" onclick="return !showMe(2, this);">2</a>
<a href="#pic3" onclick="return !showMe(3, this);">3</a>
<a href="#pic4" onclick="return !showMe(4, this);">4</a>
<a href="#pic5" onclick="return !showMe(5, this);">5</a>
</div>
<div id="imagePlaceholder" class="holder">
<div id="option1"><a name="pic1"></a><img
Adding id's to the images (eg "pic1") will eliminate the need for the
empty anchors. This may raise compatibility issues though.
Alternatively, you can wrap the anchors around the images. If you do
keep the anchors, change the name attributes to id's (or use both with
the same values) as I believe the name attribute is deprecated for all
but form elements.
src="../../i/004.jpg" alt="image one"><p>caption for image one</p></
div>
<div id="option2"><a name="pic2"></a><img src="../../i/021.jpg"
alt="image two"><p>caption for image two</p></div>
<div id="option3"><a name="pic3"></a><img src="../../i/022.jpg"
alt="image three"><p>caption for image three</p></div>
<div id="option4"><a name="pic4"></a><img src="../../i/023.jpg"
alt="image four"><p>caption for image four</p></div>
<div id="option5"><a name="pic5"></a><img src="../../i/025.jpg"
alt="image five"><p>caption for image five</p></div>
</div>
<script type="text/javascript">
if(checkFunctions()) {
You are asking for trouble here. Your checkFunctions code checks for
the existence of an element and it may not be available at this point
(normally it will.) You really don't care if the element exists at
this point anyway (you only need to check it before referencing it.)
document.write("<style type='text\/css' media='all'>
#imagePlaceholder div {display:none} <\/style>");
Not to mention that style elements are not allowed in the body.
Combining initial feature detection with the check for the container
element's existence has led you down the wrong path. Here is where it
dead ends.
}
</script>
</body>
</html>