Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 4 of 4
  1. #1
    Regular Coder
    Join Date
    Oct 2007
    Posts
    148
    Thanks
    4
    Thanked 4 Times in 4 Posts

    Code criticism wanted

    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:

    Code:
    <!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:

    Code:
    /* 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. :)

  • #2
    Regular Coder
    Join Date
    Jul 2007
    Posts
    571
    Thanks
    25
    Thanked 28 Times in 28 Posts
    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.

  • #3
    Regular Coder
    Join Date
    Oct 2007
    Posts
    148
    Thanks
    4
    Thanked 4 Times in 4 Posts
    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.

  • #4
    Regular Coder
    Join Date
    Jul 2007
    Posts
    571
    Thanks
    25
    Thanked 28 Times in 28 Posts
    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.


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •