Please critique my HTML / CSS

by dreambody

Posted: 12 years ago 12 replies

WEB DESIGN

Hi, all

I'm relatively new to HTML and CSS and I've done a website for a friend. As it's only the second one I've ever done (the first was not good), I would appreciate it if someone could take a look at the code and tell me if / how I could have done things better. For example, a class instead of an id, a div instead of a whatever, to help me next time. I am not looking for website feedback however. I've done as I was asked. Just the code.

Thanks.

The site is Door2DoorDrinks | Drinks delivered to your door!

@charset "utf-8";
/* ^1 --------------------------- global constants -------------------------*/
html, body {
margin: 0px;
padding: 0px;
}

body {
text-align:center;
font: 100% Georgia, "Times New Roman", Times, serif;
margin-top: 25px;
background-image: url(_images/wrapper_background_gradient_long.jpg);
background-repeat: repeat-x;
}
/* ^2 --------------- limited-scale reset ---------------- */
h1, h2, h3, h4, h5, h6, p, address, blockquote, div, ul, li {
margin: 0;
padding: 0;
text-align: left;
color: #000166;
}
p {
color:#000
line-height:1.8;
margin-bottom: 1em;
}
h1 {
font-size: 2em;
color: #000166;
margin-bottom: .4em;
}
h2 {
font-size: 1.6em;
color: #000166;
font-weight: normal;
margin-top: 1.2em;
margin-bottom: 1em;
}
h3 {
font-size: 1.3em;
color: #51341a;
font-weight: normal;
margin: 1.25em 0 .5em;
}
h1, h2, h3 {
clear: both;
text-align: center;
}
a:link, a:visited {
color: #000166;
text-decoration:none;
}
a:hover, a:active {
text-decoration:underline;
}
a.accent {
display: block;
text-align: right;
}
a.accent:hover {
border: none;
}
a img {
border: none;
}
/* ^3 ------ global classes -------- */
.floatRight {
float: right;
}
.floatLeft {
float: left;
}
.clearRight {
clear: right;
}
.clearLeft {
clear: left;
}
.clearBoth {
clear: both;
}
#wrapper {
position: relative;
padding: 0;
width: 1000px;
margin: 0 auto;
background: #fff;
text-align: left;
}
/* ^4 ------------- home page specfic layout styles ----------- */
/* header */
#header {
width: 1000px;
height: 170px;
background-image: url(_images/banner_test_latest.jpg);
}
#mainContent {
width:710px;
float:left;
padding:20px;
}
/* mainNav */
#mainNav {
height:30px;
width:1000px;
background-image: url(_images/nav_background_gradient_wide2.jpg);
background-repeat: repeat-x;
}
#wrapper #mainNav ul li {
font-weight:bold;
padding-top: 6px;
padding-right: 16px;
padding-left: 16px;
}
#nav {
padding: 0;
margin: 0 auto;
width: 700px;
}
#nav li {
float: left;
list-style: none;
}
#nav ul li {
float: none;
margin-top:4px;
}
#nav ul {
position: absolute;
left: -999em;
padding:4px 0;
background-repeat: repeat-x;
background-image: url(_images/nav_background_dropdown_long.jpg);
}
#nav li:hover ul {
left: auto;
}
/* sidebar */
#sidebar {
width: 250px;
float:right;
margin-bottom: 10px;
color: #000040;
padding-top: 35px;
}
/* sidebar content*/
#wrapper #sidebar #challenge {
clear: both;
width: 120px;
margin-top: 25px;
margin-bottom: 25px;
}
#wrapper #sidebar #followMe {
margin-bottom: 25px;
}
#wrapper #sidebar #followMeFacebook {
margin-top: 25px;
width: 120px;
}
#wrapper #sidebar #followMeTwitter {
margin-top: 25px;
width: 120px;
}
/* main content images */
#wrapper #mainContent #aboutUsPhoto {
height: 300px;
width: 300px;
margin:auto;
border: solid 1px;
}
#wrapper #mainContent #acceptedCards {
height: 34px;
width: 443px;
margin: auto;
margin-bottom: 25px;
}
#wrapper #mainContent #deliveryList ul{
list-style-type: disc;
list-style-position: inside;
padding-left: 150px;
}
#wrapper #mainContent #deliveryList p {
padding-left: 150px;
}
#wrapper #mainContent #logoText {
padding:10px 0 15px 0px;
}
#wrapper #mainContent #mainPhoto {
height: 302px;
width: 400px;
margin:auto;
margin-bottom:20px;
border: solid 1px;
}
#wrapper #mainContent #cardText {
margin:auto;
}
#wrapper #mainContent #facebookLink {
height: 100px;
width: 266px;
margin-bottom:20px;
padding-left:45px;
float:left;
}
#wrapper #mainContent #twitterLink {
height: 100px;
width: 167px;
margin-bottom:20px;
padding-right:45px;
float:right;
}
#wrapper #mainContent #productsRow {
height:150px;
padding-bottom: 10px;
padding-top: 20px;
clear:both;
}
/* products gallery */
#wrapper #mainContent #productsRow #leftImage {
padding-left:115px;
}
#wrapper #mainContent #productsRow #rightImage {
padding-right:115px;
}
#wrapper #mainContent #textRow #leftText1 {
float:left;
padding-left: 165px;
font-weight: bold;
text-align: left;
}
#wrapper #mainContent #textRow #rightText1 {
padding-right: 165px;
font-weight: bold;
text-align: right;
float:right;
}
#wrapper #mainContent #textRow #leftText2 {
float:left;
padding-left: 165px;
font-weight: bold;
text-align: left;
}
#wrapper #mainContent #textRow #rightText2 {
padding-right: 165px;
font-weight: bold;
text-align: right;
float:right;
}
#wrapper #mainContent #textRow #leftText3 {
float:left;
padding-left: 145px;
font-weight: bold;
text-align: left;
}
#wrapper #mainContent #textRow #rightText3 {
padding-right: 160px;
font-weight: bold;
text-align: right;
float:right;
}
#wrapper #mainContent #textRow #leftText4 {
float:left;
padding-left: 155px;
font-weight: bold;
text-align: left;
}
#wrapper #mainContent #textRow #rightText4 {
padding-right: 160px;
font-weight: bold;
text-align: right;
float:right;
}
/* beers ciders wines etc list */
#wrapper #mainContent #productsList {
height:100px;
padding-bottom: 10px;
}
#wrapper #mainContent #productsList #leftImage {
padding-left:115px;
}
#wrapper #mainContent #productsList #rightImage {
padding-right:115px;
}
#wrapper #mainContent #productListSmall {
height:50px;
padding-bottom: 10px;
}
#wrapper #mainContent #productsListSmall #leftImage {
padding-left:115px;
}
#wrapper #mainContent #productsListSmall #rightImage {
padding-right:115px;
}
#wrapper #mainContent li {
margin-top: 1em;
}
#wrapper #mainContent #productsRow {
height:150px;
padding-bottom: 10px;
padding-top: 20px;
clear:both;
}
#wrapper #mainContent #productsRow #leftImage {
padding-left:115px;
}
#wrapper #mainContent #productsRow #rightImage {
padding-right:115px;
}
#wrapper #mainContent #textRow #leftText1 {
float:left;
padding-left: 165px;
font-weight: bold;
text-align: left;
}
#wrapper #mainContent #textRow #rightText1 {
padding-right: 165px;
font-weight: bold;
text-align: right;
float:right;
}
#wrapper #mainContent #textRow #leftText2 {
float:left;
padding-left: 165px;
font-weight: bold;
text-align: left;
}
#wrapper #mainContent #textRow #rightText2 {
padding-right: 165px;
font-weight: bold;
text-align: right;
float:right;
}
/* footer */
#footer {
clear:both;
width: 1000px;
height: 100px;
text-align: center;
}
#wrapper #footer h5 {
text-align: center;
padding-top: 50px;
}

