Skip to content

Conversation

@AcmeGamers
Copy link
Contributor

@AcmeGamers AcmeGamers commented Jul 3, 2022

Double check these details before you open a PR

  • [ ✔ ] PR does not match another non-stale PR currently opened
  • [ ✔ ] PR name matches the format new icon: Icon name (versions separated by comma). More details here
  • [ ✔ ] PR's base is the develop branch.
  • [ ✔ ] Your icons are inside a folder as seen here
  • SVG matches the standards laid out here
  • [ ✔ ] A new object is added in the devicon.json file as seen here

Link to prove your SVG is correct and up-to-date.

https://www.solidjs.com/

Copy link
Contributor

@kilianpaquier kilianpaquier left a comment

Choose a reason for hiding this comment

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

Hello again, thank you for this separate PR, here are a list of standards you will have to respect :

  • You don't need aliases if you committed a separated plain and plain-wordmark SVG
  • There should only be one .eps (if you decide to commit one, the .eps is optional) file with all the SVG you included in the PR (one page in the .eps per SVG)
  • As stated in the versions standards here https://github.com/devicons/devicon/wiki/SVG-Versions, each SVG must be called iconName-version.svg -> The SVG name wordmark.svg isn't necessary, for which purpose did you add it ?
  • The paths in the plain versions must be united, I made suggestions for that part
  • Each SVG must be in a viewbox of 0 0 128 128, I made suggestions for that part, but you'll have to respect the standards for the two other PR
  • You have conflicts with the develop branch, we won't be able to merge the changes
  • Optionally, I've made suggestion for the optimization of the SVGs
  • You'll have to rename your first commit Adding Solidjs SVGs into new icon: solidjs (original, original-wordmark, plain, plain-wordmark)
  • You'll also have to rename in lowercase solidjs in the PR title

Feel free to ping me if you need any help

"aliases": []
}
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

{ identation 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot really, you really helped a lot on these TwT

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not solved yet :)

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" width="306.42" height="70.7"><defs><clipPath id="a"><path style="fill:none" d="M0 0h306.42v70.7H0z"/></clipPath></defs><g style="clip-path:url(#a)"><path d="M33.9 45a12 12 0 0 0 8.1 3.4c3.4 0 6.2-2 6.2-5.4 0-7.8-15.5-5.9-15.5-15.5 0-4.5 4-8.3 9.7-8.3a12 12 0 0 1 8.3 3l-1.5 2.6a10.38 10.38 0 0 0-6.8-2.7c-4 0-6.5 2.6-6.6 5.3 0 7.4 15.5 5.3 15.5 15.5 0 4.6-3.6 8.4-9.4 8.4a13.36 13.36 0 0 1-9.8-4ZM83 19.4a15.73 15.73 0 0 1 15.8 15.9A16 16 0 0 1 83 51.5h-.1a16 16 0 0 1-15.8-16.2v-.1A15.73 15.73 0 0 1 83 19.4m-.1 29.3a13 13 0 0 0 12.7-13.29v-.11c0-7.4-5.6-13.1-12.6-13.1a12.72 12.72 0 0 0-12.7 13c0 7.6 5.6 13.4 12.6 13.5M116.8 20h3l-.1 28.3h14.4V51h-17.5Zm33.9.1h3l-.1 31.1h-3Zm23.2 0h10c9.3 0 15.6 5.7 15.6 15.6s-6.3 15.5-15.6 15.5h-10Zm9.6 28.4c7.6 0 12.7-4.5 12.8-12.8s-5.1-12.8-12.7-12.8h-6.7l-.1 25.6Zm42.17 2.7a10.27 10.27 0 0 0 7.06-2.52 9 9 0 0 0 2.86-7.1V20.47h-6.25v20.65a4.2 4.2 0 0 1-1 3 3.61 3.61 0 0 1-2.77 1.07q-2.94 0-4.29-3.45L216 44.69q2.6 6.51 9.69 6.51m37.44.07c3.47 0 6.19-.86 8.14-2.59a8.63 8.63 0 0 0 2.93-6.79 7.09 7.09 0 0 0-2.5-5.67c-1.68-1.44-4.37-2.68-8.07-3.74a11.76 11.76 0 0 1-3.76-1.57 2.57 2.57 0 0 1-1.1-2.15 2.7 2.7 0 0 1 1.1-2.25 4.6 4.6 0 0 1 2.91-.85 5.67 5.67 0 0 1 5.71 3.72l5.17-2.83a10.12 10.12 0 0 0-4.07-4.85 12.25 12.25 0 0 0-6.54-1.7 11.11 11.11 0 0 0-7.45 2.6 8.11 8.11 0 0 0-3.09 6.55 7.5 7.5 0 0 0 2.32 5.61q2.09 2.1 7.37 3.69a16.51 16.51 0 0 1 4.52 1.82 2.33 2.33 0 0 1 1.19 2.09 2.74 2.74 0 0 1-1.29 2.31 5.64 5.64 0 0 1-3.3.91 7 7 0 0 1-6.6-4.23l-5.4 2.79a10.85 10.85 0 0 0 4.49 5.23 13.74 13.74 0 0 0 7.32 1.9" style="fill:#fff"/></g></svg> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need that SVG, it doesn't belong to any possible version stated here : https://github.com/devicons/devicon/wiki/SVG-Versions -> Unless you want to make it the solidjs-line-wordmark ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I was adding that SVG to the logo :')

Copy link
Collaborator

Choose a reason for hiding this comment

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

This icon needs to be removed from the PR

AcmeGamers and others added 6 commits July 5, 2022 11:38
Co-authored-by: kilian-paquier <107955904+kilian-paquier@users.noreply.github.com>
Co-authored-by: kilian-paquier <107955904+kilian-paquier@users.noreply.github.com>
Co-authored-by: kilian-paquier <107955904+kilian-paquier@users.noreply.github.com>
Co-authored-by: kilian-paquier <107955904+kilian-paquier@users.noreply.github.com>
Co-authored-by: kilian-paquier <107955904+kilian-paquier@users.noreply.github.com>
Co-authored-by: kilian-paquier <107955904+kilian-paquier@users.noreply.github.com>
@Snailedlt Snailedlt added the feature:icon PR when a new icon is ready to be added to the collection label Jul 28, 2022
Copy link
Collaborator

@Snailedlt Snailedlt left a comment

Choose a reason for hiding this comment

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

Getting closer!

Still a few things that needs fixing though (see comments for more details):

  1. Remove .eps files
  2. Change logos to the official ones from :https://www.solidjs.com/media
  3. Change primary color to #2c4f7c as per the website: https://www.solidjs.com/media
  4. Fix indentation
  5. Add altnames to devicon.json (empty array if there are none)
  6. Remove changes to irrelevant files (see comments)

"aliases": []
}
},
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not solved yet :)

{
"name": "devicon",
"version": "2.15.0",
"version": "2.15.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't be edited for icon requests. Please revert/reset the changes

{
"name": "devicon",
"version": "2.15.0",
"version": "2.15.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shoulsn't be edited for icon requests. Please revert/reset the changes:)

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" width="306.42" height="70.7"><defs><clipPath id="a"><path style="fill:none" d="M0 0h306.42v70.7H0z"/></clipPath></defs><g style="clip-path:url(#a)"><path d="M33.9 45a12 12 0 0 0 8.1 3.4c3.4 0 6.2-2 6.2-5.4 0-7.8-15.5-5.9-15.5-15.5 0-4.5 4-8.3 9.7-8.3a12 12 0 0 1 8.3 3l-1.5 2.6a10.38 10.38 0 0 0-6.8-2.7c-4 0-6.5 2.6-6.6 5.3 0 7.4 15.5 5.3 15.5 15.5 0 4.6-3.6 8.4-9.4 8.4a13.36 13.36 0 0 1-9.8-4ZM83 19.4a15.73 15.73 0 0 1 15.8 15.9A16 16 0 0 1 83 51.5h-.1a16 16 0 0 1-15.8-16.2v-.1A15.73 15.73 0 0 1 83 19.4m-.1 29.3a13 13 0 0 0 12.7-13.29v-.11c0-7.4-5.6-13.1-12.6-13.1a12.72 12.72 0 0 0-12.7 13c0 7.6 5.6 13.4 12.6 13.5M116.8 20h3l-.1 28.3h14.4V51h-17.5Zm33.9.1h3l-.1 31.1h-3Zm23.2 0h10c9.3 0 15.6 5.7 15.6 15.6s-6.3 15.5-15.6 15.5h-10Zm9.6 28.4c7.6 0 12.7-4.5 12.8-12.8s-5.1-12.8-12.7-12.8h-6.7l-.1 25.6Zm42.17 2.7a10.27 10.27 0 0 0 7.06-2.52 9 9 0 0 0 2.86-7.1V20.47h-6.25v20.65a4.2 4.2 0 0 1-1 3 3.61 3.61 0 0 1-2.77 1.07q-2.94 0-4.29-3.45L216 44.69q2.6 6.51 9.69 6.51m37.44.07c3.47 0 6.19-.86 8.14-2.59a8.63 8.63 0 0 0 2.93-6.79 7.09 7.09 0 0 0-2.5-5.67c-1.68-1.44-4.37-2.68-8.07-3.74a11.76 11.76 0 0 1-3.76-1.57 2.57 2.57 0 0 1-1.1-2.15 2.7 2.7 0 0 1 1.1-2.25 4.6 4.6 0 0 1 2.91-.85 5.67 5.67 0 0 1 5.71 3.72l5.17-2.83a10.12 10.12 0 0 0-4.07-4.85 12.25 12.25 0 0 0-6.54-1.7 11.11 11.11 0 0 0-7.45 2.6 8.11 8.11 0 0 0-3.09 6.55 7.5 7.5 0 0 0 2.32 5.61q2.09 2.1 7.37 3.69a16.51 16.51 0 0 1 4.52 1.82 2.33 2.33 0 0 1 1.19 2.09 2.74 2.74 0 0 1-1.29 2.31 5.64 5.64 0 0 1-3.3.91 7 7 0 0 1-6.6-4.23l-5.4 2.79a10.85 10.85 0 0 0 4.49 5.23 13.74 13.74 0 0 0 7.32 1.9" style="fill:#fff"/></g></svg> No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

This icon needs to be removed from the PR

@@ -0,0 +1,4 @@
<svg viewBox="0 0 128 128" xmlns="http://www.w3.org/2000/svg">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good! 👍

@@ -0,0 +1,57 @@
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 128 128">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please replace this icon withthe wordmark version from here:
https://www.solidjs.com/media

Notice how the text color and font is different

@@ -0,0 +1,4 @@
<svg viewBox="0 0 128 128" xmlns="http://www.w3.org/2000/svg">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please replace with the wordmark from here: https://www.solidjs.com/media

Then resize to fit inside 0 0 128 128, and lastly merge the paths and change the fill color to the primary color

"plain-wordmark",
]
},
"color": "#85bce5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change to primary color from their website: https://www.solidjs.com/media

Suggested change
"color": "#85bce5",
"color": "#2c4f7c",

}
},
{
"name": "solidjs",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add empty altnames array. You can fill it with alternative names if there are any

Suggested change
"name": "solidjs",
"name": "solidjs",
"altnames": [],

@Snailedlt
Copy link
Collaborator

Snailedlt commented Aug 6, 2022

@AcmeGamers I suggested a few changes. Let us know if you need any help with it :)

@Snailedlt Snailedlt added the stale PRs that haven't had any activity for a while and old issues label Oct 1, 2022
@Snailedlt
Copy link
Collaborator

superseeded by #1557

@Snailedlt Snailedlt closed this Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature:icon PR when a new icon is ready to be added to the collection stale PRs that haven't had any activity for a while and old issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants