patternhtmlMinor
Removing unnecessary floats, height, widths
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
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:
CSS:
-
…the
-
If you're not using some kind of CSS reset, you don't need to set
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
- 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
widthandheightinstead 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-serifin yourfont-familydeclaration 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
#fffinstead of#ffffffand 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.