-
-
Notifications
You must be signed in to change notification settings - Fork 554
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
New icons: user-search and user-round-search #1620
Conversation
@jguddas that looks good, did that exist somewhere already? missed it if so had a go at making a similar version for the |
updated this PR with the new icons, looks much better, thanks for having a look! |
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.
It's based on an existing icon, so it should inherit the contributors.
Co-authored-by: Jakob Guddas <github@jguddas.de>
Co-authored-by: Jakob Guddas <github@jguddas.de>
gotcha, missed that – fixed |
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.
user-search-2
violates naming guidelines, icons should not end with -2
.
I highly recommend creating one PR per icon.
Yes, but all our All these names should be fixed, I agree, but while they're not, using the same naming scheme is preferable to deviating from it. I'm voting for |
IMO adding icons with the same use case but with different styles is something we should avoid, it works against having one consistent feel across all icons. So 👍 for |
Thanks for having a look @jguddas and @karsa-mistmere ❤️
From CONTRIBUTING.md:
As-per these guidelines I think I was correct in making this one PR. Re the I like the sound of As for what to do with this PR, it looks like the options are:
Option 2 isn't ideal since it introduces naming inconsistency, and option 3 isn't ideal since it also introduces inconsistency considering every other Hoping we can go with option 1 but let me know what you think |
We can add backwards compatible aliases for current |
Ah, awesome! I'd be happy to have a go at setting that up |
If you are up for it, you can give my new How to use itMake sure you have the gh cli installed. Don't think this will work on Windows. gh pr checkout 1630 # go to my PR with the new rename script
./scripts/renameIcon.sh user-2 user-round # run the script
git checkout main # go back to main
git checkout -b rename/user-2-to-user-round # create a new branch
git commit -m 'Renamed user-2 to user-round' # commit the changes
gh pr create --title 'Renamed user-2 to user-round' # create a pull request
git checkout main # go back to main |
Done! #1638 Probably makes sense to get that one merged first and then I'll update this PR to rename |
* user-search and user-search-2 icons * updated user-search icons * Update icons/user-search.json Co-authored-by: Jakob Guddas <github@jguddas.de> * Update icons/user-search-2.json Co-authored-by: Jakob Guddas <github@jguddas.de> * Renamed user-search-2 to user-round-search. --------- Co-authored-by: Jakob Guddas <github@jguddas.de> Co-authored-by: Karsa <contact@karsa.org>
What is the purpose of this pull request?
Description
New icons:
user-search
user-search-2
Icon use case
Alternative icon designs
Icon Design Checklist
Concept
Author, credits & license
user-check
,user-plus
,user-x
etc. + corresponding-2
variations, plussearch
Naming
icons/[iconName].json
.Design
Before Submitting