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'm writing a markup for:


And I've split it for these sections:

<header>
    <div id="logo"></div>
    <nav id="mainNav"></nav>    
</header>
<div id="banner"></div>
<div id="content">
    <section id="advantages">
        <section class="advantage"></section>
        <section class="advantage"></section>
        <section class="advantage"></section>
        <section class="advantage"></section>
    </section>
    <div id="main"></div>
    <aside></aside>
</div>
<footer>
    <div id="extra">
        <article class="extra"></article>
        <article class="extra"></article>
        <article class="extra"></article>
        <article class="extra"></article>
    </div>
    <div id="credit"></div>
</footer>

Is it semantically correct(div, section, article) and are my identifiers explicit?

share|improve this question
1  
The template is here: templates.websitetemplatesonline.com/Progress-business/… and I don't know if it's OK to rip it off like this. –  Quentin Pradet Feb 18 '13 at 7:02

2 Answers 2

up vote 3 down vote accepted

#logoh1

Your <div id="logo"></div> should be a h1, otherwise the document outline wouldn't be correct:

<h1><img id="logo" src="logo.png" alt="Progress Business Company"></h1>

See http://stackoverflow.com/a/14920215/1591669 for an explanation.

#bannerheader or aside

If your <div id="banner"></div> doesn't contain main content, it should go in an aside element, or, if it can be understood as part of the header (probably on each page of the site), in the header element.

#mainsection (or article)

Instead of <div id="main"></div> you could use a sectioning element here. I assume your div contains a heading ("Welcome, dear visitor!"), so you have something like an implict section anyhow. Why not make it explicit? So use a section instead of the div (depending on the actual content, article might be suitable instead).

#content > asidesection

I'd say on the front page the testimonials could be considered part of the main content (in the end it depends on the real content/context), so instead of aside you could use section then.

Each single testimonial should be in its own article, of course.

#extra > articlesection

I don't think article elements in the footer are suitable for that kind of content (unless your actual page contains something different). (Only the newsletter form could possibly get article instead of section, but that's not that clear; personally I'd use section for it, too).

So, I'd go with section here. If the contact details listed under "Address" are relevant for the whole page, you should use the address element, too.

<div id="extra">
    <section class="extra"></section>
    <section class="extra">
      <h1>Address</h1>
      <address>…</address>
    </section>
    <section class="extra"></section>
    <section class="extra"></section>
</div>
share|improve this answer

After writing html structure, I explicitly check for at least two things.

  • Are there any divs that can go away?
  • Can I remove any class or id?

Let's try to apply this to your code:

<header>
    <div id="logo"></div>
    <nav id="mainNav"></nav>    
</header>

You can replace <div id="logo"></div> whith <h1>Progress Business Company</h1>. Your CSS will hide the text anyway, but it will be visible for people that don't see the logo (images disabled or screen reader).

<div id="banner"></div>

This one seems OK, but keep my advices in mind when you feel it.

<div id="content">

What about <article>?

    <section id="advantages">
        <section class="advantage"></section>
        <section class="advantage"></section>
        <section class="advantage"></section>
        <section class="advantage"></section>
    </section>

You don't need all those class here. In CSS, #advantages section will select your subsections and will make explicit the relation between the big section and the subsections. This also frees your HTML from unnecessary clutter.

    <div id="main"></div>
    <aside></aside>

This applies to the rest of the HTML and could be the most important remark: the image you're trying to integrate has been designed with a grid in mind. See how the A, B, C, D subsections are laid out? How "Testimonials" aligns with D? And so on. Using a grid such as Bootstrap or 960 grid will help you a lot here.

<div id="extra">
    <article class="extra"></article>
    <article class="extra"></article>
    <article class="extra"></article>
    <article class="extra"></article>
</div>

Same issue than with "advantages".

<div id="credit"></div>

What about a paragraph (p) instead of a div?

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.