Improve HTML structure?

Question

I am not new to html however would like to know about best practices. I need to develop a layout like this:

This is what I have proposed:

<div id="body">
    <div id="body-main">
        <div id="body-main-upper">
            <div id="body-main-upper-image">
                <image></image>
            </div>
            <div id="body-main-upper-image2">
                Text goes here
                <image></image>
            </div>
        </div>
        <div id="body-main-bottom">
            <Table><tr /><td /></Table>
        </div>
    </div>
    <div id="sidebar">
        <div id="sidebar-upper">
            <image></image>
        </div>
        <div id="sidebar-bottom">
            <image></image>
            <div id="sidebar-bottom-table">
            </div>
            Text goes here
            <image></image>
        </div>
    </div>
</div>

http://jsfiddle.net/gdWM5/

Can this be improved in any way?

Answer 1

your Table is not declared correctly in the markup.

This:

<div id="body-main-bottom">
    <Table><tr /><td /></Table>
</div>

Should look like this

<div id="body-main-bottom">
    <table>
        <tr>
            <td></td>
            <td></td>
        </tr>
    </table>
</div>

your self closing tag <tr /> means that the row is now closed, you shouldn't have a <td> outside of a row, it is bad structure and won't show correctly in browsers.

---

HTML is not made up of <div>'s alone. you should include other elements as well, even when you are only figuring out your structure.

Image tags are not <image> they are <img /> and as long as you have everything you need inside the attributes, they are self closing tags.

from the looks of the Graphic, you may not need a table, you probably want a list instead of a table. If I were you I would look into an unordered list (you can format how you like with CSS)

---

Element tags do not have Capital letters in them at all, they are Lowercase.

---

More Descriptive ID names. If you have something that is going to be styled the same as something else give them class names and declare the style once. if you have one thing that is going to be different, create a class that will style it and the surrounding items the the same and add another class to it and declare it after the other class.

---

In Closing

I would say that you should figure out your CSS and then post a new question as well (with the CSS)

this is not a finished structure, it looks like something you threw together in about a half an hour. probably took you longer to draw the picture than write the HTML

Answer 2

I have three main remarks:

1. The markup is very heavily tied to the layout, in terms of naming. You will be able to write better CSS if you name elements for their purpose rather than their position. That is, avoid positioning terms such as "upper" and "bottom" when naming your element IDs, and prefer terms that describe the content, such as "header" and "featured-news".
2. Consider introducing HTML5 elements, such as <nav> for your sidebar, if appropriate.
3. You may have a hard time implementing the CSS to put the #sidebar to the right of the #body-main, and it may be easier if you switch the order in which they appear in the HTML.

Paul O'Brien has a useful all-purpose template for creating a page with a header, footer, and left and right sidebars. Most pages are just a special case of that generic solution; you want just the right sidebar. Here is how you can apply the technique…

HTML

<div id="body">
    <nav id="sidebar">
        <div id="sidebar-upper">
            <img src="https://cdn.sstatic.net/stackexchange/img/logos/se/se-logo.png" width="125" height="37">
        </div>
        <div id="sidebar-bottom">
            <Table border="1">
                <tr><th>Table row</th></tr>
                <tr><td>Table row</td></tr>
            </Table>

            <div id="sidebar-bottom-table">
            </div>
            Text goes here
            <img src="https://cdn.sstatic.net/stackexchange/img/logos/se/se-icon.png" width="40" height="40">
        </div>
    </nav>
    <div id="body-main">
        <div id="body-main-upper-image2">
                Body upper image 2
                <img src="https://cdn.sstatic.net/stackexchange/img/logos/so/so-icon.png" width="40" height="40">
         </div>
        <div id="body-main-upper">
            <div id="body-main-upper-image">
                <img src="https://cdn.sstatic.net/stackexchange/img/logos/so/so-logo.png" width="125" height="38">
            </div>
        </div>
        <div id="body-main-bottom">
            <Table border="1">
                <tr><th>Table row</th></tr>
                <tr><td>Table row</td></tr>
            </Table>
        </div>
    </div>
</div>

CSS

#body-main {
    border: 1px solid red;
    margin-right: 30%;
}
#body-main-upper {
    border: 1px solid pink;
}
#body-main-upper {
    border: 1px solid brown;
}
#sidebar {
    border: 1px solid blue;
    position: relative;
    float: right;
    width: 30%;
}
#sidebar-upper {
    border: 1px solid lightblue;
}
#sidebar-bottom {
    border: 1px solid navy;
}
#body-main-upper-image2 {
    float: right;
    width: 50%;
}

Implementation Notes

As I noted, the #sidebar comes before #body-main; the sidebar lives in the void created by the right margin of the main div. You may also have noticed that the #body-main-upper-image2 comes before #body-main-upper; the former is floated to the right in CSS. I encourage you to experiment with the jsFiddle to see how that works.

Answer 3

You might consider adding classes. You have many broadly similar elements on that page; adding classes to them would give you more flexibility in your CSS. As things stand, you can only apply style to elements based on their id, their parent elements or their type (and hey, they're all DIVs).

I would use an identifier other than body, for clarity and to avoid any possibility of error/confusion.

Answer 4

I am going to assume your table rows are for a real table, i.e. they're full of data itemized to present to the user. If this is not the case, do not use tables. Only use tables for layout if you are a real cretin that wants to exclude visually impaired persons with screen readers from using your product.

I would look over all of your divs and see if there's a more sound tag for them. div implies a logical division with no semantic significance, so if you're using them just to divide your page visually, there is probably a better element to represent the content that can be visually blocked the way you want with CSS. The reason I suspect this to be the case is that your HTML contains only div, image, and table-related elements. Unless the web app is truly something the HTML standards committee has just never thought of, either the tables, divs, or both are being used inappropriately for layout.

Previous reviewers have mentioned replacing some of your IDs with classes and/or changing the names that include "body", as well as unwrapping some more meaningful elements from their enclosing divs. These are all good ideas. I'm not certain that the rationale has been stated, though.

id attributes are a good idea in the broadest semantic sense when the element serves a role that is logically necessarily unique among all elements. They are additionally necessary to target anchors and label elements. Long long ago, they were the most logical way to make an element accessible from JS, but preemptively iding almost every element in case it's needed in JS creates a maintenance hazard that can now be avoided. If it's conceivable that there could be more than one sidebar (there are two sides after all) in some future revision, for example, adding it will involve more work and possible catastrophic name collisions if you're identifying everything with IDs. You'll basically start defining what a sidebar is and should look like from scratch in the page's CSS and JS. Then you have to maintain that code, too. The class attribute allows you to assign similar styles and behaviors to the elements that share them. If two elements share any stylistic or functional aspect, they should share a class.

Unwrapping elements from their parent div is an extension of using more appropriate tags and abandoning the "everything needs an id" approach because there are many divs which do literally nothing but possess an id and contain a more meaningful element. If the element inside needs to be identified, give it an id or class attribute as appropriate. The div wrapping it just makes it harder to revise or transplant without breaking CSS rules and layout.