HTML and CSS for a theme engine

Question

I was wondering if there was any way I would be able to improve my HTML/CSS. Would you mind reviewing the following and telling me what I should do?

CSS:

<style type="text/css">
 body {
 font-family:Georgia, Palatino, Times, 'Times New Roman', serif;
 color:#333333;
 font-size:10px;}
 h1 {
 display:inline;
 color:#A8DBA8}
 h2 {
 display:inline;
 color:#333333;
 font-style:italic;}
 h3 {
 font-weight:bold;
 margin-bottom:5px;}
 li {
 list-style-type:none;
 margin:0;
 padding:0;}
 #portrait {
 margin-left:auto;
 margin-right:0px;
 margin-bottom:10px;
 -moz-border-radius: 64px; 
 -webkit-border-radius: 64px; 
 width: 64px;
 height: 64px; 
 background-image:url({PortraitURL-64})}
 #container {
 margin-left:300px;
 margin-top:10px;
 width:500px;}
 #tags, #notes {
 margin-top:10px;}
 .sidebar {
 position:fixed;
 width:200px;
 margin-left:75px;
 margin-top:275px;
 text-align:right;}
 .sidebar #description {
 margin-bottom:10px;}
 .sidebar #navigation {
 margin-bottom:10px;}
 .entry {
 overflow:scroll;
 margin-bottom:10px;}
 .entry #tags {
 color:#CCCCCC;}
 .entry #quote_source {
 float:right;}
 .entry #audio {
 width:100%;
 height:100%;
 background-color:#000000;}
 #audio {
 width:500px;
 height:29px;
 background-color:#000000;}
 a:link, a:visited {
 color:#333333;
 text-decoration:none;}
 a:hover {
 color:#A8DBA8;}
 a.tag:link, a.tag:visited {
 color:#CCCCCC;
 text-decoration:none;}
 a.tag:hover {
 color:#CCCCCC
 text-decoration:underline;}
</style>

HTML:

<div class="sidebar">
 <div id="portrait"></div>
 <div id="description">
 <h2>"{Description}"</h2>
 </div>
 <div id="navigation">
 <a href="/">Home</a> |
 <a href="/about">About</a> |
 <a href="/ask">Message</a> |
 <a href="/archive">Archive</a>
 </div>
</div>
<div id="container">
 {block:Posts}
 {block:Text}
 <div class="entry" id="text">
 {block:IndexPage}
 {/block:IndexPage}
 {block:Title}
 <h3>{Title}</h3>
 {/block:Title}
 {Body}
 </div>
 {/block:Text}
 {block:Photo}
 <div class="entry" id="photo">
 {block:IndexPage}
 <a href="{Permalink}"><img src="{PhotoURL-500}"></a>
 {/block:IndexPage}
 {block:PermalinkPage}
 <a href="{LinkURL}"><img src="{PhotoURL-500}"></a>
 {block:HasTags}
 <li id="tags">
 Tagged: 
 {block:Tags}
 <a href="{TagURL}" class="tag">{Tag}</a>
 {/block:Tags}
 </li>
 {/block:HasTags}
 {/block:PermalinkPage}
 </div>
 {/block:Photo}
 {block:Photoset}
 <div class="entry" id="photoset">
 {Photoset-500}
 </div>
 {/block:Photoset}
 {block:Quote}
 <div class="entry" id="quote">
 <li id="quote_content">
 {Quote}
 </li>
 <li class="entry" id="quote_source">
 - {Source}
 </li>
 </div>
 {/block:Quote}
 {block:Link}
 <div class="entry" id="link">
 <li><a href="{URL}" target="_blank">{Name} &raquo;</a>
 </ul>
 {/block:Link}
 {block:Chat}
 <div class="entry" id="chat">
 {block:Lines}
 {block:Label}
 {Label} {Line} <br />
 {/block:Label}
 {/block:Lines}
 </div>
 {/block:Chat}
 {block:Audio}
 <div class="entry" id="audio">
 {AudioPlayerBlack}
 {block:PermalinkPage}
 <div id="audio_description">
 {block:TrackName}
 {TrackName}
 {/block:TrackName}
 by
 {block:Artist}
 {Artist}
 {/block:Artist}
 </div>
 {/block:PermalinkPage}
 </div>
 {/block:Audio}
 {block:Video}
 <div class="entry" id="video">
 {Video-500}
 </div>
 {/block:Video}
 {block:Answer}
 <div class="entry" id="answer">
 <li id="answer_question">
 {Asker}: &ldquo;{Question}&rdquo;
 </li>
 <li id="answer_response">
 {Answer}
 </li>
 </div>
 {/block:Answer}
 {/block:Posts}
</div>

• jimjim: What are "{block:IndexPage}" type like things? I never seen them in html.
• Matthew: They're for a theme engine.
• Vladimir Starkov: @Matthew I posted my full detailed review for your tumblr theme

Answer 1

The first thing that really caught my eye is your usage of margin-.... If I am setting more than two sides of margin I like to use the syntax margin: <top> <right> <bottom> <left>; So this...

#portrait {
 margin-left:auto;
 margin-right:0px;
 margin-bottom:10px;
 -moz-border-radius: 64px; 
 -webkit-border-radius: 64px; 
 width: 64px;
 height: 64px; 
 background-image:url({PortraitURL-64})}

