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

add bookmark-add icon #413

Merged
merged 6 commits into from
Sep 27, 2021
Merged

Conversation

siarie
Copy link
Contributor

@siarie siarie commented Sep 27, 2021

Preview
Preview

100%
bookmark-add

Signed-off-by: Sri Aspari <[email protected]>
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.

@siarie Awesome icon!
Can you rename it to bookmark-plus?
Then the naming matches with the other "plus" icons.
As optional you could apply some extra tags in "tags.json", like the word "add" so you can find it more easily on the lucide.dev website.

@siarie
Copy link
Contributor Author

siarie commented Sep 27, 2021

Hi @ericfennis, I've rename it to bookmark-plus.

@siarie siarie requested a review from ericfennis September 27, 2021 17:38
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.

@siarie One thing, I see you used half pixels in the plus part.
That will result in blurry pixels on low res devices.
I think it is better if we slightly increase the size of the plus so we are on rounded pixels. But we need to "blunting" the point a bit.
We need to change this as wel in the bookmark icon.

Left is yours, new one on the right.
image

Comment on lines 12 to 14
<path d="M19 21l-7-5-7 5V5a2 2 0 012-2h10a2 2 0 012 2z" />
<line x1="12" x2="12" y1="7" y2="12" />
<line x1="14.5" x2="9.5" y1="9.5" y2="9.5" />
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<path d="M19 21l-7-5-7 5V5a2 2 0 012-2h10a2 2 0 012 2z" />
<line x1="12" x2="12" y1="7" y2="12" />
<line x1="14.5" x2="9.5" y1="9.5" y2="9.5" />
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
<line x1="12" x2="12" y1="7" y2="13" />
<line x1="15" x2="9" y1="10" y2="10" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well actually I copied from the bookmark icon and just added the plus icon. Do I need to change the bookmark too?

Copy link
Member

Choose a reason for hiding this comment

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

Yes if you can do 😃, that would be great.
And maybe you can add a bookmark-minus as wel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅ Done.

Signed-off-by: Sri Aspari <[email protected]>
@siarie siarie changed the title Add bookmark-add icon Add more bookmark icon variant Sep 27, 2021
@@ -9,5 +9,5 @@
stroke-linecap="round"
stroke-linejoin="round"
>
<path d="M19 21l-7-5-7 5V5a2 2 0 012-2h10a2 2 0 012 2z" />
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z" />

Comment on lines 12 to 13
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
<line x1="15" x2="9" y1="10" y2="10"/>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
<line x1="15" x2="9" y1="10" y2="10"/>
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z" />
<line x1="15" x2="9" y1="10" y2="10" />

Comment on lines 12 to 14
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
<line x1="12" x2="12" y1="7" y2="13"/>
<line x1="15" x2="9" y1="10" y2="10"/>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z"/>
<line x1="12" x2="12" y1="7" y2="13"/>
<line x1="15" x2="9" y1="10" y2="10"/>
<path d="m19 21-7-4-7 4V5a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v16Z" />
<line x1="12" x2="12" y1="7" y2="13" />
<line x1="15" x2="9" y1="10" y2="10" />

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.

@siarie Awesome great work. One last small code style thingy.

@ericfennis
Copy link
Member

@siarie Great work, thanks for the new icons!

@ericfennis ericfennis merged commit 3e61ee5 into lucide-icons:master Sep 27, 2021
@siarie
Copy link
Contributor Author

siarie commented Sep 27, 2021

You're welcome... 😄

@siarie siarie deleted the icons/bookmark-add branch September 27, 2021 19:14
@moeenio moeenio changed the title Add more bookmark icon variant add bookmark-add icon Sep 28, 2021
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.

2 participants