-
-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add sport icons #1304
Add sport icons #1304
Conversation
@danielbayley is attempting to deploy a commit to the Lucide Team on Vercel. A member of the Team first needs to authorize it. |
@karsa-mistmere Thanks, ha of course you did! Yeah cool with the Nice, like the bowling ball idea!
I have a bunch of iterations. Will post them soon… Do you think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cute icons 👍
icons/goal-net.svg
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that much of a goal feel for me here, but also no clue how to do this any better.
@ericfennis Implemented those changes, and left the removed icons above, for reference…
Are those any less of a sport than the others? 🤷🏻♂️ Seems a shame to waste
Yeah, myself included! Are you thinking we should go with |
@jguddas What about this? |
I like it, and no artifacts 👍 |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
soccer-pitch looks a bit to heavy next to others. needs maybe a little tweak |
soccer-ball looks like a life ring. needs a little tweak as well |
@danielbayley I agree with @ygd. It need some tweaking. 🤔 |
from an agile point of view, PRs with multiple icons are slowing down shipping new icons. eg. a PR with 10 icons, 9 are good but 1 is not so all icons are on hold and cannot be shipped. They must wait until the one which is not looking good is fixed. I suggest PRs containing only one icon. am I missing something, @ericfennis? |
@ygd Yess you are right. A PR with a lot of icons is harder to review. |
This PR is merged to the lab branch. This PR does not match our PR template, which requires valid use cases. To move on we merged this PR to the lab branch, which will be moved to a separate repo in the future. |
@ericfennis I have some of these tweaks ready to go… but it’s a moving target! What is the procedure with this new lab branch? Do I just |
Good point. Maybe grouping similar icons, like balls or bottles, in a PR makes more sense. Do you think it is good to write this down in contribution guidelines? |
@danielbayley You can create PR to make changes on the lab branch. |
@ericfennis Does this imply that all icons already in this PR—and all others merged into |
Alternatives for
football
(/rugby-ball
?) Maybe we should prefer to cover both with a single icon…?)Contributes to #119
Leaving removed icons here, for reference…