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
1

I'm trying to accomplish something very simple as seen here.

HTML

<div id="header">
    <div class="container">
        <img src="#">
        <ul id="header_nav">
            <li>Link</li>
            <li>Link</li>
            <li>Link</li>
        </ul>
    </div>
</div>

CSS

.container {
    padding-left: 80px;
    padding-right: 80px;
}
#header {
    height: 50px;
    line-height: 50px;
    background-color: blue;
}
#header_nav {
    float:right;
}
#header_nav li {
    display: inline-block;
}

Questions:

  1. Am I using the IDs and classes correctly? For example, should header be an ID or class?
  2. Is my naming convention ok?
  3. Should I be using nested rules instead? E.g #header ul li { ... }
  4. Is this the best way to achieve this layout?

Any advice at this stage would be good, so I can take these practices forward when doing the next steps of the structure.

share|improve this question
    
Is this your full HTML document, or did you omit the DOCTYPE etc. for this question? Which HTML version are you using? –  unor Sep 1 '14 at 14:23
    
Since this is a JSFiddle, the HTML5 DOCTYPE (<!DOCTYPE html>) is automatically included (although it would be beneficial for the poster to learn about the different doctypes, how they differ, and what HTML5 offers). –  WChargin Sep 1 '14 at 18:34

3 Answers 3

up vote 10 down vote accepted

Am I using the IDs and classes correctly? For example, should header be an ID or class?

From your example I don't see anything wrong with them. Just remember that ID's are supposed to be unique within your entire HTML page and classes not. So if you're planning to have more header's (note the exact same name) on your page, make it a class.

Is my naming convention ok?

Yes, as long as you enforce a consistent naming convention you should be okay.

Should I be using nested rules instead? E.g #header ul li { ... }.

This actually depends on your entire HTML structure and personal preference. Most of the time I find #header ul li { ... } being cleaner, because you're applying the CSS rules to the ul li's in the #header without having to define the class or id in the HTML.

Is this the best way to achieve this layout?

  • If you have an element that contains only one horizontal line of elements to be centered vertically, more often then not defining height with the same value of line-height becomes redundant.

  • Get rid of your container class and div and move it's CSS rules to the #header block. The #header is the container already.

HTML

<div id="header">
    <img src="#">

    <ul>
        <li>Link</li>
        <li>Link</li>
        <li>Link</li>
    </ul>
</div>

CSS

#header {
    line-height: 50px;
    padding: 0 80px;
    background-color: blue;
}

#header ul {
    float: right;
}

#header li {
    display: inline-block;
}

Update: Changed #header ul li to #header li, see @cimmanon's comment.

JSFiddle

share|improve this answer
1  
There's no reason to specify #header ul li over #header li. The li element can only be a descendant of ul elements. The only reason to have the ul be a part of the selector is if you're trying to single out specific ul/li elements via more specific selectors (eg. #header > ul > li). –  cimmanon Sep 1 '14 at 12:52
    
@cimmanon That's true. In my case it's just preference. I updated my answer. –  Kid Diamond Sep 1 '14 at 13:48
    
I think, its not quite true!! The <li> element can also be a descendant of an <ol>. Maybe not in this case, but lets say you dont know if #header will have <ul> or <ol>, and lets say, there are diffrences between this two in styling... then #header ul li would make sens to me... –  reyaner Sep 5 '14 at 15:40
    
@reyaner What is not true? I never said that they couldn't be descendants of those. –  Kid Diamond Sep 5 '14 at 15:47
    
cimmanon said it:"The li element can only be a descendant of ul elements.", und you said: "That's true"... am i wrong? –  reyaner Sep 5 '14 at 15:54
up vote 11 down vote

You should be using HTML 5 elements such as header and nav making some/all classes and IDs unnecessary:

<header>
    <nav>
        <img src="#">
        <ul>
            <li>Link</li>
            <li>Link</li>
            <li>Link</li>
        </ul>
    </nav>
</header>
share|improve this answer
1  
Only if he's using the HTML5 doc type, which you really cant determine from looking at the code he posted. –  Kid Diamond Sep 1 '14 at 10:49
1  
@KidDiamond Well, theoretically speaking, yes, however practically speaking, no. Browsers don't worry about the exact doctype used. In most browsers HTML 5 elements will work fine, no matter what doctype you use, if at all. In the rest (such as IE) you'll need any doctype, that triggers standard mode, which you should have anyway. That being said, nowadays there is no reason not to use the HTML 5 doctype. –  RoToRa Sep 1 '14 at 10:56
    
I'm just saying. Some people want to make code that is cross browser compatible all the way back to IE7 (even 6), which is nuts imo. And some like to have completely valid HTML code according to the specifications. So it all depends on the requirements. –  Kid Diamond Sep 1 '14 at 10:59
1  
@KidDiamond Based on the use of display: inline-block by itself, I think it's safe to say that IE6-7 support is not something the OP cares about. These are dead browsers. Unless support for them is specified, you should assume that they can be ignored. –  cimmanon Sep 1 '14 at 11:58
    
@cimmanon Fair enough. :) –  Kid Diamond Sep 1 '14 at 12:05
up vote 4 down vote

Should I be using nested rules instead?

You should use the least specific selector possible to target exactly the element(s) you want. If your choice is between .foo .bar and .bar and you will only have a single element with a class of bar in your document, use .bar as your selector.

Selectors like these are all longer than necessary:

  • table td / table th / tr td / tr th
  • ul li / ol li / dl dt / dl dd
  • figure figcaption
  • fieldset legend

In each of these cases, the second element in the selector must be a descendant of the first element in order to be valid HTML. Unless you're fighting specificity issues, there's no benefit to being more specific than td or li.

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.