Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've been writing HTML for a while, but badly, very badly. I have been violating every best practice that exists

So I have decided to clean up my act. I bought a couple up-to-date books, read them, and am going to write every document I need to write for a couple years using HTML until I feel I am doing it right.

Here's my first effort at reformation, a business letter I wrote for a friend. I would appreciate a review. (Yes, I know this is a trivial example and it would be much more beneficial to have something reviewed that is more complicated. But I want to nip bad practices in the bud now, before they become embedded in me.)

<!DOCTYPE HTML>

<html>
    <head>
        <style type="text/css">
            .bodyBody {
                margin: 10px;
                font-size: 1.50em;
            }
            .divHeader {
                text-align: right;
                border: 1px solid;
            }
            .divReturnAddress {
                text-align: left;
                float: right;
            }
            .divSubject {
                clear: both;
                font-weight: bold;
                padding-top: 80px;
            }
            .divAdios {
                float: right;
                padding-top: 50px;
            }
        </style>
    </head>
    <body class="bodyBody">
        <div class="divReturnAddress">
            Lopes de Almeido, Evanildo<br/>
            Bl&auml;siring 161 <br/>
            4057 Basel <br/>    
            <br/>
            01 Dezember, 2012
        </div>

        <div class="divSubject">
            Betreff:  Vertretung waehrend Reise
        </div>

        <div class="divContents">
            <p>
                Sehr geehrte Frau Graf,
            </p>

            <p>
                Ich fliege nach Brasilien am 29. Dezember 2012 f&uuml;r 6 Wochen.
                Meine Nichte kann mich vertreten.  Sie arbeitet sehr gesissenhaft.
                Sie heisst Yasmin.  Ihre Nummer ist xxx.
            </p>
        </div>

        <div class="divAdios">
            Freundliche Gr&uuml;sse <br/>
            <!-- Space for signature. -->
            <br/>
            <br/>
            <br/>
            Evanildo Lopes do Almeida <br/>
            Hauswart Binningerstrasse 19/23 <br/>
        </div>
    </body>
</html>
share|improve this question
    
Useful: w3.org/TR/css3-selectors/#selectors –  ANeves Dec 6 '12 at 9:40

4 Answers 4

CSS

Since this is a single document, not a webpage of a website, it's fine to use internal CSS in the style element instead of linking to an external stylesheet. However, if you plan to write more letters in the future, you should create a stylesheet that includes all the shared styles that are typical for a letter and link to it from each letter (<link rel="stylesheet" href="letter.css" />).

You could name your classes more meaningful, but this is only a good practice, not a requirement.

You could omit the class on body.

Encoding/Charset

You should add the encoding to the document. If you want to use UTF-8 (recommended), add <meta charset="utf-8" /> as the first child of the head element.

title element

You are missing the title element, which is required. Add it as child of head: <title>Vertretung während Reise – 1. Dezember 2012</title>

Return address

The use of <br/> is correct here. However, the date is not part of the address, and therefor it shouldn't be separated by br. Put the date in its own element instead. HTML5 offers the time element.

Because this is your address (and not the recipient address), you should enclose it in the address element.

You could enclose the return address and the date in the header element. It might also be possible to enclose the return address in a footer element and the time in a header (probably together with the h1), but this could be overkill ;)

Subject line / Betreff

Instead of using a meaningless div for the subject, you should use h1 here, as it is the heading of the whole document.

Salution / Anrede

It's arguably if the salution is a paragraph on its own, or if it is part of the first paragraph. I think when followed by a comma, it should be part of the following paragraph. But this is not undisputed (I have no detailed knowledge about the semantics of a letter).

Greets

The use of the br element to separate the greeting from your anme is correct here. But don't use several br elements just because you want to add more spacing. Use only one and apply a class to it, so that you can give it a higher vertical margin (CSS).

Also, include this in a p instead of a div.


Example

So your letter could look like:

<!DOCTYPE html>

<html>

  <head>
    <meta charset="utf-8" /> <!-- first element so that the encoding is applied to the title etc. -->
    <title>Vertretung während Reise – 1. Dezember 2012</title>
    <link rel="stylesheet" href="letter.css" />
  </head>

  <body>

    <header> 
      <address class="return-address">
        Lopes de Almeido, Evanildo<br/>
        Bläsiring 161 <br/>
        4057 Basel <br/>    
      </address>

      <time datetime="2012-12-01">01 Dezember, 2012</time>
    </header>

    <h1>
      Betreff:  Vertretung waehrend Reise
    </h1>

    <div class="content"> <!-- use this div only if it is required for styling -->
        <p>
          Sehr geehrte Frau Graf,
          <br class="salution" />

          Ich fliege nach Brasilien am 29. Dezember 2012 für 6 Wochen.
          Meine Nichte kann mich vertreten.  Sie arbeitet sehr gesissenhaft.
          Sie heisst Yasmin.  Ihre Nummer ist xxx.
        </p>
        <p>
          …
        </p>
      </div>

      <p class="adios">
        Freundliche Grüsse 
        <br class="greets" />
        Evanildo Lopes do Almeida <br/>
        Hauswart Binningerstrasse 19/23
      </p>

  </body>

</html>
share|improve this answer
    
Would the higher vertical margin for the <br> element be done like this br.greets { margin: 100px; } where 100px would be adjusted for the space I wanted? –  John Fitzpatrick Dec 2 '12 at 9:59
    
@John: Yes. You could also use .greets as selector, so this style can be used even if you decide to use p instead of br in the future. And instead of px you could use the unit em, which always is in relation to the font-size of parents (so if you have a higher font-size, the margin gets higher, too). –  unor Dec 2 '12 at 14:33
    
For the detailed semantics of a letter, a good forum to consult might be english.stackexchange.com –  ANeves Dec 6 '12 at 9:39
    
@ANeves: Or german.stackexchange.com in this case. I guess letter "conventions" differ by language (or more so, country). –  unor Dec 6 '12 at 12:54

First of all, since you use HTML to write a business letter I assume you want to send an HTML e-mail? Since e-mail clients are far away from adopting web standards and require special tactics, this might not be the best area to practise writing HTML. For example, you should separate your CSS from the HTML (as Ben suggests) but aren't able to do this in HTML E-Mail.

Now, focusing on your code, I can mainly echo what Ben writes (especially using div.name instead of divName, using the body-tag body instead of the selector .bodyBody).

Additionally, I would suggest you use utf-8 encoding, so you can type your German umlauts (ä,ö,ü) instead of &auml;. Do this by inserting this line right after your head (and actually saving in UTF-8 in your text editor!)

<head> 
  <meta charset="utf-8"> 

Also, use <br /> only when you actually want to have a line break. It is commonly abused as a 'quick and dirty' spacer, the better way would be, to use a <p> tag for your Freundliche Grüße and give it a margin, like so:

div.adios p {
  margin-bottom: 30px;
}

If you have several <p> you can select only the first child like this:

div.adios p:first-child {

Finally, I think I would wrap .subject into .contents, but that is just personal preference. Maybe, semantically, it would even make sense to just use the h1 tag instead of .subject like so:

<div class="contents">
  <h1>Betreff: Vertretung während Reise</h1>
  <p>Sehr geehrte Frau Graf,</p>
  <p>Ich fliege am 29. Dezember 2012 für 6 Wochen nach Brasilien...</p>
</div>
share|improve this answer

First of all, use external css file. Create style.css, put there all your css code and then link it in html file inside head tags:

<link rel="stylesheet" href="style.css">

There is no need to prefix class with selectors name. It's not semantic and you're limiting yourself to use it only on that specific selector. Not literally but semantically.

If you have two words in class name, don't write it like this: myCustomClass, use dash instead: my-custom-class.

Read this guide for css, it's very good: https://github.com/necolas/idiomatic-css and this guide for html: https://github.com/necolas/idiomatic-html

share|improve this answer

Consider using microformats for appropriate data. This may make it more searchable. Consider using hCard for example

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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