HiveBrain v1.2.0
Get Started
← Back to all entries
patternhtmlMinor

Removing unnecessary floats, height, widths

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unnecessaryremovingheightwidthsfloats

Problem

I have coded a webpage and it looks exactly how I want, but I think there could be improvements, possible as unnecessary floats, etc...

Could anybody please review my CSS code? Its not difficult or vague, I guess.

DEMO:
http://jsfiddle.net/dqC8t/2364/

CSS:

```
body{
background: url("http://i.imgur.com/cQhlsYZ.png") repeat-x;
}

h1{
font-size: 18px;
font-weight: bold;
color: #444;
}

h2{
font-size: 16px;
font-weight: bold;
color: #444;
}

#wrapper{
width: 980px;
margin: 0 auto;
}

/START HEAD CONTENT/
#header{
width: 100%;
float: left;
}

#headerLogo{
float: left;
margin-top: 20px;
margin-left: 20px;
margin-right: 70px;
}

#menu {
font-family: 'ProximaNova-Bold';
font-size: 16px;
margin-top:40px;
}

#menu ul {
list-style-type: none;
}
#menu li {
display: inline-block;
width: 150px;
border-bottom-style: solid;
border-bottom-width: 4px;
margin: -5px ;
padding: 0 15px 0 0 ;
}

.item-1 {
border-bottom-color: #0099CC;
}
.item-2 {
border-bottom-color: #FF4444;
}
.item-3 {
border-bottom-color: #669900;
}
.item-4 {
border-bottom-color: #FFBB33;
}

.item a {
text-decoration: none;
}

#headContent{
float: left;
margin-top: 20px
}

.hnItem{
width: 242px;
height: 184px;
float: left;
margin-right: 4px;
}

.hnItem img {
display: block;
}

#hnItemLast{
width: 242px;
height: 184px;
float: left;
}

#hnItemLast img{
display: block;
}

.hnTextContainer{
height: 40px;
padding: 10px 15px;
font-family: 'ProximaNova-Regular';
font-size: 14px;
line-height: 21px;
color: #c8cbcb;
background-image: linear-gradient(#262828,#1c1e1e);
}
/END HEAD CONTENT****/

/START MAIN CONTENT/
#mainContent{
width: 980px;
height: 800px;
float: left;
margin-top: 20px;
background-color: #FFF;
}

/START NEWS LIST CONTENT*/
#nlContainer{
width:660px;
float:left;
font-family: 'ProximaNova-Regular';
}

#nlContainer p{
font-size:14px;
}

#nlContainer a{
text-decoration: none;
}

#nlHeade

Solution

HTML:

  • For prototyping, you should include the # for links to actually trigger link behavior in browsers: Link



  • Instead of placing >> in your HTML, you should use…



  • If you page is not dynamically generated, you may consider using the CSS properties width and height instead of the HTML attributes



CSS:

-
…the :after pseudo element:

.rmLink:after {
    /* puts a space and two `>` after rmLink */
    content: "\00A0" "\003E" "\003E";
}


-
If you're not using some kind of CSS reset, you don't need to set font-weight: bold; on headings; All browsers should have User Agent Styles for them.

  • As already suggested, you should atleast provide sans-serif in your font-family declaration as a fallback, especially if you're not using web fonts (which you should consider)



  • When you define hex-based color values, keep in mind you can write #fff instead of #ffffff and so on; There are several occurences in your code where you could change that



Note:

You have a lot of similar margin declarations which can be simplified and stripped down. Think about where you rather should use padding on a parent instead of margin on several child elements.

Check out nlItem and nlTextContainer. You're repeating yourself.

Code Snippets

.rmLink:after {
    /* puts a space and two `>` after rmLink */
    content: "\00A0" "\003E" "\003E";
}

Context

StackExchange Code Review Q#39770, answer score: 5

Revisions (0)

No revisions yet.