CSS critique needed

[santa]

I received a CSS file form client. It just does not look that clean to me, and that's what I told a Project Manager.

I would like you to take a look at the first few styles and give me your brutally honest opinion on it too:


#html, body {
top: 0px;
right: 0px;
bottom: 0px;
left: 0px;
width:1024px;
height: 100%;
margin: auto;
background-color:#ffffff;
font-family:arial;
//border:thin dashed #000000;
}

.ui-tabs .ui-tabs-hide {
display: none;
}

#header {
position:relative;
top:20px;
left:0px;
width:1024px;
height:80px;
background-color:#004b85;
}

div#header div#header_logo {
position:absolute;
top:10px;
left:10px;
height:68px;
width:320px;
background:url('../sprite.png') 0 -293px;
}

#footer {
position:absolute;
width:1024px;
height:20px;
margin-top:30px;
}

[skywalker2208]

Personally I don't like to see the use of top, left, right and position unless there is a reason for it. I don't like position things on the page with those css attributes unless there is a reason for it.

[santa]

Great, thanks. I appreciate your insight.
Anyone notices anything else?

[skywalker2208]

Great, thanks. I appreciate your insight.
Anyone notices anything else?

What exactly don't you think looks clean?

[Fang]

#html, body {
top: 0px; /* does nothing without position */
right: 0px; /* does nothing without position */
bottom: 0px; /* does nothing without position */
left: 0px; /* does nothing without position */
width:1024px;
height: 100%;
margin: auto; /* won't center in IE6 */
background-color:#fffff; /* shorten #fff */
font-family:arial; /* should also give a generic font*/
//border:thin dashed #000000; /* '//' won't hide border in IE. thin different in browsers, use 1px. shorten #000 */
}

.ui-tabs .ui-tabs-hide {
display: none;
}

#header {
position:relative;
top:20px;
left:0px;
width:1024px; /* unnecessary same width as body, #header is probably a block element */
height:80px;
background-color:#004b85; /* not a safe color */
}

div#header div#header_logo { /* same as #header_logo, unless there are specificity problems*/
position:absolute;
top:10px;
left:10px;
height:68px;
width:320px;
background:url('../sprite.png') 0 -293px; /* no-repeat ? */
}

#footer {
position:absolute;
width:1024px; /* same as #header */
height:20px;
margin-top:30px;
}

The use of position:absolute/relative creates more problems than it solves. Leave the elements in the flow and use margin and padding to adjust their positions.

[santa]

How many BODY tags are in a document. Why call it

#html, body { and not body {

What's up with the width and height in BODY? Can CSS resize the browser? Ir car the body be less then 100% high?

What exactly is "//" in front of the property? Was it meant as a comment? Is this how we comment in CSS?
Why put margin: auto; /* won't center in IE6 */ if you know it won't center in IE6? Can't you do


margin-left: auto;
margin-right: auto;

if you need it in the center?

Clearly there's an attempt to design for 1024x768? What if we get a scroll bar? Shouldn't it be 968 instead?

I think I could go on but would like to hear others.

[Fang]

How many BODY tags are in a document. Why call itOne body element.
The #html is an id
What exactly is "//" in front of the property? Was it meant as a comment? Is this how we comment in CSS?"//" is incorrect commenting. /* this is a comment */
Why put margin: auto;This centers in all browsers except IE6. To center in IE6; text-align:center in the body, then text-align:left in child elements. Personally I think we should not support IE6 anymore.
Shouldn't it be 968 instead?Why?