Closed notacouch closed 9 years ago
I agree that it's not the most elegant solution and definitely will collide when mixing font icons.
@notacouch
With latest version can you try with [class^="icon-"]
or [class*="icon-"]
and confirm if it's working.
@thesabbir I still wouldn't do that, because that would conflict with fontawesome 3.x.
One way would be changing prefix and using [class*=#{$simple-line-icon-prefix}]
Other way is going back to listing all classes.
@notacouch @MichielDeMey suggestions please.
I chose the latter as other projects' markup is already using the icon-
prefix.
Well, I see nothing wrong with listing all the classes.
What fontawesome does for current and older versions is use the base of the prefix as well as the actual icon class. e.g. <i class="icon icon-key"></i>
and <i class="fa fa-key"></i>
. This serves a number of purposes, e.g. right now I am constructing a menu and want to change the color of icons in that menu, instead of targeting a tag, if every icon has a common class like icon
, I can just select them via #menu .icon
as opposed to #menu i
.
In order to avoid conflicting with other libraries, instead of icon[-]
you'd namespace it e.g. sli[-]
(where sli
is short for "simple line icon").
TL;DR
scss:
// $simple-line-icon-prefix ... becomes
$simple-line-icon-prefix-base: "sli" !default;
$simple-line-icon-prefix: "#{$simple-line-icon-prefix-base}-" !default;
// ...
//[class^=""], [class*=" "] becomes
.#{$simple-line-icon-prefix-base} {
markup:
<span class="icon-key"></span>
<!-- becomes -->
<span class="sli sli-key"></span>
Problem is... moves away from the simplicity that you have as well as changes the class name.
Listing all the classes :+1: Please check out https://github.com/thesabbir/simple-line-icons/tree/notacouch And see if it's working
What @notacouch suggested looks like a good approach.
Although it could be simpler by simply appending a font-icon prefix (sli
) to the existing classes
<span class="icon-key"></span>
<!-- becomes -->
<span class="sli icon-key"></span>
Looking at commit https://github.com/thesabbir/simple-line-icons/commit/ba17e6f8c42692f05ee538a96b06488cbfadae94 collision can still occur when other font icons are using something like icon-user
, right?
@MichielDeMey that's true. But as @notacouch said:
Problem is... moves away from the simplicity that you have as well as changes the class name.
I'm fine with the commit, but it might be a good idea to let the developers know about the classnames (and how you can set them with the sass/less variable) in the readme?
@MichielDeMey Please have a look https://github.com/thesabbir/simple-line-icons/tree/notacouch#customizing-lesssass-variables. Not a good how though.
So #23 it is
That's ok, anyone who knows either SASS or LESS will most likely know how those variables work.
That's better.
Thanks!
I would consider having the simplicity as the default and a namespaced version (where they have to add an additional class to their tags) as an option. This way backward compatibility can be maintained while being able to play it safe if needed.
@notacouch @MichielDeMey Thanks!
I have a project in which I am using fontawesome and simple line icons, the older code which lists out all the classes worked fine, but when I updated my fontawesome icons vanished.
Problem was this selector:
I wound up reverting to this commit: https://github.com/thesabbir/simple-line-icons/blob/a8dcf0c5bd4cd0c36604e054478c021dda6aef8e/scss/simple-line-icons.scss
Could you explain why the change was made?