...
PDA

Code criticism wanted

Aceramic
10-29-2007, 04:39 PM
As the title says, just looking for your thoughts on my code. I'm relatively new to CSS, so for all I know, I may be going about this entirely the wrong way. :)


HTML:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
<title>Cartennes Nations - Home</title>
<link href="styles.css" rel="stylesheet" type="text/css" />
</head>
<body>
<!-- Begin banner -->
<div id="banner"><img src="images/Banner1.png" alt="Banner Image" width="750" height="180" border="0" /></div>
<!-- End banner -->
<!-- Begin Site Content -->
<div class="background">

<!-- Begin Content Wrapper -->
<div id="content">-content snipped-
<br />
<br />
-for client confidentiality-.
</div>
<!-- End Content Wrapper -->
</div>
<!-- End Site Content -->
<!-- Begin Navigation Wrapper -->
<div id="nav">

<!-- Index Button -->
<div class="index"><a href="index.html">&nbsp;</a></div>
<!-- End Index Button -->
<hr />
<!-- Rules Button -->
<div class="rules"><a href="rules.html">&nbsp;</a></div>
<!-- End Rules Button -->
<hr />
<!-- Sponsors Button -->
<div class="sponsors"><a href="sponsors.html">&nbsp;</a></div>
<!-- End Sponsors Button -->
<hr />
<!-- Timeline Button -->
<div class="timeline"><a href="timeline.html">&nbsp;</a></div>
<!-- End Timeline Button -->
<hr />
<!-- Events Button -->
<div class="events"><a href="events.html">&nbsp;</a></div>
<!-- End Events Button -->
<hr />


</div>
<!-- End Navigation Wrapper -->
</body>
</html>


CSS:

/* CSS Document */
<!--
/* Begin Content Classes */
/* Needed for proper positioning */
* {
margin:0;
padding:0;
}
/* Page Background Properties */
body {
background-color:#000000;
background-image:url(images/starfield1024_2.jpg);
background-repeat:no-repeat;
}
/* Content Wrapper Properties */
.background {
background-color:#000000;
border-color:#CC6600;
border-style:solid;
border-width:2px;
width:750px;
height:auto;
position:absolute;
top:200px;
left:215px;
opacity:0.7;
}
/* Banner Properties */
#banner {
background:none;
width:750px;
height:180px;
position:absolute;
left:215px;
}
/* Text Properties */
#content {
font-family:"Courier New", Courier, monospace;
font-size:18px;
color:#990000;
}
/* End Content Classes */
/* Begin Navigation Classes */
/* Navigation Wrapper Properties */
#nav {
background-color:#000000;
width:150px;
height:600px;
position:absolute;
top:200px;
left:0px;
border:2px solid #CC6600;
opacity:0.7;
}
/* Index Button */
#nav .index {
background-image:url(images/HomeButton.png);
width:148px;
height:20px;
}
/* Index Rollover */
#nav .index:hover {
background-image:url(images/HomeButtonRO.png);
width:148px;
height:20px;
}
/* Rules Button */
#nav .rules {
background-image:url(images/RulesButton.png);
width:148px;
height:20px;
}
/* Rules Rollover */
#nav .rules:hover {
background-image:url(images/RulesButtonRO.png);
width:148px;
height:20px;
}
/* Sponsors Button */
#nav .sponsors {
background-image:url(images/SponsorsButton.png);
width:148px;
height:20px;
}
/* Sponsors Rollover */
#nav .sponsors:hover {
background-image:url(images/SponsorsButtonRO.png);
width:148px;
height:20px;
}
/* Timeline Button */
#nav .timeline {
background-image:url(images/TimelineButton.png);
width:148px;
height:20px;
}
/* Timeline Rollover */
#nav .timeline:hover {
background-image:url(images/TimelineButtonRO.png);
width:148px;
height:20px;
}
/* Events Button */
#nav .events {
background-image:url(images/EventsButton.png);
width:148px;
height:20px;
}
/* Events Rollover */
#nav .events:hover {
background-image:url(images/EventsButtonRO.png);
width:148px;
height:20px;
}
/* Link Properties */
a {
font-family: Courier New, Courier, monospace;
font-size: 12px;
color: #CC6600;
}
a:link {
text-decoration: none;
}
a:visited {
text-decoration: none;
color: #990000;
}
a:hover {
text-decoration: underline;
color: #990000;
}
a:active {
text-decoration: none;
color: #CC6600;
}
hr {
background-color:#000000;
}
/* End Navigation Classes */
-->

As usual, posting the code screwed up some of the formatting. :/

You'll also notice that I commented the code to pieces. :)
srule_
10-29-2007, 05:57 PM
Just looking over your code quickly and have a few suggestions:

1. I see some <br /> in your html. You should not be using these, Use padding and margin in your css to archive this spacing.

2. Comments are good, but I think you use to many! this makes it hard to read and you increase your chance of mistakes, for exampl u have ".-for client confidentiality-." in one area because u did not press the SHIFT KEY. Use comments to seperate the page into sections so it is easy to find what you are looking for.
Aceramic
10-29-2007, 06:05 PM
Thanks for the comments, but I'm not quite sure I know what you mean for #1. For #2, that was actually entirely intentional, I cut all the text out, for confidentiality. ;)
srule_
10-29-2007, 07:32 PM
For #1: You used <br /> tags in your HTML to achive line brakes. You should be using padding/margins in css to achive this, not the <br> tag.

EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum