# Review for CSS layout code

I want to get my first CSS layout reviewed. First of all, the related HTML code is as follows -

<!DOCTYPE html>

<html lang="en">
<meta charset="utf-8" />
<title> Preferences </title>

<script src="options.js"></script>

<body>
<h1> Search in New Tab - Preferences </h1>
<form id="preference-form">
<label for="search-engine"> Search Engine </label>
<select id="search-engine">
<option id="bing" value="bing"> Bing </option>
<option id="yahoo" value="yahoo"> Yahoo! </option>
<option id="duckduckgo" value="duckduckgo"> DuckDuckGo </option>
<option id="wikipedia" value="wikipedia"> Wikipedia </option>
</select>

<br>

<label for="focus-search-tab"> Make the search tab active </label>
<input type="checkbox" value="focus-search-tab" id="focus-search-tab" />
</form>
</body>
</html>


I wrote the following CSS file in order to provide satisfactory presentation to the HTML -

h1
{
width : 40%;
background : #eeeeee;
box-shadow : 0 0 5px 5px #888;
margin-top : 10%;
margin-left : 30%;
margin-right : 30%;
text-align : center;
font-family : "Arial", "Century Gothic", "Lucida Sans Unicode";
}

form
{
width : 30%;
background : #eeeeee;
box-shadow: 0 0 5px 5px #888;
margin-top : 1%;
margin-left : 35%;
margin-right : 35%;
}

label
{
width : 60%;
display : inline-block;
text-align : right;
margin-top : 10px;
margin-bottom : 10px;
margin-right : 10px;
font-family : "Arial";
}


The resulting HTML document looks like this:

Basically, I wanted to avoid the use of float property. I felt that I could accomplish the same by using combinations of inline-block property, padding and margins. So, I used that. Was that the right way? Other than that, I would like to know if I did things right. Also, any improvements that make my CSS code better are welcome.

-
Get rid of the <br> –  Shmiddty Jan 4 '13 at 19:28

I'm no CSS expert, but two things I would change are:

• remove unnecessary spaces (indenting and in ' : ').
• condense explicit margin-top, margin-left etc (and padding):

h1{
width:40%;
background:#eee;
margin:10% 30% 0 30%;
text-align:center;
font-family:"Arial", "Century Gothic", "Lucida Sans Unicode";
}

form{
width:30%;
background:#eee;
box-shadow: 0 0 5px 5px #888;
margin:1% 35% 35%;
}

label{
width:60%;
display:inline-block;
text-align:right;
margin:10px 10px 10px;
font-family:"Arial";
}

-
+1 for condensing items. I have to disagree with the indenting of lines. I find it easier to read and find the different tags within the file with the white space. –  Jeff Vanzella Oct 23 '12 at 20:39
@JeffVanzella, yes it is true. Perhaps indent with a single TAB rather than many spaces (possibly the OP did that). –  William Morris Oct 23 '12 at 20:57
That would work, but in VS tab is replaced by spaces ;) –  Jeff Vanzella Oct 23 '12 at 21:22
@WilliamMorris yes. I am used to a tab indent rather than several spaces. Thanks for the condensation tip, though; it looks better, and saves a lot of lines. I will stick with indentation, though. –  Ghost Oct 24 '12 at 6:51
However, I was hoping for a review of the code that I have written for CSS - like if my usage of inline-block + margin + padding is better than taking the headache of using float, to achieve the same result. –  Ghost Oct 24 '12 at 6:53

I think the no fundamental difference using the "float:left;" and "display:inline-block;". In the first case, we have to clear the output before inserting the new elements that are not on the floated blocks. In the second case we make of inline element inline-block.

Here's another variant:

<!DOCTYPE html>

<html lang="en">
<meta charset="utf-8">
<title> Preferences </title>

<body>
<h1> Search in New Tab - Preferences </h1>
<br>
<form id="preference-form">
<label> Search Engine
<select>
<option id="bing" value="bing"> Bing </option>
<option id="yahoo" value="yahoo"> Yahoo! </option>
<option id="duckduckgo" value="duckduckgo"> DuckDuckGo </option>
<option id="wikipedia" value="wikipedia"> Wikipedia </option>
</select>
</label>

<label> Make the search tab active
<input type="checkbox" value="focus-search-tab" id="focus-search-tab">
</label>
</form>
<script src="options.js"></script>
</body>
</html>​


Styles:

body { text-align: center; }
h1, form {
display: inline-block;
background: #eee;
box-shadow: 0 0 5px 5px #888;
font-family: Arial,Helvetica,Garuda,sans-serif;
}

h1 {
margin: 10% auto 1%;
text-align: center;
}

form {
margin: 1% auto;

1. The <br> element "br elements must be used only for line breaks that are actually part of the content, as in poems or addresses"., and so should not be used to send the label to the new line. 2. USing overflow: hidden; in the container is curious, I had not thought of that and cannot find any flaws in it. –  ANeves Nov 15 '12 at 12:50
What about the element <br> - I agree. For more semantics then it is logical to make the form elements out of <p> tag or wrap them in a separate <div>. Because they are not part of paragraph. –  Ademaro Nov 15 '12 at 14:09