PDA

View Full Version : critique my code?


walkamongus
06-10-2009, 03:24 PM
This is my first attempt at using relatively positioned divs. My idea is to use this as a template for most, perhaps all, subsequent pages. I would love critical advice from you guys and gals. The "sidebar" div seems to be doing something fishy, as it looks ok in the browser but doesnt look right in design view. Also, Im not sure if Im using the stylesheet correctly, so any advice is welcome...

CSS titled "styles" in folder "css" or css/styles.css

@charset "UTF-8";
.header {
margin: auto;
height: 107px;
width: 950px;
top: 50px;
}
.content {
margin: auto;
height: 1200px;
width: 950px;
border:2px solid #666666;
}
.photos {
height: 107px;
width: 950px;
top: 150px;
}
.text {
height: 950px;
width: 700px;
top: 3px;
position: relative;
}
.sidebar {
height: 950px;
width: 250px;
left: 698px;
position: relative;
top: -970px;
border: 2px solid #666666;
}
.navbar {
height: 28px;
width: 950px;
position: relative;
top: 0px;
background-color: #994285;
font-family: Verdana, Arial, Helvetica, sans-serif;
text-align: center;
color: #FFFFFF;
word-spacing: normal;
padding-top: 3px;
border-bottom-width: 2px;
border-bottom-style: solid;
border-bottom-color: #666666;
}
.headline {
font-family: "Times New Roman", Times, serif;
font-size: 30px;
margin-top: 18px;
margin-bottom: 18px;
}
.donate {
height: 85px;
width: 112px;
position: relative;
right: -70px;
top: 15px;
}


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=UTF-8" />
<title>Untitled Document</title>
<link href="css/styles.css" rel="stylesheet" type="text/css" />
<style type="text/css">

</style>
</head>

<body>
<div class="content">

<div class="header"><img src="images/header.jpg" width="950" height="107" /></div>
<div class="photos"><img src="images/photos.jpg" width="950" height="107" /></div>
<div class="navbar">Threats &amp; Victories &nbsp; | &nbsp; Take Action &nbsp; | &nbsp; Bookstore &nbsp; | &nbsp; Press &nbsp; | &nbsp; About Us &nbsp; | &nbsp; Newsletter &nbsp; | &nbsp; Blog</div>
<div class="text">
<div align="center" class="headline">**** ****** ******</div>
<p>Are you now, or have you ever been an environmentalist? An animal rights activist or peace activist? Have you ever fought for economic justice? Demonstrated against abortion? Do you belong to a mosque? Are you a veteran of the war in Iraq or Afghanistan? Is there a Ron Paul bumper sticker on your car?</p>
<p>If so, you may have been the target of unconstitutional police spying.</p>
<p>In 2005, the FBI testified to Congress that animal rights and environmental 'extremists' are the number one domestic terrorism threat in the U.S. Recently-leaked documents indicate that FBI and other police informants have unjustifiably infiltrated mosques as well as anarchist, peace, anti-death penalty and other activist groups. Documents indicate that police agencies at the federal, state, local and tribal level are tracking activists of all stripes based on their legitimate First Amendment-protected activities.</p>
<p>Find out how we're fighting back,</p>
<div align="center" class="headline">******* ***** *******</div>
<p>The right to dissent is the foundation of a strong democracy. Unfortunately the history of dissent in America goes hand in hand with the history of government repression of dissent:

The inspiring story of Martin Luther King Jr. is paired with the treacherous FBI campaign of spying, sabotage and smear tactics to 'neutralize' him. Beloved folk singer Pete Seeger was 'investigated for sedition, harassed by the FBI and blacklisted," and sentenced to a year in jail for refusing to answer questions posed by the House Committee on Un-American Activities (HUAC). Union activists were often met with violence, members of the International Workers of the World gunned down by police in Everett Washington in 1916, or the Minneapolis Teamsters killed by police in 1934 ... read more</p>
<div align="center" class="headline">******'* ******** *** *** ************ </div>
**************** has endorsed the Bill of Rights Defense Committee's grassroots campagin to take back our constitional rights. We encourage you to join also, and help build a coalition in your community or congressional district to hold your legislators accountable for fulfilling their oaths to protect and defend the Constitition.

Find out more about the campaign at</div>
<div class="sidebar">
<div class="donate"><img src="images/donate.jpg" width="113" height="85" /></div>
</div>
</div>
</body>
</html>


Thanks

coloeagle
06-10-2009, 05:08 PM
Not too bad although there are a few minor things that could cause you problems across different browsers.