becomes this...

#portrait {
 margin: auto 0px 10px auto;
 -moz-border-radius: 64px; 
 -webkit-border-radius: 64px; 
 width: 64px;
 height: 64px; 
 background-image:url({PortraitURL-64})}

---

I noticed you are missing a semi-colon

a.tag:hover {
 color:#CCCCCC /* here */
 text-decoration:underline;}

---

According to the W3C Validation service, your HTML does not conform to the HTML 4.01 Strict Standard because of 13 errors... I'm not going to list them all here, but you can to http://validator.w3.org/check and paste your HTML, select HTML 4.01 Strict, and view the results.

---

Also... This isn't important, but I personally find the way you position your curly braces to be unusual. I usually only see one of the following syntax styles...

#portrait {
 margin: auto 0px 10px auto;
 width: 64px;
 height: 64px;
}
#portrait 
{
 margin: auto 0px 10px auto;
 width: 64px;
 height: 64px;
}
#port { margin:auto 0px 10px auto; width:64px; height:64px; }

• waitinforatrain: Agree about the curly braces, but I think it is important. You'll find that out soon enough when you have a stray curly brace!

Answer 2

All links from the answer was included to the bottom.

---

Small recommendations for CSS:

/* space after colon required */
body {
 /* Georgia is more spread in OS than Palatino that's why browser will always select 
 Georgia and never Palatino, that's why you must swapped these two fonts 
 http://meiert.com/en/blog/20080220/helvetica-arial/ */
 font-family: Palatino, Georgia, Times, 'Times New Roman', serif;
 /* If you can use short syntax for color codes (#333333 = #333, #000000 = #000) */
 color: #333;
 font-size: 10px;
}
/* ‘Your selector’s intent must match that of your reason for styling something;
 ask yourself ‘am I selecting this
 because it’s a ul inside of .header or because it is my site’s main nav?’.’
 http://csswizardry.com/2012/07/shoot-to-kill-css-selector-intent/ */
h1 {
 display: inline;
 color: #A8DBA8
}
/* color is unnessesary because is cascaded from body */
h2 {
 display: inline;
 font-style: italic;
}
/* `font-weight: bold;` is unnessesary because it is default behaviour */
h3 {
 margin-bottom: 5px;
}
/* drop default list-style from all lists on page is not very good idea,
 use `.nav` abstraction instead 
 http://csswizardry.com/2011/09/the-nav-abstraction/ */
li {
 /* in this situation `list-style-type` equal to `list-style` */
 list-style: none;
 margin: 0;
 padding: 0;
}
/* `moz` prefix can be dropped if only you are not support Firefox 3.6 
 http://hacks.mozilla.org/2010/09/firefox-4-recent-changes-in-firefox/ */
#portrait {
 margin-left: auto;
 margin-right: 0;
 margin-bottom: 10px;
 /* use this smart syntax for vendor prefixes for better readability */
 -webkit-border-radius: 64px;
 border-radius: 64px;
 width: 64px;
 height: 64px;
 /* wrap url value in quotes for better syntax highlghting */
 background-image: url('{PortraitURL-64}');
}
/* use Top-Right-Bottom-Left rule to describe detailed properties
 also try not to use id for selectors
 http://csswizardry.com/2011/09/when-using-ids-can-be-a-pain-in-the-class/ */
