My first stab at this

1,392 views
Skip to first unread message

Walt

unread,
Sep 29, 2010, 11:29:39 AM9/29/10
to Object Oriented CSS
OK, maybe this will sound like a stupid question, but I would
appreciate the feedback. Here's my first stab at OOCSS - http://66.94.67.189/v2
- links aren't active, just one page.

So the question is, how does the CSS look? Since this is the first
time working with it, I don't want to assume that I did it the "right"
way. I know there's no single correct way to do anything, but I want
to make sure I'm still within the spirit of OOCSS.

Walt

Nicole Sullivan

unread,
Sep 29, 2010, 4:04:58 PM9/29/10
to object-oriented-css
Hi Walt,

I was wondering how brave you are feeling? ;)

If I do a code review, can I use it as an example on the website?

Cheers,
Nicole


--
You received this message because you are subscribed to the Google Groups "Object Oriented CSS" group.
To post to this group, send email to object-or...@googlegroups.com.
To unsubscribe from this group, send email to object-oriented...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/object-oriented-css?hl=en.


erik.f...@googlemail.com

unread,
Sep 29, 2010, 4:46:11 PM9/29/10
to Object Oriented CSS
Hi Walt,

I just did a quick scan of your CSS Code, no thorough review, but some
things I did notice (and just as an aside, my findings are based on my
interpretation(!) of OOCSS and how to write good/clean CSS):

- you have a class blkbrd (which is a 1px black border). I think the
name blkbrd is just a shorthand for what your CSS does. What are you
going to do if you decide that it should be a blue border? Having a
class blkbrd which actually makes a blue border would be confusing
indeed.
- same goes for your fontsize classes (e.g. .f125). Again, I get the
idea, but as soon as you change the font sizes just slightly your
naming convention will fall.
- You give a better example of how if should be done as well. The
class name (thintop) is not optimal (because again it implies how the
result of your class should look) but its not as specific as blkbrd.
- you seem to be a fan of descriptive class names..I just found "p15",
which gives a 15px padding. I won't go into detail with every finding,
you get the idea. Try to prevent using classnames that hint at how the
element should look. Use more generic classnames (like "sommers" or
"glow" from OOCSS).
-just found this selector: ".nav li a.current". I bet you only ever
assign "current" to the anchor tags, no to the list items, so
".nav .current" would do
- by the by, why are you using a .nav wrapper element anyways? You
only set the margin, which could easily be done using only the ul.
Were you using HTML5, you would be using the <nav> element, but since
you are not, you could just drop the wrapping div and assign the
".nav" class to the <ul> directly
- all your headings are bold, but you repeat the declaration for every
selector. Using "h1, .h1, h2, .h2, h3, ..., .h5 {font-weight: bold}"
would do the trick and is in my opinion a little cleaner when setting
defaults.
- I see that you are setting "h1, h2, h3, h4, h5, h6, ul, ol, dl, p,
blockquote" to "padding: 10px"...but then on most elements you take
the top and the left padding away again. In that case you should think
about whether or not setting the padding to 10px on all sides for all
those elements really is a sensible default.
- ".callout span.quot"...is the "span" really necessary in that
selector? It's one more element that needs to be validated before the
browser can apply the style....
- ".media .img img"...quite a funny selector, don't you think? ;)

Scanning your HTML:

- found a div that has those classes: "unit size1of3 mr30"...where
mr30 gives a 30px margin to the right. I think using a pixel margin on
an element that is part of the OOCSS grid (size1of3) is very
dangerous. Sometimes it won't even work (as you probably figured out
by yourself...you have a div with "unit size2of3 lastUnit mr30", in
which case the mr30 has no effect at all). I would try to stay away
from assigning anything but the width to the grid elements, and rather
set the margin or padding to the elements inside that grid element.
- one thing CSS unrelated...beneath your "Daily Headlines" (although
not just there) you are listing a lot of subheadlines; have you
thought about using an ordered list for that? In effect this is what
you are doing...
- in your head you are using the class "unit size1of6"....size1of6 is
not defined anywhere ;)
- what are your reasons for using display: inline on the <li> elements
for your navigation? Problem with that is that your <li> elements are
basically outside the <ul> box, which will make the styling a bit odd.
Using float would work just as well...but I guess that's just personal
preference...
- I was about to congratulate you for your use of the standard module
format (huge fan right here)...then I realized that you are using CSS3
for "mod littlered"'s rounded corners ;) If you are going to use CSS3
with no IE fallback, using the standard module format will probably be
overweight (in my experience). I would however make perfect sense if
you included the rounded corners per image only in the IE
stylesheet...did you do that? I didn't check, but if you did, I owe
you a brownie. (matter of fact, I just checked and you don't have an
IE stylesheet ;) But using the mod format for IE only and CSS3 for the
rest would make perfect sense. I strongly recommend SASS and Compass
to you, maintaining separate stylesheets is amazingly simple with
those).

Anyway, I hope that was of some help. Your CSS all in all looks pretty
lean so far, I have seen far worse ;)

On Sep 29, 5:29 pm, Walt <w...@schmidty.com> wrote:
> OK, maybe this will sound like a stupid question, but I would
> appreciate the feedback. Here's my first stab at OOCSS -http://66.94.67.189/v2

Walt

unread,
Sep 30, 2010, 11:35:31 AM9/30/10
to Object Oriented CSS
The chance to improve my technique far outweighs the potential blow to
my self-esteem. :) So please feel free to use it as an example.

I moved the page to http://waltschmidt.com/v2 and made the content
generic. I should have done that in the first place, since I shouldn't
be displaying it without approval at such an early stage.



On Sep 29, 4:04 pm, Nicole Sullivan <nic...@stubbornella.org> wrote:
> Hi Walt,
>
> I was wondering how brave you are feeling? ;)
>
> If I do a code review, can I use it as an example on the website?
>
> Cheers,
> Nicole
>
> On Wed, Sep 29, 2010 at 8:29 AM, Walt <w...@schmidty.com> wrote:
> > OK, maybe this will sound like a stupid question, but I would
> > appreciate the feedback. Here's my first stab at OOCSS -
> >http://66.94.67.189/v2
> > - links aren't active, just one page.
>
> > So the question is, how does the CSS look? Since this is the first
> > time working with it, I don't want to assume that I did it the "right"
> > way. I know there's no single correct way to do anything, but I want
> > to make sure I'm still within the spirit of OOCSS.
>
> > Walt
>
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Object Oriented CSS" group.
> > To post to this group, send email to object-or...@googlegroups.com.
> > To unsubscribe from this group, send email to
> > object-oriented...@googlegroups.com<object-oriented-css%2Bunsu...@googlegroups.com>
> > .

Walt

unread,
Sep 30, 2010, 1:00:48 PM9/30/10
to Object Oriented CSS
Thanks for the feedback, Erik. Some of the stuff you mentioned come
from bad habits that I need to work through.

Looking through it again, I can see where I need to spend more time
thinking about defaults. My original thought was to extend the object
if I needed something like padding. But that quickly got out of hand.

On Sep 29, 4:46 pm, "erik.fris...@googlemail.com"

Nicole Sullivan

unread,
Sep 30, 2010, 2:32:20 PM9/30/10
to object-oriented-css
Erik,

Wow. Awesome!

(I must admit, I get really excited when I see you helping each other figure stuff out)

Walt,

Overall, great job. The CSS is pretty lean.

You also made some improvements to the code in the second version. :)

I like it that all the media blocks going down the center of the page are now media blocks (the display works much more consistently than in the first version).

--------

For horizontal dividers, I generally take two approaches:

  1. If it is a list of items, I make a UL or OL which automatically gets the divider *after* each item. I then use the list itself to add a dividing line at the top, if required. You can then use :last-child to remove the last dividing line if you don't want it. This won't work for IE, but generally the bottom of the list will be below the fold, so I find this an acceptable fall back. If you really want absolutely the same display for IE, you can apply a class "lastItem" to the last LI. A good example of a list divider is "thintop".
  2. If it really is just dividing space between two areas of the page, I use an HR. Crazy, right? But it means that my divider isn't dependent on whatever other styles have been applied around it, the way your "thintop" would be. A good example of and HR divider is "dashbot".

--------

I'm totally in agreement with Erik that the presentation based classes have got to go. Clients will change their minds. It is too dangerous to end up with a "littlered" that is actually giant and blue. (I've *so* had that happen, which is why I'm so emphatic about it). Choose classes that describe how it should be used or abstract classes or classes that represent relative values.

For example, you don't want to say "mr30", but you might set up spacing classes that apply none, small, medium, and large spacing, because those values stay correct relative to one another even if the pixel value changes.

I released a spacing classes recently: http://github.com/stubbornella/oocss/blob/master/core/spacing/space.css

-------

Don't feel like you have to keep stuff you aren't using. Feel free to delete skins that aren't relevant to the current design. Skins especially are more of an example than something you will use on your own site.

-------

Also agree with Erik, if you aren't putting rounded corners on IE6-8 (which is a fine choice, and probably much better for these users), you won't need the fancy module code. OTOH, some borders can't be replicated with CSS3 borders, rounding, and shadows. In that case, you'll still need the module code. e.g. different sized borders on different sides of the box makes border-radius go nuts in safari. Try to build "grab" in CSS3.

-------

You have an old version of the code for the media block (with an error! my bad). Latest is released as it's own object: http://github.com/stubbornella/oocss/tree/master/core/media/

-------

I'd recommend making a library page with all your base objects on it (as a plus, this serves as documentation and a fab way to test any errors). Then start building pages out of it. The goal is to make the library so comprehensive that you are building pages by modifying HTML-only.


Cheers,
Nicole


To unsubscribe from this group, send email to object-oriented...@googlegroups.com.

Nicole Sullivan

unread,
Sep 30, 2010, 2:36:42 PM9/30/10
to object-oriented-css
Walt,

One more thing. :)

In production, you should combine all the CSS files into one (or maybe two).

Cheers,
Nicole


On Thu, Sep 30, 2010 at 11:34 AM, Nicole Sullivan <nic...@stubbornella.org> wrote:
Erik,

Any chance you want to add this to the wiki? You really did a great job with the code review and I'd love to see others benefit from it.

Nicole Sullivan

unread,
Sep 30, 2010, 2:34:56 PM9/30/10
to object-oriented-css
Erik,

Any chance you want to add this to the wiki? You really did a great job with the code review and I'd love to see others benefit from it.

Nicole



On Thu, Sep 30, 2010 at 11:32 AM, Nicole Sullivan <nic...@stubbornella.org> wrote:

erik.f...@googlemail.com

unread,
Sep 30, 2010, 5:10:38 PM9/30/10
to Object Oriented CSS
Hi Nicole,

sure...I would be happy to. I will post it up over the weekend and let
you know.

Erik

On Sep 30, 8:34 pm, Nicole Sullivan <nic...@stubbornella.org> wrote:
> Erik,
>
> Any chance you want to add this to the wiki? You really did a great job with
> the code review and I'd love to see others benefit from it.
>
> Nicole
>
> On Thu, Sep 30, 2010 at 11:32 AM, Nicole Sullivan
> <nic...@stubbornella.org>wrote:
>
>
>
> > Erik,
>
> > Wow. Awesome!
>
> > (I must admit, I get really excited when I see you helping each other
> > figure stuff out)
>
> > Walt,
>
> > Overall, great job. The CSS is pretty lean.
>
> > You also made some improvements to the code in the second version. :)
>
> > I like it that all the *media blocks* going down the center of the page
> > are now media blocks (the display works much more consistently than in the
> > first version).
>
> > --------
>
> > For *horizontal dividers*, I generally take two approaches:
>
> >    1. If it is a list of items, I make a UL or OL which automatically gets
> >    the divider *after* each item. I then use the list itself to add a dividing
> >    line at the top, if required. You can then use :last-child to remove the
> >    last dividing line if you don't want it. This won't work for IE, but
> >    generally the bottom of the list will be below the fold, so I find this an
> >    acceptable fall back. If you really want absolutely the same display for IE,
> >    you can apply a class "lastItem" to the last LI. A good example of a list
> >    divider is "thintop".
> >    2. If it really is just dividing space between two areas of the page, I
> >    use an HR. Crazy, right? But it means that my divider isn't dependent on
> >    whatever other styles have been applied around it, the way your "thintop"
> >    would be. A good example of and HR divider is "dashbot".
>
> > --------
>
> > I'm totally in agreement with Erik that the *presentation based classes*have got to go. Clients will change their minds. It is too dangerous to end
> > up with a "littlered" that is actually giant and blue. (I've *so* had that
> > happen, which is why I'm so emphatic about it). Choose classes that describe
> > how it should be used or abstract classes or classes that represent relative
> > values.
>
> > For example, you don't want to say "mr30", but you might set up spacing
> > classes that apply none, small, medium, and large spacing, because those
> > values stay correct relative to one another even if the pixel value changes.
>
> > I released a *spacing classes* recently:
> >http://github.com/stubbornella/oocss/blob/master/core/spacing/space.css
>
> > -------
>
> > Don't feel like you have to keep stuff you aren't using. Feel free to *delete
> > skins that aren't relevant to the current design*. Skins especially are
> > more of an example than something you will use on your own site.
>
> > -------
>
> > Also agree with Erik, if you aren't putting *rounded corners* on IE6-8
> > (which is a fine choice, and probably much better for these users), you
> > won't need the fancy module code. OTOH, some borders can't be replicated
> > with CSS3 borders, rounding, and shadows. In that case, you'll still need
> > the module code. e.g. different sized borders on different sides of the box
> > makes border-radius go nuts in safari. Try to build "grab" in CSS3.
>
> > -------
>
> > You have an old version of the code for the media block (with an error! my
> > bad). Latest is released as it's own object:
> >http://github.com/stubbornella/oocss/tree/master/core/media/
>
> > -------
>
> > I'd recommend making a library page with all your base objects on it (as a
> > plus, this serves as documentation and a fab way to test any errors). Then
> > start building pages out of it. The goal is to make the library so
> > comprehensive that you are building pages by modifying HTML-only.
>
> > Cheers,
> > Nicole
>
> > On Thu, Sep 30, 2010 at 8:35 AM, Walt <w...@schmidty.com> wrote:
>
> >> The chance to improve my technique far outweighs the potential blow to
> >> my self-esteem. :)  So please feel free to use it as an example.
>
> >> I moved the page tohttp://waltschmidt.com/v2and made the content
> >> > > object-oriented...@googlegroups.com<object-oriented-css%2Bunsu bsc...@googlegroups.com>
> >> <object-oriented-css%2Bunsu...@googlegroups.com<object-oriented-css%252 Bunsub...@googlegroups.com>
>
> >> > > .
> >> > > For more options, visit this group at
> >> > >http://groups.google.com/group/object-oriented-css?hl=en.
>
> >> --
> >> You received this message because you are subscribed to the Google Groups
> >> "Object Oriented CSS" group.
> >> To post to this group, send email to object-or...@googlegroups.com
> >> .
> >> To unsubscribe from this group, send email to
> >> object-oriented...@googlegroups.com<object-oriented-css%2Bunsu bsc...@googlegroups.com>

erik.f...@googlemail.com

unread,
Oct 3, 2010, 9:55:48 AM10/3/10
to Object Oriented CSS
Hi Walt,

I added this thread to the OOCSS wiki (http://github.com/stubbornella/
oocss/wiki/FAQ - at the very bottom).

Erik

On Sep 30, 8:34 pm, Nicole Sullivan <nic...@stubbornella.org> wrote:
> Erik,
>
> Any chance you want to add this to the wiki? You really did a great job with
> the code review and I'd love to see others benefit from it.
>
> Nicole
>
> On Thu, Sep 30, 2010 at 11:32 AM, Nicole Sullivan
> <nic...@stubbornella.org>wrote:
>
> > Erik,
>
> > Wow. Awesome!
>
> > (I must admit, I get really excited when I see you helping each other
> > figure stuff out)
>
> > Walt,
>
> > Overall, great job. The CSS is pretty lean.
>
> > You also made some improvements to the code in the second version. :)
>
> > I like it that all the *media blocks* going down the center of the page
> > are now media blocks (the display works much more consistently than in the
> > first version).
>
> > --------
>
> > For *horizontal dividers*, I generally take two approaches:
>
> >    1. If it is a list of items, I make a UL or OL which automatically gets
> >    the divider *after* each item. I then use the list itself to add a dividing
> >    line at the top, if required. You can then use :last-child to remove the
> >    last dividing line if you don't want it. This won't work for IE, but
> >    generally the bottom of the list will be below the fold, so I find this an
> >    acceptable fall back. If you really want absolutely the same display for IE,
> >    you can apply a class "lastItem" to the last LI. A good example of a list
> >    divider is "thintop".
> >    2. If it really is just dividing space between two areas of the page, I
> >    use an HR. Crazy, right? But it means that my divider isn't dependent on
> >    whatever other styles have been applied around it, the way your "thintop"
> >    would be. A good example of and HR divider is "dashbot".
>
> > --------
>
> > I'm totally in agreement with Erik that the *presentation based classes*have got to go. Clients will change their minds. It is too dangerous to end
> > up with a "littlered" that is actually giant and blue. (I've *so* had that
> > happen, which is why I'm so emphatic about it). Choose classes that describe
> > how it should be used or abstract classes or classes that represent relative
> > values.
>
> > For example, you don't want to say "mr30", but you might set up spacing
> > classes that apply none, small, medium, and large spacing, because those
> > values stay correct relative to one another even if the pixel value changes.
>
> > I released a *spacing classes* recently:
> >http://github.com/stubbornella/oocss/blob/master/core/spacing/space.css
>
> > -------
>
> > Don't feel like you have to keep stuff you aren't using. Feel free to *delete
> > skins that aren't relevant to the current design*. Skins especially are
> > more of an example than something you will use on your own site.
>
> > -------
>
> > Also agree with Erik, if you aren't putting *rounded corners* on IE6-8
> > (which is a fine choice, and probably much better for these users), you
> > won't need the fancy module code. OTOH, some borders can't be replicated
> > with CSS3 borders, rounding, and shadows. In that case, you'll still need
> > the module code. e.g. different sized borders on different sides of the box
> > makes border-radius go nuts in safari. Try to build "grab" in CSS3.
>
> > -------
>
> > You have an old version of the code for the media block (with an error! my
> > bad). Latest is released as it's own object:
> >http://github.com/stubbornella/oocss/tree/master/core/media/
>
> > -------
>
> > I'd recommend making a library page with all your base objects on it (as a
> > plus, this serves as documentation and a fab way to test any errors). Then
> > start building pages out of it. The goal is to make the library so
> > comprehensive that you are building pages by modifying HTML-only.
>
> > Cheers,
> > Nicole
>
> > On Thu, Sep 30, 2010 at 8:35 AM, Walt <w...@schmidty.com> wrote:
>
> >> The chance to improve my technique far outweighs the potential blow to
> >> my self-esteem. :)  So please feel free to use it as an example.
>
> >> I moved the page tohttp://waltschmidt.com/v2and made the content
> >> <object-oriented-css%2Bunsu...@googlegroups.com<object-oriented-css%252Buns...@googlegroups.com>

Walt

unread,
Oct 3, 2010, 3:00:00 PM10/3/10
to Object Oriented CSS
Thanks for the help, Nicole and Erik. I'm working back through the
code now, but I'll leave /v2 up there as is. Otherwise, it probably
won't make much sense. :)


On Oct 3, 9:55 am, "erik.fris...@googlemail.com"
> > >> I moved the page tohttp://waltschmidt.com/v2andmade the content
Reply all
Reply to author
Forward
0 new messages