I've done some rewriting of the css for you. Try this out. Hopefully it will help you along and advance your learning.

@charset "utf-8";
.content {
overflow:hidden;
width:950px;
margin:0 auto;
text-align:center;
border:2px solid #000;
}
.header {
text-align:left;
height: 107px;
width: 950px;
}
.photos {
text-align:left;
height:107px;
width:950px;
}
.text {
float:left;
text-align:left;
padding:10px;
width:660px;
}
.sidebar {
float:right;
text-align:left;
height:950px;
width:250px;
padding:5px;
border-left:2px solid #666;
}
.navbar {
height:28px;
width:950px;
background-color: #994285;
font-family:Verdana, Arial, Helvetica, sans-serif;
text-align:center;
color:#FFF;
padding-top:3px;
border-bottom: 2px solid #666;
}
.headline {
font-family: "Times New Roman", Times, serif;
text-align:center;
font-size:30px;
margin:18px 0;
}
.donate {
width:112px;
}
On a further note for your navigation you should look at using an unordered list for the links. Style this so that you won't need to use extra spacing in the html.

With your headline div, unless you really need to have your headline in a separate div you could/should use h tags and then style them with css. By doing this you can remove the headline div from both the css and the html.

walkamongus
06-10-2009, 09:40 PM
Wow thanks. That cleans things up a lot! The float option is new to me. I have a few questions...

-Is there by any chance a "float:center;" or something similar?
-In ".content", why did you assign "text-align:center;", as there is no text?

Will look into unordered lists. The reason I have the headline in a separate div is because I thought it would be easier to update. The person Im doing the site for has less DW experience than me, so I wanted to make it very clear, but perhaps Im wrong.

Thanks again

MagicPower
06-10-2009, 09:53 PM
can i also point out that when previewing your design in the browser, you should use multiple previews in different browser coz what it looks like in firefox won't be exactly the same as in IE or safari or any other browser. DW design view is very confusing. to correct the design view sometimes, i have to first save and then close the page i'm editing and the reopen it. then everything goes back to normal - doesn't always work. float:left can float your divs left of each other or under depending on if they have fixed widths.

walkamongus
06-10-2009, 10:01 PM
Thanks MagicPower. Another Q... how can an unordered list eliminate the need for "&nbsp;". I wrapped some text in "ul" tags on a practice page but I dont see how to add more than one space.

MagicPower
06-10-2009, 10:12 PM
if adding more than one space is what you're after you should: hold ctr + shift at the same time and then hit space for Windows OS. option + space for Mac.

walkamongus
06-10-2009, 10:39 PM
I just pressed option + space (im on a powerbook) and the cursor did not move. tried in design and code view.

coloeagle
06-10-2009, 10:40 PM
The text-align:center for your .content is for IE6. IE6 doesn't recognize margin:auto;

Try this out for your navigation. You just need to change the paddings to give each more space.
The css;

.navbar {
height:28px;
width:950px;
background-color:#994285;
font-family:Verdana, Arial, Helvetica, sans-serif;
text-align:center;
border-bottom: 2px solid #666;
}
.navbar ul {padding:5px 0;}
.navbar ul li {display:inline; padding:0px 8px 0px 5px; border-right:1px solid #fff;}
.navbar li.end {border:none;}
.navbar a:link {text-decoration:none; color:#fff;}
.navbar a:visited {text-decoration:none; color:#006;}
.navbar a:hover, a:focus {text-decoration:none; color:#000;}
.navbar a:active {text-decoration:none; color:#006;}


The html;

<div class="navbar">
<ul>
<li><a href="#">Threats</a></li>
<li><a href="#">Victories</a></li>
<li><a href="#">Take Action</a></li>
<li><a href="#">Bookstore</a></li>
<li><a href="#">Press</a></li>
<li><a href="#">About Us</a></li>
<li><a href="#">Newsletter</a></li>
<li class="end"><a href="#">Blog</a></li>
</ul>
</div>

coloeagle
06-10-2009, 10:54 PM
-Is there by any chance a "float:center;" or something similar?

Nope, just float left or right. Consider center the default. If you just add div's without a float they will just stack on top of each other.


The reason I have the headline in a separate div is because I thought it would be easier to update. The person Im doing the site for has less DW experience than me, so I wanted to make it very clear, but perhaps Im wrong.


Nope not wrong, what ever works to make things easy. :-D

walkamongus
06-11-2009, 01:09 AM
Thanks for taking the time to help me out Coloeagle! Its looking great and Im learning a lot.:-D