change images

D

David Mark

Hi Folk and Gurus

This may help some of you (though probably not the gurus), in changing
images:

http://www.sunnysideup.co.nz/j/imageChange/

Any feedback appreciated.

XHTML 1.1 is not appropriate for Web pages. Certainly you don't want
to serve it as text/html.

Preloading the images via an innerHTML assignment is odd and far less
compatible than the usual method of creating Image objects.

Unless you plan to create a page to display the pictures without
scripting, you should make the images, captions and containing DIV's
static content. You don't need to use innerHTML at all with this
approach. Just display one DIV at a time.
 
T

Thomas 'PointedEars' Lahn

windandwaves said:
This may help some of you (though probably not the gurus), in changing
images:

http://www.sunnysideup.co.nz/j/imageChange/

Any feedback appreciated.

Works in Firefox 2.0.0.6 and Opera 9.22 on Windows XP, however:

- You are serving XHTML 1.1 as text/html, which is recommended
against per W3C Note "XHTML Media Types".
- You are serving XHTML that is not HTML-compatible as per XHTML 1.0,
Section C, as text/html. (We are gracefully ignoring that there is
no such allowance of HTML compatibility for XHTML 1.1 anyway.)
- You are serving XHTML where HTML 4.01 would have sufficed.
- You are declaring UTF-8 in a meta element that is ignored by an XML
parser; the corresponding XML declaration, which would not be required
if the Content-Type header was set appropriately, is missing.
- You are using the proprietary "author" value instead of the
"DC.Creator" or "DCTERMS.creator" from the Dublin Core Metadata
standard (ISO 15836-2003).
- By serving as text/html (see above) this is parsed by an
SGML-based parser. However, ETAGOs in the `script' element have not
been escaped.
- Does not work in NN4 because of "className" switching, although NN4
is capable of both hiding and switching the image and its description.
- The feature test for all W3C DOM methods called is missing.
- You are mixing W3C DOM Level 2 HTML with the proprietary innerHTML
property without testing for the latter.
- W3C DOM Level 2 and innerHTML are used without being required,
and no alternative is provided.
- Vertical offset of the image is too great (wrong) in IE 7.
- Hard to maintain because images are identified by number instead
of by name.

And probably I have forgotten something else.


PointedEars
 
W

windandwaves

XHTML 1.1 is not appropriate for Web pages. Certainly you don't want
to serve it as text/html.

Preloading the images via an innerHTML assignment is odd and far less
compatible than the usual method of creating Image objects.

Unless you plan to create a page to display the pictures without
scripting, you should make the images, captions and containing DIV's
static content. You don't need to use innerHTML at all with this
approach. Just display one DIV at a time.

Thanks for your help, I have made the changes....
 
W

windandwaves

windandwaves said the following on 8/4/2007 4:51 AM:



That would have to be about the worse way/example of an image change
script as I have seen in a very long time.

--
Randy
Chance Favors The Prepared Mind
comp.lang.javascript FAQ -http://jibbering.com/faq/index.html
Javascript Best Practices -http://www.JavascriptToolbox.com/bestpractices/

I have changed it...
 
D

David Mark

Thanks for your help, I have made the changes....

Looks better. See notes below.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN""http://www.w3.org/TR/
html4/strict.dtd">

Missing a space before the DTD.

<html>
<head>
<title>Sunny Side Up Webdesign - ImageChange</title>
<meta name="robots" content="all">
<script type="text/javascript">
/* <![CDATA[ */

You don't need this now.

document.write("<style type='text\/css' media='all'>
#imagePlaceholder div {display: none;} <\/style>");

Use visibility:hidden instead of display. And don't write this unless
the features you need to show the images (eg gEBI) are present.

var htmlSnippets = new Array();
var exOwner;
var exEl;
function showMe(optionNumber, owner) {
var elName = "option" + optionNumber;
var el = document.getElementById(elName);

Check if el (and el.style) exists before proceeding. Example:

if (el && el.style) ...

el.style.display = "block";
owner.className ="highlight";
if(exOwner) {
exOwner.className = "";
exEl.style.display = "none";
}
exEl = el;
exOwner = owner;
return true;
}

/* ]]> */

</script>
<style type="text/css" media="all">
body {margin: auto;width: 370px; position: absolute; top: 13%; left:
13%;}

What is the purpose of the body positioning?

.highlight:link, .highlight:visited {color: red; background-color:
green;}

What will a red-green color-blind person make of this? I have good
vision and I can't read the text.

.menu a {display: block; width: 30px; height: 30px; background-
color: #ddd; float: left; text-align: center; }

Don't dimension elements containing text with pixel units. Use em's.

#imagePlaceholder {clear: both; height: 300px; overflow: auto;}

You don't need the clear:both rule.

#imagePlaceholder img {display: block;

Missing closing bracket. You don't need the rule anyway as only one
image will be displayed at a time.

</style>
</head>
<body>
<div class="menu">
<a href="alternativelink.html" onclick="return !showMe(1, this);">1</
a>
<a href="alternativelink.html" onclick="return !showMe(2, this);">2</
a>
<a href="alternativelink.html" onclick="return !showMe(3, this);">3</
a>
<a href="alternativelink.html" onclick="return !showMe(4, this);">4</
a>
<a href="alternativelink.html" onclick="return !showMe(5, this);">5</
a>
</div>
<div id="imagePlaceholder" style="background-color: red;">

No color? What if my default text color is red?

<div id="option1"><img src="../../i/004.jpg" alt="image one">caption
for image one</div>
<div id="option2"><img src="../../i/021.jpg" alt="image two">caption
for image two</div>
<div id="option3"><img src="../../i/022.jpg" alt="image
three">caption for image three</div>
<div id="option4"><img src="../../i/023.jpg" alt="image
four">caption for image four</div>
<div id="option5"><img src="../../i/025.jpg" alt="image
five">caption for image five</div>
</div>
<p><a href="http://validator.w3.org/check?uri=referer"><img
src="http://www.w3.org/Icons/valid-xhtml11" alt="Valid XHTML 1.1"
height="31" width="88" style="border: 0px;" /></a></p>

Wrong emblem.

</body>
</html>
 
D

David Mark

[snip]


Missing closing bracket. You don't need the rule anyway as

Disregard the second sentence. Currently it is needed when scripting
is disabled. Preferably, I would put the captions in their own DIV's,
which will make it look better when CSS is disabled.

Also, the numbered links' href's should point to anchors for each
image. Furthermore, I would return true from the onclick handlers to
cover the situation where scripting is enabled, but CSS is not.

The main thing to remember with something like this is to test without
script, without style and without either. It has to make sense in all
three cases.

One other note. In the current example, the alternative text for the
images runs into the captions when images are disabled. Regardless, I
think the only logical alternative text for these images is "".
 
D

David Mark

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>
 
W

windandwaves

You really shouldn't be using the display property for this (see the
other thread that Randy mentioned.)

I dont understand this - why? visibility hidden means that it reserves
space for it, meaning that it does not sit as nice. What is wrong with
display: none?
.....
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.

What I do is I return true and in the onclick I put
return !myFunction() {return true;} this means that if the function
does not get to the end for some reason it returns false, meaning that
the alternative link will be followed - right?
.....
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?

Just a lazy way to put it on a nice place on the page. Nothing
special - that is not really important for this page.
........

<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.


This is where i got into trouble. I have two options:

1. wait until the page is loaded (body>onload) and then hide the
relevant elements if the javascript functions are available - I dont
like this, because it will make the page load in a fractured way
(especially when you have a big page or a slow connection).

2. hide the elements immediately (run JS with css immediately) and
risk that JS functions will not be available.

How can I check for the JS functions before the page is loaded?

Also, it think it would be safe to assume that elements exist -
right? you dont really need to test things like
if(document.getElementById) {
var el = document.getElementById("myDiv");
if(el) {
etc....
}
}
that seems a bit over the top.

Thanks again for all your help (new version loaded):

http://www.sunnysideup.co.nz/j/imageChange/

Nicolaas
 
D

David Mark

I dont understand this - why? visibility hidden means that it reserves
space for it, meaning that it does not sit as nice. What is wrong with
display: none?

Some UA's can't update the display style with script, in which case,
your images will never show up.
....


What I do is I return true and in the onclick I put
return !myFunction() {return true;} this means that if the function
does not get to the end for some reason it returns false, meaning that
the alternative link will be followed - right?

But even if it gets to the end, there is no guarantee that it actually
did anything. Try the app with CSS disabled (View | Page Style | No
Style in FF) and you will see that all of the images are there from
the start and clicking the numbered links does nothing unless the
links are followed. In other words, the script is useless in that
context, but doesn't know that it is useless, so you want to allow the
links to scroll the associated images into view.
....


Just a lazy way to put it on a nice place on the page. Nothing
special - that is not really important for this page.

I don't understand the concept of "a nice place on the page" from
reading those rules. They really don't make sense to me. What does
giving the body automatic margins and absolute positioning
accomplish? That seems contradictory to me. Using percentages for
top and left confuses the issue even more. But you are right as this
is not relevant to the script.
.......





This is where i got into trouble. I have two options:

1. wait until the page is loaded (body>onload) and then hide the
relevant elements if the javascript functions are available - I dont
like this, because it will make the page load in a fractured way
(especially when you have a big page or a slow connection).

2. hide the elements immediately (run JS with css immediately) and
risk that JS functions will not be available.

Or:

3. Write the style block to hide the images in the head of the
document after checking for the features required to show them (eg
getElementById.) This isn't foolproof, especially if the script has
to modify display styles.

4. Hide the container with a visibility:hidden rule so that the images
won't show during page load. When the page loads, set the inner DIVs'
display styles to "none" and then show the container. This is what I
would do.
How can I check for the JS functions before the page is loaded?

I don't understand the question. You mean check the features like
document.getElementById and document.getElementsByTagName? You can
check those at any time. However, you can't reliably inspect (most)
DOM elements during page load, which you tried to do in
checkFunctions. The two tasks should not be mixed up in the same
function.
Also, it think it would be safe to assume that elements exist -
right? you dont really need to test things like
if(document.getElementById) {
var el = document.getElementById("myDiv");
if(el) {
etc....
}}

You certainly do need to test the result returned from
getElementById. You can't rely on it 100% even if the element does
exist in the particular document. And the element is not 100%
guaranteed to exist in any document is it? Imagine if you (or
somebody else) gave it a new ID and didn't update the script.
that seems a bit over the top.

Not at all. You have to think like that. A JavaScript error is the
last thing you want to happen as there is no recovery from it and the
user will be left with a document in an unexpected state.

Anyway, you still had several problems. Here is a more reliable and
unobtrusive version of the. I made a few style changes too.

<!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 http-equiv="Content-type" content="text/html; charset=utf-8">
<meta name="robots" content="all">
<script type="text/javascript">
var oldMenuItem;
var oldEl;
function changeImage(optionNumber, menuItem) {
var el = document.getElementById("option" + optionNumber);
if (el && el.style) {
if(oldEl && el !== oldEl) {
oldMenuItem.className = "";
oldEl.style.display = "none";
}
el.style.display = "block";
menuItem.className = "highlight";
oldEl = el;
oldMenuItem = menuItem;
}
}

if (document.getElementById && document.getElementsByTagName) {
document.write('<style type="text/css"> #imagePlaceholder
{visibility:hidden} <\/style>');
window.onload = function() {
var el = document.getElementById('menu');
if (el && el.style) {
var links = el.getElementsByTagName('a');
if (links && links.length) {
for (var i = links.length - 1; i >= 0; i--) {
(function(i) {
links.onclick = function() { changeImage(i + 1, this); };
})(i);
}
el = document.getElementById('imagePlaceholder');
if (el) {
var divs = el.getElementsByTagName('div');
if (divs && divs.length) {
for (i = divs.length - 1; i >= 0; i--) {
divs.style.display = 'none';
}
}
el.style.visibility = 'visible';
}
}
}
};
}
</script>
<style type="text/css">
body {margin:3em 0 0 3em}
#menu {list-style-type:none;margin:0;padding:0}
#menu li {display:inline}
#menu a:link, #menu a:visited {padding: 1em 1em 1em 1em; background-
color: #ddd; color:black; text-decoration:none}
#menu a:hover, #menu a:focus, #menu a:active {color: white;
background-color: red}
#menu a.highlight {color: white; background-color: green; cursor:
default}
#imagePlaceholder {clear: both; height: 300px; overflow: auto;
margin-top:1em}
#imagePlaceholder img {display: block}
#imagePlaceholder p {margin: 0; padding: 0; font-style: italic;
margin-bottom: 30px}
</style>
</head>
<body>
<ul id="menu"><li><a href="#pic1">1</a></li><li><a href="#pic2">2</
a></li><li><a href="#pic3">3</a></li><li><a href="#pic4">4</a></
li><li><a href="#pic5">5</a></li></ul>
<div id="imagePlaceholder">
<div id="option1"><a name="pic1" id="pic1"><img src="../../i/
004.jpg" alt="image one"></a><p>caption for image one</p></div>
<div id="option2"><a name="pic2" id="pic2"><img src="../../i/
021.jpg" alt="image two"></a><p>caption for image two</p></div>
<div id="option3"><a name="pic3" id="pic3"><img src="../../i/
022.jpg" alt="image three"></a><p>caption for image three</p></div>
<div id="option4"><a name="pic4" id="pic4"><img src="../../i/
023.jpg" alt="image four"></a><p>caption for image four</p></div>
<div id="option5"><a name="pic5" id="pic5"><img src="../../i/
025.jpg" alt="image five"></a><p>caption for image five</p></div>
</div>
</body>
</html>
 
D

David Mark

Anyway, you still had several problems. Here is a more reliable and
unobtrusive version of the. I made a few style changes too.

One note on the revised version. Add a rule to the script-added style
block to hide the menu as well. It won't be able to do anything while
the page is loading and style is enabled.

And I just noticed that you still have alternate text for the images.
Change the alt attributes to "".
 
D

David Mark

David Mark said the following on 8/7/2007 12:53 AM:




I have to ask why you think the alt attribute should be changed to ""
instead of something reasonable for the image impaired UA's? If the

Any way you look at it, a slide show like this one isn't going to make
much sense in a text-only browser.
caption for the image is simple duplication for the ALT attribute then
it should be put in the ALT attribute, read back out by script and
turned into a caption. If the caption isn't

But then you need script to see the captions. The captions belong in
the static content and I don't see them as alternatives for the
images. If you put things like alt="A landscape with a sunset" in
image tags, then text-only browsers will display that, but it won't
make sense in a text-only context. It is easier to picture this if
you imagine an image floated next to a paragraph.

duplication, then the ALT
attribute should be there and be something meaningful in the event
images aren't supported, are disabled or the image can't be downloaded

What is meaningful in this case? The captions describe what would
have been in the empty spaces. That's another reason why captions
should be part of the static content.

I only recently saw the light on this when I started testing pages
with Lynx. It exposed some misconceptions I had about alt
attributes. They are not for describing images, but to provide
alternative content when alternate content is needed to make sense of
the page (eg diagrams, images that contain text, images that are
links, etc.)
 
D

David Mark

David Mark said the following on 8/7/2007 8:37 AM:




David Mark said the following on 8/7/2007 12:53 AM:
[snip]
Anyway, you still had several problems. Here is a more reliable and
unobtrusive version of the. I made a few style changes too.
One note on the revised version. Add a rule to the script-added style
block to hide the menu as well. It won't be able to do anything while
the page is loading and style is enabled.
And I just noticed that you still have alternate text for the images.
Change the alt attributes to "".
I have to ask why you think the alt attribute should be changed to ""
instead of something reasonable for the image impaired UA's? If the
Any way you look at it, a slide show like this one isn't going to make
much sense in a text-only browser.

It doesn't make much sense in a non-script browser either. It simply
becomes a plain image gallery. But, I agree

It actually works pretty well if CSS is enabled due to the
overflow:auto rule on the container (and the fact that the container
is roughly the size of one image.)

with the concept of "this
particular page" but I was more interested in any supposition of
removing/nullifying the alt attribute.

You never remove it as it is required for accessibility compliance,
but often you nullify it (typically for decorative images.)
Then it should, as I wrote, be in the static page. The sentence saying
that is broken in half above/below your text. But, with a text only
browser the captions alone don't make a lot of sense either do they?

In an image gallery like this, that's all the sense you can make of it
without using longdesc attributes to provide a links to descriptions
of the images. My point is that alt should not be used for such a
description.
The captions wouldn't make a lot of sense in a text only browser without
something to indicate what the caption is about though would they?

What makes sense of it is the context of the page. Presumably a
header above the index list would indicate that what follows is a
series of images (eg "Pictures from my summer vacation.")
See above though :) The captions without anything to go with them don't
make sense either.

In the context of this page I think they would make as much sense as
you could hope for in a text-only browser (or aural browser.)
Yes, but I usually test non-images in FF2.0 and Amaya because I am
spoiled by GUI's :)

I still don't know what more you could put in the alt attributes to
make it look right. Instead of having a single caption per image, you
would have alternating descriptions and captions. Assuming the
captions are not redundant (eg they don't indicate what the image is),
you would need use longdesc attributes to link to descriptions.

All in all, this type of presentation is an odd case. But there are
lots of instances where alt="" is the only sensible markup. It is
typically the best replacement for things like "[image]" or "[image of
my dog]", etc.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,159
Messages
2,570,879
Members
47,414
Latest member
GayleWedel

Latest Threads

Top