And this is the page where I had the most trouble with the layout:

<!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>Door2DoorDrinks | Drinks delivered to your door!</title>
<meta name="keywords" content="door2doordrinks, alcohol, booze, where can I buy drinks" />
<meta name="description" content="Get alcohol, snacks & tobacco delivered to you door between 8pm and 3am just by making a quick phone call to door2doordrinks. Check our site for product list and more information." />
<link href="main.css" rel="stylesheet" type="text/css" media="screen" />
</head>
<body>
<div id="wrapper">
<div id="header">
</div>
<div id="mainNav">
<ul id="nav">
<li><a href="index.html">Home</a></li>
<li><a href="products.html">Products</a>
<ul>
<li><a href="beers.html">Beers</a></li>
<li><a href="ciders.html">Ciders</a></li>
<li><a href="wines.html">Wines</a></li>
<li><a href="spirits.html">Spirits</a></li>
<li><a href="champagne.html">Champagne</a></li>
<li><a href="mixers.html">Mixers</a></li>
<li><a href="tobacco.html">Tobacco</a></li>
<li><a href="snacks.html">Snacks</a></li>
</ul>
</li>
<li><a href="delivery.html">Delivery</a></li>
<li><a href="terms.html">Terms</a></li>
<li><a href="contact_us.html">Contact</a></li>
<li><a href="about_us.html">About Us</a></li>
</ul>
</div>
<div id="mainContent">
<div id="logoText">
<h1>Drinks delivered to your door!</h1></div>
<h1>Coming Soon!</h1>
<div id="mainPhoto"><img name="phoneNumber" src="_images/girls_partying.jpg" width="400" height="302" alt="" /></div>
<div id="cardText">
<h2><strong>We accept</strong>:</h2></div>
<div id="acceptedCards"><img src="_images/accepted_payments_image.png" alt="Cards accepted by door2doordrinks" name="acceptedCards" width="443" height="34" id="acceptedCards" /></div>
<div id="facebookLink"><a href="https://www.facebook.com/#!/pages/Door2Door-Drinks/156237937825084" target="_blank"><img src="_images/facebook_image.jpg" alt="Facebook Link" width="266" height="100" /></a></div>
<div id="twitterLink"><img src="_images/twitter-bird.jpg" alt="Twitter Link" width="167" height="100" /></div>
</div>
<div id="sidebar">
<div id="openingTimes"><table width="72%" border="0" cellpadding="2" cellspacing="0">
<tr>
<th colspan="2" scope="col"><h4>Opening Times</h4></th>
</tr>
<tr>
<td width="30%">Mon</td>
<td width="70%">Closed</td>
</tr>
<tr>
<td>Tue</td>
<td>8pm - 3.30am</td>
</tr>
<tr>
<td>Wed</td>
<td>8pm - 3.30am</td>
</tr>
<tr>
<td>Thu</td>
<td>8pm - 3.30am</td>
</tr>
<tr>
<td>Fri</td>
<td>8pm - 3.30am</td>
</tr>
<tr>
<td>Sat</td>
<td>8pm - 3.30am</td>
</tr>
<tr>
<td>Sun</td>
<td>8pm - 2am</td>
</tr>
</table></div>
<div id="challenge"><img src="_images/challenge25.jpg" alt="Challenge 25" width="120" height="60" /></div>
<div id="followMe"><h4>Follow Me</h4></div>
<div id="followMeFacebook"><a href="https://www.facebook.com/#!/pages/Door2Door-Drinks/156237937825084" target="_blank"><img src="_images/facebook_button.png" alt="Facebook Link" width="64" height="64" /></a></div>
<div id="followMeTwitter"><img src="_images/twitter_button.jpg" alt="Twitter Link" width="64" height="64" /></div>
</div>
<div id="footer">
<iframe src="//www.facebook.com/plugins/like.php?href=http%3A%2F%2Fwww.door2doordrinksyork .com%2F&send=false&layout=standard&wid th=450&show_faces=false&action=like&co lorscheme=light&font=arial&height=35" scrolling="no" frameborder="0" style="border:none; overflow:hidden; width:450px; height:35px; text-align: center; font-style: italic;" allowTransparency="true"></iframe>
<h5>copyright© door2doordrinksyork.com 2012</h5></div>
</div>
</body>
</html>

#critique #css #html #website

alan9187 12 years ago

Just a quick one, Not everything.

Place comments at the start and end of the div tags.. <-- start maincontent -->your dive opener. your dive closer <-- end maincontent -->

This will alow you go back and edit the content inbtween it especialy as the site grows.
- Thanks
{{ DiscussionBoard.errors[6091101].message }}
Warrior Machine Banned 12 years ago

[DELETED]

{{ DiscussionBoard.errors[6095811].message }}
- mmrumii 12 years ago
  
  Originally Posted by Warrior Machine
  
  Looks good enough for me. But your twitter follow up link is not active yet. Good and clean look works for me.
  
  You are absolutely right, It also enough for me. Thanks dude.
  
  Thanks
  
  Signature
  Blog about Content Locking and CPA
  
  {{ DiscussionBoard.errors[6096202].message }}
riinfotech45 12 years ago

very nice thanks for this..
- Thanks
{{ DiscussionBoard.errors[6110274].message }}
dagnyjbarber 12 years ago

really nice if you are new in this field.
- Thanks
- 1 reply
Signature
{{ DiscussionBoard.errors[6110284].message }}
- dreambody 12 years ago
  
  Thanks for the feedback!
  
  Thanks
  
  {{ DiscussionBoard.errors[6548560].message }}
