current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 5 down vote favorite

I'm starting up a new project, and I've always had an issue getting vertical alignment right in CSS. Is there anything I could do better that what I've currently come up with? Replacing the header and nav with divs, and changing the rgba values to hex values this seems to work all the way back to IE7 which I'm happy with.

CSS

html, * {
    -moz-box-sizing:border-box;
    -webkit-box-sizing:border-box;
    -ms-box-sizing:border-box;
    -o-box-sizing:border-box;
    box-sizing:border-box;
}


body {
    margin:0;
    padding:0;
}

nav {
    background-color:rgba(0, 0, 0, 0.1);
}

nav a.tab {
    text-align:center;
    padding:0 15px;
    color:#FFFFFF;
    display:inline-table;

    vertical-align:middle;  text-decoration:none;
}

nav a.tab > span {
    display:table-cell;
    vertical-align:middle;
}

nav a.tab:hover {
    background-color:rgba(0, 0, 0, 0.1);
    text-decoration:underline;
}

header {
    background-color:#27AE60;
    font-size:1.2em;
    color:#FFF;
    display:table;
    width:100%;
}

header, header h1, header nav, header nav a.tab {
    height:80px;
    vertical-align:middle;
}

header nav {
    display:inline-block;
}

header h1 {
    margin:0 20px;
    display:inline;
    text-shadow:2px 2px 2px #333333;
}

HTML

<header>
    <h1>Projects</h1>
    <nav>
        <a href="/" class="tab active">
            <span>Home</span>
        </a>
        <a href="/projects" class="tab">
            <span>Projects</span>
        </a>
        <a href="/" class="tab">
            <span>Contact</span>
        </a>
    </nav>
</header>

jsFiddle

share|improve this question

3 Answers 3

up vote 2 down vote accepted

Your code is good - both the CSS and the HTML validate at the validators:

HTML validator
CSS validator

However, you should put your wrap your HTML code like this:

<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8">
        <title>Page title here...</title>
        <!-- other links here, such as external CSS/JS files -->
    </head>
    <body>
        <!-- your code here -->
    </body>
</html>

I really like how you use the HTML5 header element instead of using the old style <div class="header">

When people write CSS, they often format it slightly different, like this:

nav a.tab > span {
    display:        table-cell;
    vertical-align: middle;
}

It is just easier to read this way. Also, you should have each element have its own line, not put many on the same line like this:

vertical-align:middle;  text-decoration:none;

Otherwise, this looks good to me.

share|improve this answer
    
It actually is inside all the html tags etc, I just posted the relevant part to the question :), and also those rule we're suppose to be on separate lines, I just messed up the copy paste haha :P. I haven't actually seen CSS wrote with the spaces like that, but it is a whole lot easier to read, so I'm gonna change it all over now. Thanks for your tips :) –  Tom Hart Feb 10 at 20:28
    
Glad to help. Yes, I thought this might just be the relevant part, but I have seen so much bad HTML that displays different in every browser out there that I always put this in my answers. –  Hosch250 Feb 10 at 21:22
up vote 1 down vote

There's one more way to center vertically

element { 
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
  position: absolute;
}
share|improve this answer
    
If you're hoping to support IE8 or older, this method will be outright broken because the transform property is only supported in IE9+ (note that the OP specifically mentions that IE7 support is a "good thing"). –  cimmanon Feb 13 at 13:26
    
yes, that's right –  dimitryeurope Feb 13 at 19:07
up vote 3 down vote

I only have some nitpicks about formatting and readability. Hopefully somebody else will comment specifically about good approaches for treating vertical alignment.

It would be more readable (and the common convention) to put a space after colons. So instead of:

background-color:rgba(0, 0, 0, 0.1);
text-decoration:underline;

Write as:

background-color: rgba(0, 0, 0, 0.1);
text-decoration: underline;

And don't put two definitions on the same line like this:

vertical-align:middle;  text-decoration:none;

It's better when code reads from top to bottom, and your eyes can stay focusing on the left, rather than dipping deeper into the right part, like this:

vertical-align: middle;
text-decoration: none;
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.