Skip to content
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

Merged
merged 7 commits into from
Dec 15, 2023

Conversation

jmsv
Copy link
Contributor

@jmsv jmsv commented Oct 19, 2023

What is the purpose of this pull request?

  • New Icon
  • Bug fix
  • New Feature
  • Documentation update
  • Other:

Description

New icons:

  • user-search
  • user-search-2

Icon use case

  • Searching for users
  • Recruitment

Alternative icon designs

Icon Design Checklist

Concept

  • I have provided valid use cases for each icon.
  • I have not added any a brand or logo icon.
  • I have not used any hate symbols.
  • I have not included any religious or political imagery.

Author, credits & license

  • The icons are solely my own creation.
  • The icons were originally created in # by @
  • I've based them on the following Lucide icons: user-check, user-plus, user-x etc. + corresponding -2 variations, plus search
  • I've based them on the following design:

Naming

  • I've read and followed the naming conventions
  • I've named icons by what they are rather than their use case.
  • I've provided meta JSON files in icons/[iconName].json.

Design

  • I've read and followed the icon design guidelines
  • I've made sure that the icons look sharp on low DPI displays.
  • I've made sure that the icons look consistent with the icon set in size, optical volume and density.
  • I've made sure that the icons are visually centered.
  • I've correctly optimized all icons to two points of precision.

Before Submitting

@github-actions github-actions bot added 🎨 icon About new icons 🫧 metadata Improved metadata labels Oct 19, 2023
@github-actions
Copy link

github-actions bot commented Oct 19, 2023

Added or changed icons

icons/user-round-search.svg icons/user-search.svg

Preview cohesion icons/square-dashed-bottom-code.svg icons/square-user-round.svg
icons/user-round-search.svg icons/user-search.svg
icons/train-front.svg icons/file-lock-2.svg
Preview stroke widths icons/user-round-search.svg icons/user-search.svg
icons/user-round-search.svg icons/user-search.svg
icons/user-round-search.svg icons/user-search.svg
DPI Preview (24px) icons/user-round-search.svg icons/user-search.svg
Icon X-rays icons/user-round-search.svg icons/user-search.svg

@jmsv
Copy link
Contributor Author

jmsv commented Oct 19, 2023

@jguddas that looks good, did that exist somewhere already? missed it if so

had a go at making a similar version for the user-2 style:

icon (2)

@jmsv
Copy link
Contributor Author

jmsv commented Oct 19, 2023

updated this PR with the new icons, looks much better, thanks for having a look!

Copy link
Member

@jguddas jguddas left a 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.

icons/user-search.json Outdated Show resolved Hide resolved
icons/user-search-2.json Outdated Show resolved Hide resolved
jmsv and others added 3 commits October 21, 2023 15:27
Co-authored-by: Jakob Guddas <github@jguddas.de>
Co-authored-by: Jakob Guddas <github@jguddas.de>
@jmsv
Copy link
Contributor Author

jmsv commented Oct 21, 2023

It's based on an existing icon, so it should inherit the contributors.

gotcha, missed that – fixed

Copy link
Member

@jguddas jguddas left a 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.

@karsa-mistmere
Copy link
Member

user-search-2 violates naming guidelines, icons should not end with -2.

Yes, but all our user*-2 icons violate this naming guideline, so it's not @jmsv's fault, strictly speaking.

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 user-round-*, but I do really feel that this is a separate issue.

@jguddas
Copy link
Member

jguddas commented Oct 26, 2023

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 user-search and 👎 for user-search-2 from me.

@jmsv
Copy link
Contributor Author

jmsv commented Oct 26, 2023

Thanks for having a look @jguddas and @karsa-mistmere ❤️


I highly recommend creating one PR per icon.

From CONTRIBUTING.md:

If you want submit multiple icons, please separate the icons and group them. That makes reviewing the icons easier and keep the thread clean and scoped. So don't submit multiple icons in one PR that have noting to do with each other. So for example don't create one PR with icons: arrow-up, bicycle, arrow-down. Seperate them by two PRs; 'pr-01' arrow, arrow-down and 'pr-02' bicycle.

As-per these guidelines I think I was correct in making this one PR.


Re the user*-2 naming I'm aware that the Naming conventions advise against -2 for "what makes the alternate unique" but I opted for naming consistency instead – I agree with @karsa-mistmere that it makes more sense than deviating.

I like the sound of user-round-*! Should take into account though that this would be a breaking change for anyone currently using user*-2 icons.


As for what to do with this PR, it looks like the options are:

  1. Keep the user-search-2 name for now
  2. Rename user-search-2 to user-round-search
  3. Remove user-search-2 entirely

Option 2 isn't ideal since it introduces naming inconsistency, and option 3 isn't ideal since it also introduces inconsistency considering every other user-* icon has a -2 variant.

Hoping we can go with option 1 but let me know what you think

@karsa-mistmere
Copy link
Member

We can add backwards compatible aliases for current user-*-2 icons.

@jmsv
Copy link
Contributor Author

jmsv commented Oct 26, 2023

We can add backwards compatible aliases for current user-*-2 icons.

Ah, awesome! I'd be happy to have a go at setting that up

@jguddas
Copy link
Member

jguddas commented Oct 26, 2023

We can add backwards compatible aliases for current user-*-2 icons.

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 renameIcon.sh script a try.

#1630

How to use it

Make 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

@jmsv
Copy link
Contributor Author

jmsv commented Oct 26, 2023

Done! #1638

Probably makes sense to get that one merged first and then I'll update this PR to rename user-search-2 to user-round-search

@jmsv jmsv marked this pull request as draft October 27, 2023 16:49
@jmsv jmsv marked this pull request as ready for review November 5, 2023 23:58
@jmsv jmsv changed the title New icons: user-search and user-search-2 New icons: user-search and user-round-search Nov 26, 2023
@ericfennis ericfennis merged commit 494f795 into lucide-icons:main Dec 15, 2023
6 checks passed
@jmsv jmsv deleted the icon-user-search branch December 15, 2023 13:07
realguse pushed a commit to realguse/lucide that referenced this pull request Dec 25, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 icon About new icons 🫧 metadata Improved metadata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants