search

somerandomdude / Iconic

A minimal set of icons in raster, vector and font formats — free for public use.
http://useiconic.com/open/
Other
2.37k stars 349 forks source link

Consider namespacing css classes? #18

Closed ghost closed 11 years ago

ghost commented 12 years ago

Very common class names can cause unintended content insertions at times. For instance, the Wordpress template function body_class() inserts a .home class on the homepage's body tag. Using the Iconic font as is will insert the designated character before any body content.

Maybe it might be a good idea to declare icon classes as something like like .icon-home, or .iconic.home (has it's own "old IE" problems, though)? It's easy enough to change the classes yourself or even override any problems body.home:before {content: ""}. But, I thought I'd bring it up in case it hadn't been considered yet.

borisrorsvort commented 12 years ago

+1

somerandomdude commented 11 years ago

I will be working on this soon. Apologies for the radio silence.

somerandomdude commented 11 years ago

Great call on this guys. The latest commit addresses this issue.

bryceadams commented 11 years ago

Didn't want to open a new issue for this but I was thinking why not just change it so instead of: .book_alt2:before {content:'\e049';} or .iconic-book_alt2:before {content:'\e049';} it be made: .iconic.book_alt2:before {content:'\e049';} as the .iconic class is required regardless.

Any thoughts on this? I personally find it to be much more useful.

somerandomdude commented 11 years ago

Yeah, that makes good sense. I'll see if I can fix that in the coming days.

On Oct 11, 2012, at 12:18 AM, Bryce Adams notifications@github.com wrote:

Didn't want to open a new issue for this but I was thinking why not just change it so instead of: .book_alt2:before {content:'\e049';} or .iconic-book_alt2:before {content:'\e049';} it be made: .iconic.book_alt2:before {content:'\e049';} as the .iconic class is required regardless.

Any thoughts on this? I personally find it to be much more useful.

— Reply to this email directly or view it on GitHub.

bryceadams commented 11 years ago

If you'd like I can make a pull request with the changes already made?

Bryce Adams

On Friday, 12 October 2012 at 4:35 PM, P.J. Onori wrote:

Yeah, that makes good sense. I'll see if I can fix that in the coming days.

On Oct 11, 2012, at 12:18 AM, Bryce Adams <notifications@github.com (mailto:notifications@github.com)> wrote:

Didn't want to open a new issue for this but I was thinking why not just change it so instead of:
.book_alt2:before {content:'\e049';} or .iconic-book_alt2:before {content:'\e049';}
it be made:
.iconic.book_alt2:before {content:'\e049';}
as the .iconic class is required regardless.

Any thoughts on this? I personally find it to be much more useful.

—
Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub (https://github.com/somerandomdude/Iconic/issues/18#issuecomment-9367083).

somerandomdude commented 11 years ago

Well, I'd like to change the scripts that generate the CSS. If you'd be up for making those changes as well, I wouldn't stand in your way. :)

On Oct 11, 2012, at 12:18 AM, Bryce Adams notifications@github.com wrote:

Didn't want to open a new issue for this but I was thinking why not just change it so instead of: .book_alt2:before {content:'\e049';} or .iconic-book_alt2:before {content:'\e049';} it be made: .iconic.book_alt2:before {content:'\e049';} as the .iconic class is required regardless.

Any thoughts on this? I personally find it to be much more useful.

— Reply to this email directly or view it on GitHub.

bryceadams commented 11 years ago

Probably best you do it then :)