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.

Sign up for GitHub

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

Jump to bottom

new icon: webassembly (original, plain) #741

Closed
wants to merge 15 commits into from
Closed

new icon: webassembly (original, plain) #741

wants to merge 15 commits into from

Conversation

Utkarsh1504
Copy link
Contributor

issue #367

Screenshot (52)

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

Hi there,

I'm Devicons' Peek Bot and it seems we've ran into a problem.

Expecting value: line 4094 column 13 (char 86000)

Make sure that:

  • Your svgs are named and added correctly to the /icons folder as seen here.
  • Your icon information has been added to the devicon.json as seen here
  • Your PR title follows the format seen here

I will retry once everything is fixed. If I still fail or there are other error, the maintainers will investigate.

Best of luck,
Peek Bot ☺️

Thomas-Boi
Thomas-Boi reviewed Jul 10, 2021
View reviewed changes
Copy link
Member

@Thomas-Boi Thomas-Boi left a comment

Choose a reason for hiding this comment

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

Please be careful of commas. Otherwise, your SVGs look fine.

devicon.json Outdated
"versions": {
"svg": [
"original",
"plain",
Copy link
Member

Choose a reason for hiding this comment

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

Please watch out for commas. JSON is a stricter than JS objects regarding this.

Suggested change
"plain",
"plain"

devicon.json Outdated Show resolved Hide resolved
@Thomas-Boi Thomas-Boi added the feature:icon Use this label for pull requests when a new icon is ready to be added to the collection label Jul 10, 2021
@Utkarsh1504
Copy link
Contributor Author

@Thomas-Boi fixed the commas

Panquesito7
Panquesito7 reviewed Jul 11, 2021
View reviewed changes
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.

Both icons are one color only, so I suggest making an alias for the plain version instead of making another icon. @Thomas-Boi, thoughts?

icons/webassembly/webassembly-original.svg Outdated Show resolved Hide resolved
devicon.json Outdated Show resolved Hide resolved
devicon.json Outdated Show resolved Hide resolved
devicon.json Outdated Show resolved Hide resolved
@Thomas-Boi
Copy link
Member

Upon closer look, it seems like the WA in both icons are white, not transparent. I would recommend leaving the original as is (so the color scheme stays the same), but for the plain, remove the white letter.

@Panquesito7
Copy link
Member

remove the white letter

Or also make it transparent, instead of white. 🙂

Panquesito7
Panquesito7 requested changes Jul 26, 2021
View reviewed changes
icons/webassembly/webassembly-original.svg
@@ -0,0 +1 @@
<svg width="128" height="128" viewBox="0 0 128 128" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M78.64 0v.691c0 8.106-6.57 14.676-14.675 14.676-8.106 0-14.676-6.574-14.676-14.676V0H0v128h128V0H78.64z" fill="#654FF0"/><path d="M29.734 68.98h8.48l5.794 30.844h.105l6.961-30.843h7.934l6.285 31.222h.125l6.601-31.222h8.32L69.528 114.3H61.11l-6.234-30.844h-.164l-6.672 30.844h-8.578l-9.727-45.32zm60.168 0h13.375l13.282 45.321h-8.754l-2.887-10.086H89.684l-2.23 10.086H78.93l10.972-45.32zm5.09 11.172-3.699 16.625h11.516l-4.246-16.625h-3.57z" fill="#fff"/></svg>
Copy link
Member

Choose a reason for hiding this comment

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

Please make the WA text transparent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Panquesito7 in both of the svg or just in the original one?

Copy link
Member

Choose a reason for hiding this comment

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

Both SVGs, please.

Copy link
Member

Choose a reason for hiding this comment

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

@Utkarsh1504 and @Panquesito7 I think just the original version is fine. Since both are the similar, we can actually use the original version for the plain version as well.

So pretty much:

  • Make the WA text in the original version transparent
  • Delete the plain version
  • Update the devicon.json so it includes the original as a font friendly version

@Thomas-Boi
Copy link
Member

Hi @Utkarsh1504, are you still working on this PR?

@Utkarsh1504 Utkarsh1504 deleted the add-webassembly branch August 31, 2021 06:48
@Panquesito7 Panquesito7 removed the bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger label Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Panquesito7 Panquesito7 Panquesito7 requested changes

@Thomas-Boi Thomas-Boi Thomas-Boi left review comments

Assignees
No one assigned
Labels
feature:icon Use this label for pull requests when a new icon is ready to be added to the collection
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Utkarsh1504 @Thomas-Boi @Panquesito7 @amacado