Critique this div resizing script

J

johkar

The below script sets and maintains the equal heights of 2 (or 3) divs
which are floated next to each other on a page. I want to ensure that
I have not overlooked anything and it will not error. In other words,
it either executes successfully or not at all. This script is a
compilation of code found on various posts along with my
personalization thrown in to meet our specs. Note that this is just a
"nice to have" script so if it doesn't execute the page is still
usable.

John

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
"http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=iso-8859-1">
<style type="text/css">
div {
border:1px solid #003366;
padding:7px;
width:40%;
float:left;
margin-left:15px;
}
</style>
<script type="text/javascript">
function setDivHeights(id1,id2,id3){
if(document.getElementById){
var div1 =
(document.getElementById(id1))?document.getElementById(id1):false;
var div2 =
(document.getElementById(id2))?document.getElementById(id2):false;
var div3 =
(document.getElementById(id3))?document.getElementById(id3):false;
if(div1 && div2){
var max = div1.offsetHeight;
if(div2.offsetHeight > max)
max = div2.offsetHeight;
if(div3 && (div3.offsetHeight > max))
max = div3.offsetHeight;

document.getElementById(id1).style.height = max + "px";
document.getElementById(id2).style.height = max + "px";
if(div3)
document.getElementById(id3).style.height = max + "px";
}
if(window.onresize)
window.onresize=(div3)?function(){setDivHeights(div1,div2,div3)}:function(){setDivHeights(div1,div2)};
}
}
</script>
</head>
<body onload="setDivHeights('div1','div2')">
<div id="div1">
<p>Some Text Some Text Some Text Some Text Some Text Some Text Some
Text Some Text Some</p>
</div>
<div id="div2">
<p>Some Text Some Text Some Text Some Text Some Text Some Text Some
Text Some Text Some Text Some Text Some Text Some Text Some Text Some
Text Some Text</p>
</div>
</body>
</html>
 
R

Randy Webb

johkar said the following on 11/15/2006 8:57 PM:
The below script sets and maintains the equal heights of 2 (or 3) divs
which are floated next to each other on a page. I want to ensure that
I have not overlooked anything and it will not error. In other words,
it either executes successfully or not at all. This script is a
compilation of code found on various posts along with my
personalization thrown in to meet our specs. Note that this is just a
"nice to have" script so if it doesn't execute the page is still
usable.

document.getElementById(id1).style.height = max + "px";

You are assuming, falsely so, that the browser will support .style and
..height just because it supports getElementById. Feature test before
using anything.

That is not the only occurrence of it, but the first one I noticed.
 
R

RobG

johkar said:
The below script sets and maintains the equal heights of 2 (or 3) divs
which are floated next to each other on a page. I want to ensure that
I have not overlooked anything and it will not error. In other words,
it either executes successfully or not at all. This script is a
compilation of code found on various posts along with my
personalization thrown in to meet our specs. Note that this is just a
"nice to have" script so if it doesn't execute the page is still
usable.

John

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
"http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=iso-8859-1">
<style type="text/css">
div {
border:1px solid #003366;
padding:7px;
width:40%;
float:left;
margin-left:15px;
}
</style>
<script type="text/javascript">
function setDivHeights(id1,id2,id3){
if(document.getElementById){
var div1 =
(document.getElementById(id1))?document.getElementById(id1):false;

That seems a superfluous test and means you lookup the element twice.
Use:

var div1 = document.getElementById(id1);

If getElementById doesn't find the element, div1 will be set to 'null',
which will handily convert to false when you test it later. The same
goes for div2 & 3.
var div2 =
(document.getElementById(id2))?document.getElementById(id2):false;
var div3 =
(document.getElementById(id3))?document.getElementById(id3):false;
if(div1 && div2){
var max = div1.offsetHeight;
if(div2.offsetHeight > max)
max = div2.offsetHeight;
if(div3 && (div3.offsetHeight > max))
max = div3.offsetHeight;

document.getElementById(id1).style.height = max + "px";

Why look the element up again, you stored a reference in div1, so why
not:

if (div1 && div1.style && typeof div1.offsetHeight == 'number'){
div1.style...

Can't you do this with pure CSS? Try:

news:comp.infosystems.www.authoring.stylesheets
<URL:
http://groups.google.com.au/group/comp.infosystems.www.authoring.stylesheets
 
J

johkar

Randy said:
<snip>

You are assuming, falsely so, that the browser will support .style and
.height just because it supports getElementById. Feature test before
using anything.

That is not the only occurrence of it, but the first one I noticed.

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

Thanks, I will test for those features.
 
J

johkar

RobG said:
That seems a superfluous test and means you lookup the element twice.
Use:

var div1 = document.getElementById(id1);

If getElementById doesn't find the element, div1 will be set to 'null',
which will handily convert to false when you test it later. The same
goes for div2 & 3.


Why look the element up again, you stored a reference in div1, so why
not:

if (div1 && div1.style && typeof div1.offsetHeight == 'number'){
div1.style...

Can't you do this with pure CSS? Try:

news:comp.infosystems.www.authoring.stylesheets
<URL:
http://groups.google.com.au/group/comp.infosystems.www.authoring.stylesheets

Points taken Rob. I am not sure why I tested twice for the existence
of the divs. Probably innattention as I was trying different things.
As far as using pure CSS, that is fine if the content within the divs
will always be the same, but we need this script for dynamic content.
Of course, I should have mentioned that.

Thanks for your insight, it is appreciated.
John
 

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

Forum statistics

Threads
473,968
Messages
2,570,153
Members
46,699
Latest member
AnneRosen

Latest Threads

Top