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>

What are "{block:IndexPage}" type like things? I never seen them in html. — jimjim, May 9, 2012 at 3:46
@Matthew I posted my full detailed review for your tumblr theme — Vladimir Starkov, Jul 30, 2012 at 11:20

Drew Chapin · Accepted Answer · 2012-05-09 02:45:57Z

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; }

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! — waitinforatrain, May 9, 2012 at 5:51

Vladimir Starkov · Accepted Answer · 2012-07-21 20:53:14Z

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: '&raquo;';
}

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:

unofficial guide is unavailable by direct link now, so check it in wayback machine web.archive.org/web/20121021062653/http://disattention.com/labs/… — Vladimir Starkov, Sep 1, 2013 at 15:58

Community · Accepted Answer · 2017-05-23 12:40:53Z

+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?

Stack Exchange Network

HTML and CSS for a theme engine

3 Answers 3

Small recommendations for CSS:

Small recommendations for HTML:

Title code of block:

`#navigation` code of block:

Tags code of block:

`{PostType}` code of block:

Quote code of block:

Link code of block:

Chat code of block:

Links from answer and some additionals

CSS Links:

HTML links:

Tumblr links:

Your Answer

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

Hot Network Questions

HTML and CSS for a theme engine

3 Answers 3

Small recommendations for CSS:

Small recommendations for HTML:

Title code of block:

#navigation code of block:

Tags code of block:

{PostType} code of block:

Quote code of block:

Link code of block:

Chat code of block:

Links from answer and some additionals

CSS Links:

HTML links:

Tumblr links:

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions

`#navigation` code of block:

`{PostType}` code of block:

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