search

phosphor-icons / homepage

The homepage of Phosphor Icons, a flexible icon family for everyone
https://phosphoricons.com
MIT License
4.26k stars 101 forks source link

chore(docs): list phosphor-css in community projects #209

Closed lucagoslar closed 1 year ago

rektdeckard commented 1 year ago

I quite like this implementation! Will take a further look and demo it tonight before adding. But thanks for the contribution!

rektdeckard commented 1 year ago

Exactly what I was hoping for in publishing the core package :)

lucagoslar commented 1 year ago

Fantastic, thanks!

lucagoslar commented 1 year ago

Found a bug - I missed adding a dollar sign, breaking most SASS and CSS behaviour. You can find a demo at https://github.com/lucagoslar/phosphor-css-demo-pr-209, which I deployed to https://lucagoslar.github.io/phosphor-css-demo-pr-209/.

lucagoslar commented 1 year ago

My apologies.

rektdeckard commented 1 year ago

@lucagoslar thanks for the fix, I was having trouble testing weights other than regular on my end and thought I was doing something wrong 😅. Testing again...

rektdeckard commented 1 year ago

I had one small comment. Not to tell you how to make your lib, but I have a feeling that using compound class selectors .ph.train.duotone might lead to some unintentional outside styles being applied, since the individual class names are so simple. That's why we used atomic classes like .ph-train-duotone in our implementation, to try to avoid clashing.

rektdeckard commented 1 year ago

Also, out of curiosity, is it possible to color the icons without resorting to CSS image filters?

lucagoslar commented 1 year ago

@lucagoslar thanks for the fix, I was having trouble testing weights other than regular on my end and thought I was doing something wrong 😅. Testing again...

Oh no - I‘m so sorry. For whatever reason, I didn’t notice earlier.

lucagoslar commented 1 year ago

Fair point. However, ‘.train’ has no effect without ‘.ph’. So, unless one is using ‘.ph’ for another purpose, there should be no trouble. In addition, using exclusively atomic classes would not allow reusing styles, producing a much larger output. How about ‘.ph.icon’ instead?

lucagoslar commented 1 year ago

Excellent idea. This could work using a mask instead of a background image - using filters might not be necessary. I will be taking care of this in the future.