Css code quality?

Question

I'm not a css expert so please help me figuring this out. Would you consider this as good css code?

Please point any issues if you believe this is not what you would exactly consider as good code.

                               html { -webkit-text-size-adjust: 70%;  }
  body {  margin:0px; padding:0px;font-family:Arial, Helvetica, sans-serif;  }
 { font-size:14px;}
 #wrap { width:100%; height:100%; background-color:#FFFFFF; padding: 0;}

  #header {
  padding: 10px;

  height:251px;
  }

  #menu { width:100%; height:2.2em; margin-top:1%;  background:#80b2e4; border-      radius:10px; -moz-border-radius:10px; -webkit-moz-border-radius:10px;}
 .top_menu_left {float:left; width:86%;height:35px; margin-left:3%;}
 .top_menu_left li a    { color:#fff !important; }
 .top_menu_left li a:hover  { color:#336699 !important; }

 .top_menu_right {float:right; width:17%;;height:1.2em;}
 li{list-style:none; float:left;  color:#74677E;font-size:70%;font-   weight:bold;font-     family:Arial, Helvetica, sans-serif; text-align:left; padding:0  5px;  -top:9px;  color:#fff;}

.top_menu_left li {padding-right:1%; color:#fff;}
.top_menu_right li {padding-right:5%; color:#fff;}

#searchbar { width:100%; height:auto;margin-top:8%;}

#top_search {width:100%; height:auto;}
#bottom_search {width:89%; height:auto;padding-left:10.5%;font-size:75%;}

.sarch_box { width:10%; height:auto; float:left; margin-left:1%;font-size:75%;}

.box_1 { width:20%; height:auto; float:left;font-size:85%;}
.box_1 input { width:80%; height:1.2em;font-size:85%;}
.box_1 select { width:70%; height:1.5em;font-size:85%;}

.box_2 {width:2%; height:auto; float:left;}

#content { width:87%; height:auto; margin-left:11%; margin-top:0; }
#left_content { width:7%; height:auto; float:left; }
#left_searchbar_title {width:95%;height:2%;padding-top:1%;padding-  bottom:1%;padding-     left:5%;background-color:#A5D959;font-family:Arial, Helvetica,   sans-serif;font-size:75%;}
#left_sidebar_content{width:100%;height:2%;background-color:#EFFFD8;font-  family:Arial,   Helvetica, sans-serif;font-size:75%;}

 #main_content { width:59%; height:auto;float:left;  }
.main_content_header  {margin-left:2%;color:black;font-size:100%;font- family:Arial, Helvetica, sans-serif;font-weight:bold;margin-bottom:5%;}

.content_header {width:70%; height:auto; margin-left:3%;}

 /*.left_1 {margin-left:2%;width:.05em; height:100px;background- image:url(images/verticalbar.gif);background-repeat:repeat-y;float:left;padding-left:2px;    }*/
 .left_1 {margin-left:2%;width:.05em; height:100px;background-image:none;background-   repeat:repeat-y;float:left;padding-left:2px; }
/*.left_6 { width:.05em;height:100px;background-   image:url(images/verticalbar.gif);background-repeat:repeat-y;float:left; }*/
.left_6 { width:.05em;height:100px;background-image:none;background-repeat:repeat-   y;float:left; }

.left_content_middle    {width:95%;float:left;font-size:80%;font-family:Arial, Helvetica, sans-serif; background:white; border:1px solid #c6c6c6;border-radius:10px; webkit-border-radius:10px; -moz-border-radius:10px}

.content_title {padding:2%; text-align:center; color:black; <!--#336699;-->font-  weight:bold;font-size:100%;font-family:Arial, Helvetica, sans-serif; }

.content_line{size:2px; width:95%; background-image:url(images/horizotal-line.gif);}

.left_2 { margin-left:3%; width:21.5%; height:auto; float:left; }
.left_3 { width:23.5%;height:auto;float:left;}
.left_4 { width:23.5%;height:auto;float:left;}
.left_5 { width:23.5%;height:auto;float:left;}


  #right_content { width:20%;height:auto;background-color:white;float:left;margin-   left:1%;padding:10px; border:1px solid #c6c6c6; border-radius:10px; webkit-border-   radius:10px; -moz-border-radius:10px}
 .right_top { width:100%; height:auto; color:white; font-size:100%; color:#336699;}

 .Left_portion { width:45%; height:auto; float:left;padding-left:8%;}

 .right_portion { width:45%; height:auto; float:left;}

 .right_content_title {color:#BB532E;font-weight:bold;font-size:70%;font-family:Arial,  Helvetica, sans-serif;padding-bottom:4%;padding-top:5%; }
 .right_content_body {color:#5E6472;font-size:70%;font-weight:bold;font-family:Arial, Helvetica, sans-serif;padding-bottom:3%;}



  .menu_bar { width:10%; font-family:Arial, Helvetica, sans-serif; color:red; float:left; font-size:50%;}

  .gap {width:35%;}

   #footer   {height:40px;width:100%;margin:auto;text-align:center;text-  align:center;margin-top:2%;background:url('images/footer_bg.png') repeat-x;    line-height:40px; color:white;}
  .footer_menu {
   color: white;
   float: right;
   height: 1.2em;
   -left: 3%;
   width: 70%;
   }
     .footer_menu li {
     color: white;
     padding-right: 1%;
  }
    .footer_menu li a   { color:#fff; text-decoration:none;}
    .footer_menu li a:hover { text-decoration:underline}
    .footer_menu2 {
    background: url("images/footer_bg2.png") repeat-x scroll 0 0 transparent;
    color: grey;
    float: left;
    font-size: 13px;
    margin-top: 21px;
    width: 100%;
    }
   .footer_menu strong  { font-weight:lighter; color:#666}

Looks pretty good to me, whole point of using a css file is to reduce redundancy, and you don't seem to have much. Maybe the .Left_i from 3-5, but other than that it looks pretty good. Really depends on what you are trying to get out of it. — Travis J
– Travis J, Commented Dec 28, 2011 at 22:37
What's the context? It looks like everything is relatively sized so the page will look different when the browser window is a different resolution. Can we see the HTML it's associated with? Also, is there a specific question you have about the css? "Good" and "bad" are kind of arbitrary right now — tedski
– tedski, Commented Dec 28, 2011 at 22:38
Others will add more specifics, but as with all code, grouping and commenting CSS will greatly aid in maintainability. — Ed Schembor
– Ed Schembor, Commented Dec 28, 2011 at 22:40
There is a lot of redundancy and overspecification in your stylesheet. Too many !important declarations, too many repeated selectors and properties... the one thing that you don't have enough of is indentation. — BoltClock
– BoltClock, Commented Dec 28, 2011 at 23:18

powerbuoy · Accepted Answer · 2011-12-28 23:11:29Z

html { -webkit-text-size-adjust: 70%;  }
body {  margin:0px; padding:0px;font-family:Arial, Helvetica, sans-serif;  }

There's no need to specify the unit (px) if the value is 0

{ font-size:14px;}

This isn't styling anything

#wrap { width:100%; height:100%; background-color:#FFFFFF; padding: 0;}

Assuming #wrap is a div or other block level element it's unnecesarry to give it a width of 100% as all block elements are 100% wide by default. Padding is also 0 by default in almost all elements.

#header {
padding: 10px;

height:251px;
}

You should never give elements (except images/canvases) a height because if the user increases the font-size the element should adjust to the new size of the content.

#menu { width:100%; height:2.2em; margin-top:1%;  background:#80b2e4; border-      radius:10px; -moz-border-radius:10px; -webkit-moz-border-radius:10px;}

Again, no need to set the width and why would you suddenly use em for the height? It's common to also put the -browser-specific properties before the "real" ones.

.top_menu_left {float:left; width:86%;height:35px; margin-left:3%;}
.top_menu_left li a    { color:#fff !important; }
.top_menu_left li a:hover  { color:#336699 !important; }

Having something like "left" (or right, bold, red, etc) as a class name is not separating the content from the presentation. Also you should try to avoid using !important as it often leads to more use of !important and hard to find bugs.

.top_menu_right {float:right; width:17%;;height:1.2em;}

Again the class name is design related.

li{list-style:none; float:left;  color:#74677E;font-size:70%;font-   weight:bold;font-     family:Arial, Helvetica, sans-serif; text-align:left; padding:0  5px;  -top:9px;  color:#fff;}

Do you really want all your li:s in your entire document to have this styling? There's also syntax errors (like -top).

.top_menu_left li {padding-right:1%; color:#fff;}
.top_menu_right li {padding-right:5%; color:#fff;}

Again the class name - also, this should probably be next to the other styling of the same element.

#searchbar { width:100%; height:auto;margin-top:8%;}

No need to set width or height here as you set them to default values. Also - why would you use an #id here but a .class for almost everything else?

I'll stop quoting code here as the rest contains pretty much the same faults.

A few things I would improve:

Don't use design related class or ID-names like left and right.
Don't use !important - instead plan your code in a way that doesn't require it.
Don't use percentages for everything.
Structure your CSS in a way that makes a little more sense. You don't have to keep it all in one file (in fact, you probably shouldn't (but you should only serve one file to the browser though)). This tip would probably require an entire book but a few things to get you started:
- Start with basic element selectors (like h1, p, a, ul etc) - the generic styling that will be default for everything on your page.
- Next do your layout - right/left columns and other areas for your "modules".
- Last style your modules - the #searchbar should have this bg, #recent-comments links should be red, the #footer's ul shouldn't have any bullets etc.

Also, since everything else in HTML uses dash-separation I use that for my class and ID names as well.

!important is very necessary when doing adaptive (or reactive) design. Dumb example: body article br {display: none} would mean there's no actual break when I use a <br> tag in within an <article> tag. But, lets say that I insert the <br> to cause a break on screen sizes less than 320px @media screen and (max-width: 320px){ body article br {display: block !important;}} without the !important that style will not display properly. !important exists for some very good reasons and should be used when appropriate since there are times when you can't "html around" it. — tkone
– tkone, Commented Dec 31, 2011 at 1:53
I still don't see why using !important is "bad" and why it should be avoided. ergo css-tricks.com/when-using-important-is-the-right-choice — tkone
– tkone, Commented Jan 3, 2012 at 15:42
Well, in the article you link to it's explained why it's "bad" in the first paragraph. I agree that it can be useful sometimes but very rarely. I've used it to to style .todo elements to always have a red dotted border. But as the name suggests the .todo class is very temporary and never makes it to the live site. — powerbuoy
– powerbuoy, Commented Jan 4, 2012 at 9:02
I agree that !important should only be used when absolutely necessary. It's often used in a lazy fashion, which causes hell for developers that come across this later. — danchet
– danchet, Commented Oct 10, 2012 at 17:40

Petah · Accepted Answer · 2011-12-28 22:43:13Z

1

I don't beleave it is "good" CSS code because:

It is inconsistant (white space, multi/single line, etc)
There are obvious syntax errors
There is a lot of repetition
It is mostly percent based sizes which will break in a lot of cases
There is rules that will be used by default anyway, hence don't need to be specified
There is too much use of the !important keyword

answered Dec 28, 2011 at 22:43

Petah

1113 bronze badges

Add a comment |

Stack Exchange Network

Css code quality?

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Css code quality?

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions