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

feat: pin icon #438

Merged
merged 7 commits into from Dec 27, 2021
Merged

feat: pin icon #438

merged 7 commits into from Dec 27, 2021

Conversation

mittalyashu
Copy link
Member

@mittalyashu mittalyashu commented Nov 13, 2021

  • Name of the icon : Pin
  • - Tags (alternative names for this icon)
Full scale 100%
<img
CleanShot 2021-11-16 at 21 31 42@2x CleanShot 2021-11-16 at 21 31 49@2x

@mittalyashu
Copy link
Member Author

If anyone has any suggestion for un-pin icon, let me know. I came up with these two but didn't liked both 😞

CleanShot 2021-11-13 at 17 53 55@2x

@ericfennis
Copy link
Member

@mittalyashu Nice icon!! I like it.
I think the needle feels a bit too long. I see also that there is 2 pixels margin on the top and only 1 pixel at the bottom. Maybe decrease the needle one pixel on the bottom so the margins are equal top and bottom.

tags.json Outdated Show resolved Hide resolved
@bduffany
Copy link
Contributor

bduffany commented Nov 14, 2021

@mittalyashu I like the second unpin icon in your screenshot (the one on the right). It seems to match the other "un-" / "-off" styles of the other Lucide icons, with the strikethrough connecting to the bottom-left of the icon, and the gap to the upper-right of the strikethrough.

Co-authored-by: Eric Fennis <eric.fennis@gmail.com>
@mittalyashu
Copy link
Member Author

I also don't like leaving this small part, which looks kind of weird.

CleanShot 2021-11-16 at 21 11 00@2x

@mittalyashu
Copy link
Member Author

I think the needle feels a bit too long. I see also that there is 2 pixels margin on the top and only 1 pixel at the bottom. Maybe decrease the needle one pixel on the bottom so the margins are equal top and bottom. @ericfennis

fixed! Thanks for pointing it out.

@ericfennis
Copy link
Member

@mittalyashu this one feels better!

@mittalyashu
Copy link
Member Author

LOL! I literally forget to git push the commit 🤣

icons/pin.svg Outdated
stroke-linecap="round"
stroke-linejoin="round"
>
<path d="M9 3v5l-2 4v2h10M9 3h6M9 3H7m10 11h-5v7m5-7v-2l-2-4V3m0 0h2" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you split this path up in smaller parts. Otherwise design programs like figma don't like it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like this? Please check the most recent commit.

@ericfennis
Copy link
Member

I think you need to split it once more. maybe the top stroke?
image

@rodshtein
Copy link

I also don't like leaving this small part, which looks kind of weird.

CleanShot 2021-11-16 at 21 11 00@2x

Left have a correct shadow

icons/pin.svg Outdated Show resolved Hide resolved
@ericfennis
Copy link
Member

@mittalyashu Can you take a look, looks like you have some conflicts

@locness3
Copy link
Contributor

Would be great to have a tilted version as well.

Co-authored-by: Eric Fennis <eric.fennis@gmail.com>
@mittalyashu
Copy link
Member Author

Would be great to have a tilted version as well.

@locness3 What do you mean by titled version?

Copy link
Member

@ericfennis ericfennis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@ericfennis
Copy link
Member

@mittalyashu I assume @locness3 thinks about this kind of icon.
image

@ericfennis ericfennis merged commit 2139900 into lucide-icons:master Dec 27, 2021
@karsa-mistmere karsa-mistmere mentioned this pull request Jun 15, 2022
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants