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: stackblitz (Original) #1954

Closed
wants to merge 6 commits into from
Closed

New Icon: stackblitz (Original) #1954

wants to merge 6 commits into from

Conversation

kalkeshwar
Copy link

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

This PR closes NONE

fixes #1953

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

https://github.com/stackblitz/core

lunatic-fox and others added 3 commits September 1, 2023 15:46
* Fix build icons workflow

* Moving Build Icons to Ubuntu machine
@kalkeshwar kalkeshwar changed the base branch from master to develop October 29, 2023 21:59
Copy link
Author

@kalkeshwar kalkeshwar left a comment

Choose a reason for hiding this comment

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

addes correct sized svg icon

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.

Hi and welcome to devicons!
This is a pretty decent first PR, but there are a lot of changes that need to be made. I recommend reading through the wiki (see point 3.) so that you know more about how to submit a PR

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 changed. The reason it's changed here is since you based your branch on the master branch instead of the develop branch.
To fix this, simply create a new branch based on develop (git checkout develop && git pull && git checkout -b new_brannch_name), then apply your changes to that branch.

devicon.json Show resolved Hide resolved
Comment on lines +11288 to +11289
"FullStack Development Online",
"online ide"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"FullStack Development Online",
"online ide"
"Online IDE"

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 can be simplified, so that it contains only one path. See this tutorial on how to unify paths in inkscape: #1306 (comment).

After that the icon can be optimized using SVGOMG: #1306 (comment)

Once the paths are unified, and the icon is optimized, it can also be a plain Icon. To do this simply add an alias object to devicon.json

"original"
],
"font": [
"plain"
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no plain icon here. Since the original icon can be made to fit the requirements for plain icons, this can be changed to original, once my suggestions for stackblitz-original.svg have been implemented

]
},
"color": "#587dbd",
"aliases": ["stack-blitz"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not what the aliases object is used for. Please read see the wiki for more info: on updating devicon.json: https://github.com/devicons/devicon/wiki/Updating-%60devicon.json%60

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

Choose a reason for hiding this comment

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

Add missing altnames

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

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.

[ICON REQUEST]: StackBlitz
3 participants