purpleviolet 12 years ago

Wow .. its very impressive for a 2nd work on html... I would suggest you to use a background color instead of leaving it blank
- Thanks
{{ DiscussionBoard.errors[6549218].message }}
Istvan Horvath 12 years ago

Instead of posting miles long code here... you guys should learn to use Pastebin.com - #1 paste tool since 2002!
- Thanks
Signature
{{ DiscussionBoard.errors[6549664].message }}

jamaks

12 years ago

Hi dreambody, it looks good to me. Certainly more advanced than my second attempt was! The only thing I did notice was the unescaped ampersand in the meta name description.
<meta name="description" content="Get alcohol, snacks & tobacco
Should read <meta name="description" content="Get alcohol, snacks & tobacco

Enjoy your coding. Jim

Thanks
1 reply

{{ DiscussionBoard.errors[6552393].message }}

MadHu5tle

12 years ago

It's not bad, but there is a lot of unneccessary markup and styling. You could make it much simpler by using classes, and styling elements based on inheritance and position.

First, while it works to use ID's for everything, it makes things a lot more complicated than they need to be. Use some classes. For instances, you have several different ID's for leftImage, and rightImage as well as leftText and rightText that (in most cases, at least) are virtually the same. Instead of using rightText1, rightText2, etc to apply the same styling, just use a class of rightText and reuse it.

Second, a lot of elements are wrapped in div's, when they can be styled directly. For instance, the div mainPhoto...all it contains is a photo, why not just style the image directly and eliminate that entire div from your markup? Another one is the navigation lists. You don't really need a div ID, AND a list ID. You can either style ul#nav or div#mainNav ul. From there you can also reach the nested list using something like ul#nav ul li ul and ul#nav ul li ul li to style list items in the nested list, another id is not necessary.

In fact, if you give it an a doctype of html 5 instead of xhtml you can be more efficient and just use:

<nav id="main">
<ul>
<li>Blah Blah</li>
</ul>
</nav>

And then style it using nav#main, nav#main ul, nav#main ul li, etc on down.

Again, nothing too major, just a few things that could clean it up a little.

Thanks
1 reply

{{ DiscussionBoard.errors[6553880].message }}

dreambody

12 years ago

Originally Posted by MadHu5tle

It's not bad, but there is a lot of unneccessary markup and styling. You could make it much simpler by using classes, and styling elements based on inheritance and position.

Yes I thought this but too late on. By the time I realised how many id's I had I knew there had to be a much easier way using classes, then I realised how little I understood about classes!

Thanks

{{ DiscussionBoard.errors[6570812].message }}

Randize 12 years ago

MadHu5tle has said it right. Good code should be clean, only put what's necessary. But for the needs, I think you are doing it right.

You can also use firefox or chrome to inspect the css. And then make changes accordingly. Just a tip.
- Thanks
{{ DiscussionBoard.errors[6558190].message }}
rising_sun Banned 12 years ago

Not bad for the first site ,but keep going on,practice more.
After completing it ,design a full site layout.
Then make a PSD and convert it to HTML ,it will be a great practice for you,if you can.

Now my advice is be grammatical like set comments,make space in between <div>,just it.
- Thanks
{{ DiscussionBoard.errors[6570911].message }}

Please critique my HTML / CSS

Trending Topics

Are Solo ADs the Most Cost Effective Way to Promote your Product?

How do I effectively build and nurture an email list for affiliate marketing purposes?

Are billboards still effective in driving customers?

Been a while! Any of you Old Timers here?

How much does the rebranding affect CR ?