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 icon: Bootstrap (original) #750

Merged
merged 19 commits into from Aug 6, 2021
Merged

new icon: Bootstrap (original) #750

merged 19 commits into from Aug 6, 2021

Conversation

roythearsonist
Copy link
Contributor

Remake of #739 with nicer history

@roythearsonist
Copy link
Contributor Author

@Thomas-Boi Ok I fixed it like you said too last time, please run the peek bot. Thanks in advance

@Panquesito7 Panquesito7 added the feature:icon Use this label for pull requests when a new icon is ready to be added to the collection label Jul 16, 2021
devicon.json Outdated Show resolved Hide resolved
devicon.json Outdated Show resolved Hide resolved
icons/bootstrap/bootstrap-original.svg Outdated Show resolved Hide resolved
roythearsonist and others added 3 commits July 16, 2021 17:31
Co-authored-by: David Leal <halfpacho@gmail.com>
Co-authored-by: David Leal <halfpacho@gmail.com>
Co-authored-by: David Leal <halfpacho@gmail.com>
@roythearsonist
Copy link
Contributor Author

@Panquesito7 I added your suggestions, anything else I should add?

@Panquesito7 Panquesito7 changed the title New icon: Bootstrap (original) new icon: Bootstrap (original) Jul 16, 2021
@roythearsonist
Copy link
Contributor Author

I am finally done after disappearing from this pr for a week. @Panquesito7 can you review this, please? Thanks.

@roythearsonist
Copy link
Contributor Author

Whoops I made an error uploading it

@github-actions

This comment has been minimized.

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Great, looks amazing now, but the viewBox must be 0 0 128 128.

@roythearsonist
Copy link
Contributor Author

Done

Co-authored-by: David Leal <halfpacho@gmail.com>
@roythearsonist
Copy link
Contributor Author

This good?

@Panquesito7 Panquesito7 added the bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger label Jul 26, 2021
@github-actions
Copy link
Contributor

Hi there,

I'm Devicons' Peek Bot and I just peeked at the icons that you wanted to add using icomoon.io.
Here is the result below (top left):

Imgur Images

Here are the zoomed-in screenshots of the added icons:
Imgur ImagesImgur Images

Note: If the images don't show up, it's probably because it has been autodeleted by Imgur after 6 months due to our API choice.

The maintainers will now take a look at it and decide whether to merge your PR.

Thank you for contributing to Devicon! I hope everything works out and your icons are accepted into the repo.

Cheers,
Peek Bot 😊

@Panquesito7
Copy link
Member

Panquesito7 commented Jul 26, 2021

The parts marked with red are not shown in the images provided by the Peek Bot. Not sure why. @Thomas-Boi, thoughts?

image

Ahh, I know why. That's because the original version was added to this iconset, but the plain version isn't modified. That's fine, I guess.

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your dedication and contributions to our community! 👍 🎉

@roythearsonist
Copy link
Contributor Author

LGTM. Thank you for your dedication and contributions to our community! 👍 🎉

I'm just happy that I was able to make a svg without messing it up.

@roythearsonist
Copy link
Contributor Author

I think Thomas forgot to review this

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

I'll merge this now. If anybody has any objections, please make another PR to fix it.

@Panquesito7 Panquesito7 merged commit 555b02e into devicons:develop Aug 6, 2021
@Thomas-Boi
Copy link
Member

Thomas-Boi commented Aug 6, 2021

Hello,

Sorry for the late review. This was buried in my email inbox and I was unaware that things have been updating.

Ahh, I know why. That's because the original version was added to this iconset, but the plain version isn't modified. That's fine, I guess.

You are correct @Panquesito7. Only the original version was updated and peek only checks the plain version.

However, this raises the question of why the plain version does not match the original version. Since we updated the original version, we should be having the plain version matching the original version as well.

Furthermore @theblobscp, I notice that the original version is a little "squished" compared to the one I found online. There's also a line that divides the SVG into two different colors:
image

It would be great if you can resize the image so it looks more natural. If this specific version (the one with the weird side poking out) is causing you trouble, perhaps you can use this version, which is Bootstrap's official icon, instead. Bootstrap also provides a plain version of their icon here. Just search for bootstrap in the search bar and they will offer you two icons.

It's probably best if you can update the plain versions with these new versions. However, if that's too much work, just updating the original is fine. I'll update the plain version in a future PR.

@roythearsonist roythearsonist deleted the og-bootstrap branch August 7, 2021 18:32
@amacado amacado mentioned this pull request Aug 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger feature:icon Use this label for pull requests when a new icon is ready to be added to the collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants