Personal website, done after one week of learning HTML and CSS

Question

I've just started learning CSS/HTML a week ago and I made a quick site today. It looks pretty good, but I think that I reused/wrote some really messy CSS. This is because I haven't used the float property in CSS too well, so I keep using position:relative and top to offset the float.

/* General Elements* /
body {
    background: #53777a;
    font-family: Garamond, Baskerville, "Baskerville Old Face", "Hoefler Text", "Times New Roman", serif;      
}

h1 {
    font-size: 28pt;       
}

h2 {}

p {}

a:link {
    color: black;
    text-decoration: none;
}

a:visited {
    color: purple;
    text-decoration: none;
}
a:hover {
    color: green;
    text-decoration: underline;
}
a:active {
    color:yellow;
    text-decoration: none;
}

/* Curvy Shapes */
 #wrapper, #footer {
    -moz-border-radius-bottomright: 50px;
        -moz-border-radius-topleft: 50px;
       -moz-border-radius-topright: 50px;
     -moz-border-radius-bottomleft: 50px;
        border-bottom-right-radius: 50px;
            border-top-left-radius: 50px;
           border-top-right-radius: 50px;
         border-bottom-left-radius: 50px;
}

#links {
    -moz-border-radius-bottomright: 50px;
     -moz-border-radius-bottomleft: 50px;
        border-bottom-right-radius: 50px;
         border-bottom-left-radius: 50px;
}

#header {    
     -moz-border-radius-topleft: 50px;
    -moz-border-radius-topright: 50px;
         border-top-left-radius: 50px;
        border-top-right-radius: 50px;    
}

/* Structure */
#wrapper {
    width: 900px;
    margin: 0 auto;
    margin-top: 30px;
    overflow: auto;
    background: #E0E4CC;
    padding: 20px;
    -moz-border-radius-bottomright: 50px;
        -moz-border-radius-topleft: 50px;
       -moz-border-radius-topright: 50px;
     -moz-border-radius-bottomleft: 50px;
        border-bottom-right-radius: 50px;
            border-top-left-radius: 50px;
           border-top-right-radius: 50px;
         border-bottom-left-radius: 50px;       
}

#header {
    text-align: center;
    background: #ECD078;
    padding: 4px;
}

#links {
    width: 900px;
    background-color: #A7DBD8;
    position: relative;
    top: -20px;
    text-align: center;
}

#links ul {
    list-style-type: none;
    padding: 5px;
}

#links li{
    display: inline;
    font-size: 14pt;
    padding: 20px;
}

.sidebar_left {
    float: left;
    width: 180px;
    margin-left: 10px;
    position: relative;
    top: -10px;
    text-align: justify;
    line-height: 150%;
}

#post {
    float: right;
    width: 680px;
    margin-left: 0px;
    margin-right: 10px;
    position: relative;
    top: -25px;
    text-align: justify;
    line-height: 150%;      
}

#post b {
    font-size: 18pt;
    text-decoration: underline;
}

#content {
    float: right;
    width: 680px;
    margin-left: 0px;
    margin-right: 10px;
    position: relative;
    top: -25px;
    text-align: justify;
    text-indent: 25px;
    line-height: 150%;
}

#social {
    float: right;
}

#footer {
    width: 890px;
    background: #A7DBD8;
    float: left;
    padding: 5px;
    text-align: right;
}

#footer b {
    margin-left: 10px;
}

/* End Structure */

/*images*/
.navimg {
    width: 2px;
    height: 20px;
}

#tree {
    width: 175px;
    height: 200px;
    float: right;
    margin-left: 20px;
    margin-top: 24px;
}

#blackwhite {
    width: 200px;
    height: 125px;
    float: left;
    margin-right: 20px;
    margin-top: 10px;       
}

#quickshot {
    width: 125px;
    height: 100px;
    display: block;
    margin-left: auto;
    margin-right: auto;        
}

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
<head>    
    <title>Kevin Li</title>
    <link rel="stylesheet" type="text/css" href="css/style.css" />
</head>

<body>
    <div id="wrapper">
        <div id="header">
            <h1>Kevin Li</h1>
        </div>

        <div id="links">
            <ul>
                <li><a href="#">Home</a></li>    
                <li><a href="#">Biography</a></li>    
                <li><a href="#">Portfolio</a></li>    
                <li><a href="#">Blog</a></li>    
                <li><a href="#">Images</a></li>    
                <li><a href="#">Contact Me</a></li>
            </ul>
        </div>

        <div class="sidebar_left">
            Lorem ipsum dolor sit amet...<br />
        </div>

        <div id="post">
            <b>Introduction</b><br />
            <i>Thursday, January 27, 2011</i>
        </div>

        <br />

        <div id="content">
            <img id="tree" src="images/c2_i6.png" />   
            <p>
                Lorem ipsum dolor sit amet...<br />
                <br />
                <img id="blackwhite" src="images/c3_i7.png" /> Ut venenatis diam nunc...<br />
                <br />
            </p>
        </div>

        <div id="footer">
            <b>Copyright 2010 Kevin Li</b>
        </div>
    </div>
</body>
</html>

By the way, I know it doesn't validate. I am working on that now.

Could you post the relevant HTML and CSS for us to see? Most people are not going to follow your link. — Jeremy, Commented Jan 27, 2011 at 18:36
we have a policy of requiring the important bits of the code to be in the post.. it's fine to post a "see more" link but not showing any code at all is disallowed. I also think generic requests to "review my website!" are a bit too broadly scoped as that encompasses html, css, design.. etc.. — Jeff Atwood, Commented Jan 28, 2011 at 23:48
@Jeff: Since he specifically mentioned the messiness of the CSS, I it should be added to the question as well (though it does make the question rather crowded). — sepp2k, Commented Jan 29, 2011 at 0:19
The first line is not needed and I wouldn't do one attribute per line. — user1101, Commented Jan 29, 2011 at 3:30
the XML prolog before the doctype makes IE6 trigger the quirks mode — Knu, Commented Jan 29, 2011 at 3:49

Michael K · Accepted Answer · 2011-01-28 23:39:20Z

16

A few quick comments looking through the source:

You did very well in structuring things semantically. I only see a few <br /> or <b> tags. That said, you probably want to include more semantic markup for some things, e.g.,

<div id="post">
    <b>Introduction</b><br />
    <i>Thursday, January 27, 2011</i>
</div>
<br />
<div id="content">

If I were to rework it, I would do:

<div id="post">
 <h1>Introduction</h1>
 <h2>Thursday, blah blah</h2>
</div>
<div id="content">

Then your CSS will style those elements:

div#post h1 { ... }
div#post h2 { ... }

For your images, unless you need a javascript id selector, I'd probably make them a class, rather than unique ID's. It looks like your images will all be styled similarly, so why not group them using a class? Or just override the CSS defaults for the image tag.

Finally, you should probably use a CSS reset. Browsers all use different defaults, so the only sensible thing is to use a reset so that all styling attributes start out the same across all browsers. Eric Meyer is the CSS guru, he has his reset at http://meyerweb.com/eric/tools/css/reset/ (along with more explanation about why to use it).

edited Jan 28, 2011 at 23:39

Michael K

2,8791 gold badge21 silver badges24 bronze badges

answered Jan 27, 2011 at 20:28

Travis Leleu

2611 silver badge4 bronze badges

\$\begingroup\$ might want to wrap your tags in ``` otherwise things dont show up... \$\endgroup\$
– Hailwood
Commented Jan 28, 2011 at 2:22
\$\begingroup\$ Are there such tags as or tags? \$\endgroup\$
– Geoffrey
Commented Jan 28, 2011 at 8:48
\$\begingroup\$ I would question the semantics here. <strong> could work as well. <em> should also be used over <i>. I would personally never remove unique identifiers from elements, as the flexibility of being able to style/jQuery a specific element is more efficient than navigating through a list. But a class is a good idea if styles will be reused for many images. \$\endgroup\$
– Visionary Software Solutions
Commented Jan 29, 2011 at 1:30
\$\begingroup\$ @VisionarySoftwareSolutions I think you mistook the first piece of code for an answer, I was using it as an input. OP, as you can see there are lots of good ways of doing it. As you progress, you'll develop your own style. I recommend that you be try to develop a consistent style - eg, if you prefer #post instead of div#post, that's fine, but be consistent. \$\endgroup\$
– Travis Leleu
Commented Feb 20, 2014 at 0:48

Add a comment |

Hack Saw · Accepted Answer · 2011-01-27 18:01:11Z

3

Your CSS doesn't handle what happens if the reader closes down the window to smaller than your planned size.

Test: resize window to less than 900px.
Results: Window is cut off.

This is a design issue, more than a coding issue.

edited Jan 27, 2011 at 18:01

answered Jan 27, 2011 at 17:53

Hack Saw

2651 silver badge5 bronze badges

Add a comment |

Jamal · Accepted Answer · 2014-01-05 00:21:19Z

2

A few semantic issues:

Do not use <b> tags.
If you must, you can use <strong>, or even better, <span class="bold"></span>, and then style the class in your CSS.
Do not use <i> tags.
If you must, you can use <em>, or even better, <span class="italic"></span>, and then style the class in your CSS.
Do not use <br /> tags for spacing.
If you require spacing, use margin-bottom or padding-bottom.

edited Jan 5, 2014 at 0:21

Jamal

34.9k13 gold badges133 silver badges237 bronze badges

answered Jan 28, 2011 at 2:20

Hailwood

4674 silver badges11 bronze badges

1

\$\begingroup\$ Why is <span> better than <i> or <b>? I don't see what semantic value is obtained by replacing those tags. \$\endgroup\$
– André Paramés
Commented Jan 28, 2011 at 3:12
\$\begingroup\$ Probably because the W3C says its better to use CSS. I can see the pros and cons.. various levels of bold, different color bold, etc. w3schools.com/tags/tag_phrase_elements.asp \$\endgroup\$
– Kevin
Commented Jan 28, 2011 at 3:17
4

\$\begingroup\$ But that's not a semantic issue. Unless a different class is used, I don't see how "bold" tells anything more about the content (as opposed to telling about the presentation) than <b>. \$\endgroup\$
– André Paramés
Commented Jan 28, 2011 at 3:27
7

\$\begingroup\$ @Kevin You do realise that the W3Schools have absolutely nothing to do with the W3C? \$\endgroup\$
– Yi Jiang
Commented Jan 28, 2011 at 12:23
1

\$\begingroup\$ Good call on identifying things that shouldn't be done. The rationale, however, is lacking. The point of <strong> and <em> is not to get bold text and italicized text. You can overwrite the default behavior with CSS. The point is to add semantic meaning so that intelligent queries can be done. \$\endgroup\$
– Visionary Software Solutions
Commented Jan 29, 2011 at 1:32

| Show 3 more comments

Jeremy · Accepted Answer · 2011-01-27 18:00:12Z

I didn't look at your site, as I can't afford to click on random links right now.

However, I add overflow: hidden; to the parent element to make sure it wraps its immediate floated child elements. Take my example and try it with and with out the overflow.

HTML:

<div id="page">
    <div id="main">main content</div>
    <div id="side">side content</div>
</div>

CSS:

div#page{
  overflow: hidden;
  background-color: #ccc;
}

div#main{
  float: left;
}

div#side{
  float: right;
}

potench · Accepted Answer · 2014-01-04 23:56:16Z

A couple notes:

Use HTML5

Instead of <div id="header"> consider using an <header>
Instead of <div id="post"> consider using <article>
Instead of <div class="sidebar_left"> use <aside>. Also, you should NOT use "style specific" classes such as "left", "red", etc because it's very easily overridable in CSS and no-longer relevant. I would also suggest sticking with a convention for CSS that uses dashes instead of underscores. When you look through your JavaScript it will be more legible (underscores mean JS, dashes mean selectors or DOM).
Instead of <div id="footer"> use <footer>
Instead of <div id="content"> use <section> or <div id="main" role="main">. You should use role to provide more context.
Anchors with hashes: I'm not sure why you are using a hash in your anchors (ex: <a href="#">Contact Me</a>). I see developers use it to simply enable a pointer cursor. If this is the case you should instead use:

HTML

<a>Contact Me</a>

CSS

a { cursor: pointer;}

There is no point to add a { cursor: pointer;}. The practice of using <a href="#">Link</a> is common sense to all of us that a url will be inserted later. — JGallardo, Commented Feb 3, 2014 at 20:55
Note that this question was posted in 2011, before HTML5 was available. — 200_success, Commented Mar 31, 2018 at 14:29

António Almeida · Accepted Answer · 2014-01-05 01:36:38Z

Here is some code golf tips, that can improve performance and code readability:

This:

#wrapper, #footer {
    -moz-border-radius-bottomright: 50px;
    border-bottom-right-radius: 50px;
    -moz-border-radius-topleft: 50px;
    border-top-left-radius: 50px;
    -moz-border-radius-topright: 50px;
    border-top-right-radius: 50px;
    -moz-border-radius-bottomleft: 50px;
    border-bottom-left-radius: 50px;
}

could be just this:

#wrapper, #footer {
    border-radius: 50px;
    -moz-border-radius: 50px;
}

or if the values differ:

#elem {
    border-radius: 50px 0 25px 10px; /* Top 50px, Right 0, Bottom 25px, Left 10px */
    /* or */
    border-radius: 50px 0; /* Top & Bottom 50px, Right & Left 0 */
}

This rule also applies when you are using margin's or padding's, example:

margin: 0 auto;
margin-top: 30px;

could be:

margin: 30px auto 0;

JGallardo · Accepted Answer · 2014-02-03 21:14:15Z

Leave out empty CSS rules

In your example, you have these

h2 {}

p {}

You can have one file for development that has those so you can work on them later. But in your production file, leave those out to reduce your file size. It also makes your files more readable when you submit them for review.

Do not use <br> tags for spacing

<br />
<img id="blackwhite" src="images/c3_i7.png" /> Ut venenatis diam nunc...<br />
<br />

If you want to create white space around images. You could do something like

img { margin: 6px; }

You could of course make that margin anything you want. Or you could pass down rules based on how you want them to display at certain cases.

Use class instead of id more often.

In that same element you have this

blackwhite is not something that sounds like it would describe a specific element in a document. But as a matter fact, it looks like you are creating a blog considering how there is a "post." And because you will have many images on this page, you should make this classes.

Use absolute paths

The images/c3_i7.png in <img id="blackwhite" src="images/c3_i7.png" /> would become /images/c3_i7.png which is good on a server because it helps prevent errors if things get moved around.

Stack Exchange Network

Personal website, done after one week of learning HTML and CSS

7 Answers 7

Your Answer

Not the answer you're looking for? Browse other questions tagged
beginner
html
css
or ask your own question.

Hot Network Questions

Personal website, done after one week of learning HTML and CSS

7 Answers 7

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged beginnerhtmlcss or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
beginner
html
css
or ask your own question.