#container {
 margin-top: 10px;
 margin-left: 300px;
 width: 500px;
}
/* divide muilti-selectors to one selector on one line for better diffs in SVN or GIT */
#tags,
#notes {
 margin-top: 10px;
}
.sidebar {
 position: fixed;
 width: 200px;
 margin-left: 75px;
 margin-top: 275px;
 text-align: right;
}
/* selectors is overqualified 
 and may be merged into one rule 
 http://csswizardry.com/2011/09/writing-efficient-css-selectors/
 http://csswizardry.com/2012/07/quasi-qualified-css-selectors/ */
#description,
#navigation {
 margin-bottom: 10px;
}
.entry {
 overflow: scroll;
 margin-bottom: 10px;
}
/* selector is overqualified
 old code: .entry #tags */
#tags {
 color: #CCC;
}
/* selector is overqualified
 old code: .entry #quote_source */
#quote_source {
 float: right;
}
/* `#audio` and `.entry audio` may be swapped to delete repeated `background-color`
 `background-color` in this situation is equal to simple `background` */
#audio {
 width: 500px;
 height: 29px;
 background-color: #000;
}
.entry #audio {
 width: 100%;
 height: 100%;
}
/* divide muilti-selectors to one selector on one line for better diffs in SVN or GIT */
a:link,
a:visited {
 color: #333;
 text-decoration: none;
}
a:hover {
 color: #A8DBA8;
}
/* divide muilti-selectors to one selector on one line for better diffs in SVN or GIT */
a.tag:link,
a.tag:visited {
 color: #CCC;
 text-decoration: none;
}
a.tag:hover {
 color: #CCC;
 text-decoration: underline;
}

---

Small recommendations for HTML:

Title code of block:

{block:Title}
 <h3>{Title}</h3>
{/block:Title}

Problems:

• Post page has no h1 with post's title.

Solution:

HTML:

<!-- Tumblr post title non-minified description
 https://gist.github.com/2795206 -->
<!-- snippet outputs `h2` on posts list pages and `h1` on single post pages -->
{block:Title}
 <{block:IndexPage}h2{/block:IndexPage}{block:PermalinkPage}h1{/block:PermalinkPage} title="{Title}" class="heading">
 {block:IndexPage}<a href="{Permalink}" title="{Title}" class="heading">{/block:IndexPage}
 {Title}
 {block:IndexPage}</a>{/block:IndexPage}
 </{block:PermalinkPage}h1{/block:PermalinkPage}{block:IndexPage}h2{/block:IndexPage}>
{/block:Title}

---

#navigation code of block:

<div id="navigation">
 <a href="/">Home</a> |
 <a href="/about">About</a> |
 <a href="/ask">Message</a> |
 <a href="/archive">Archive</a>
</div>

Problems:

• Not use | symbols just for design, you can reproduce this visual elements with this CSS and HTML;
• Unordered list is more semantical, accessible and seo-optimized alternative for your variant of menu;
• Use .nav abstraction for reset margin,padding and list-style.

Solution (demo on dabblet):

HTML:

<ul id="navigation" class="nav">
 <li>
 <a href="/">Home</a>
 </li>
 <li>
 <a href="/about">About</a>
 </li>
 <li>
 <a href="/ask">Message</a>
 </li>
 <li>
 <a href="/archive">Archive</a>
 </li>
</ul>

CSS:

.nav {
 margin: 0;
 padding: 0;
 list-style: none;
}
#navigation {
 overflow: hidden; /* clear float */
}
#navigation li {
 float: left;
 padding: 0 10px;
}
#navigation li + li {
 border-left: 1px solid gray;
}

---

Tags code of block:

{block:HasTags}
<li id="tags">
Tagged: 
 {block:Tags}
 <a href="{TagURL}" class="tag">{Tag}</a>
 {/block:Tags}
</li>
{/block:HasTags}

Problems:

• There are not ul wrapper;
• #tags blocks existed for each post, so you will have duplicating id's.

Solution:

HTML:

{block:HasTags}
<div class="tags">
Tagged: 
 <ul>
 {block:Tags}
 <li>
 <a href="{TagURL}" class="tag">{Tag}</a>
 </li>
 {/block:Tags}
 </ul>
</div>
{/block:HasTags}

---

{PostType} code of block:

<div class="entry" id="text"> ...
<div class="entry" id="photo"> ...
<div class="entry" id="photoset"> ...
/* and others */

Problems:

• #text blocks existed for each text post-type, so you will have duplicating id's;
• If you are using tumblr engine you can use simple {PostType}.

Solution:

HTML:

<div class="entry text"> ...
<div class="entry photo"> ...
<div class="entry photoset"> ...
/* or if Tumblr */ 
<div class="entry {PostType}"> ...

---

Quote code of block:

{block:Quote}
 <div class="entry" id="quote">
 <li id="quote_content">
 {Quote}
 </li>
 <li class="entry" id="quote_source">
 - {Source}
 </li>
 </div>
{/block:Quote}

Problems:

• There are not ul wrapper;
• Use special, semantical blockquote element for quotes instead ul element;
• Duplicating id’s.

Solution:

HTML:

{block:Quote}
 <div class="entry {PostType}">
 <blockquote class="quote_content">
 {Quote}
 </blockquote>
 <p class="quote_source">
 - {Source}
 </p>
 </div>
{/block:Quote}

---

Link code of block:

{block:Link}
<div class="entry" id="link">
 <li><a href="{URL}" target="_blank">{Name} &raquo;</a>
 </ul>
{/block:Link}

Problems:

• There is no end of div wrapper;
• There is no ul wrapper for li elements;
• Duplicating id’s problem;
• Leaked SEO PR states — use rel="nofollow" solution;
• Non­-semantical additional symbols, use css generated content instead;
• target="_blank" is depricated construction — give your user choice in which tab to open your pages.

Solution:

HTML:

{block:Link}
<div class="entry {PostType}">
 <a href="{URL}" rel="nofollow">{Name} </a>
</div>
{/block:Link}

CSS:

.entry.link a:after {
 content: '»';
}

---

Chat code of block:

{block:Chat}
 <div class="entry" id="chat">
 {block:Lines}
 {block:Label}
 {Label} {Line} <br />
 {/block:Label}
 {/block:Lines}
 </div>
{/block:Chat}

Problems:

• Never use <br /> element for markup, it exist only for text;
• Duplicating id’s problem;
• Situation for using ul element.

Solution:

HTML:

{block:Chat}
 <div class="entry {PostType}">
 {block:Lines}
 <ul>
 {block:Label}
 <li>
 {Label} {Line}
 </li>
 {/block:Label}
 </ul>
 {/block:Lines}
 </div>
{/block:Chat}

---

Links from answer and some additionals

CSS Links:

• "helvetica, arial", Not "arial, helvetica"
• Shoot to kill; CSS selector intent
• Firefox 4: recent changes in Firefox
• The ‘nav’ abstraction
• When using IDs can be a pain in the class...
• Writing efficient CSS selectors
• Quasi-qualified CSS selectors
• 70 Expert Ideas For Better CSS Coding (not all ideas are ideal but some of them is quite usefull)

HTML links:

• HTML5 Semantics

Tumblr links:

• How to create a custom HTML theme
• Unofficial Theme Guide

• Vladimir Starkov: unofficial guide is unavailable by direct link now, so check it in wayback machine web.archive.org/web/20121021062653/http://disattention.com/labs/…

Answer 3

+1 to @druciferre, but I'd keep the

#portrait {
 margin-left:auto;
 margin-right:0px;
 margin-bottom:10px;
}

margin settings. It's easier to read (and modify) since readers don't have to memorize the top, right, bottom, left order.

---

A common style about whitespace would improve the readability a little bit. Sometimes there is a space before the value, sometimes not.

width:200px;
...
width: 64px;

---

<a href="/">Home</a> |
<a href="/about">About</a> |
<a href="/ask">Message</a> |
<a href="/archive">Archive</a>

This could be an unordered list formatted by a proper CSS (as Drupal does, for example). Why use list to do navigation menu instead of buttons?