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 netlify, pnpm, gridsome and eleventy icons #518

Closed
wants to merge 10 commits into from

Conversation

hacknug
Copy link
Contributor

@hacknug hacknug commented Aug 22, 2019

This PR:

  • adds support for other netlify-related files and folders as requested in [Icon Request] Add .netlify folder icon #399
  • adds support for pnpm-related files
  • adds support for grisome-related files
  • adds support for eleventy-related folders
  • fixes the preview issue where some of the last icons might not be shown because there weren't enough to fill a new row.

Closes #399.

@hacknug hacknug changed the title Add netlify folder icons Add netlify folder and pnpm icons Aug 22, 2019
@hacknug hacknug changed the title Add netlify folder and pnpm icons Add netlify, pnpm, gridsome and eleventy icons Aug 30, 2019
@hacknug
Copy link
Contributor Author

hacknug commented Sep 30, 2019

@PKief anything I can do to help with this?

@PKief
Copy link
Member

PKief commented Nov 23, 2019

fixes the preview issue where some of the last icons might not be shown because there weren't enough to fill a new row

It's a good idea of you, but it's the intention that the columns of the table should always be filled down to the bottom, because it simply looks nicer that way, even if sometimes some icons can't be displayed.

In general, it is much easier if you could create a separate pull request for each change. If a lot of things are changed in a single pull request, it makes the review much more cumbersome. If there is something in this single pull request that doesn't fit, everything can't be merged, even if there may be good things in it.

src/icons/fileIcons.ts Outdated Show resolved Hide resolved
src/icons/folderIcons.ts Outdated Show resolved Hide resolved
Copy link
Member

@PKief PKief left a comment

Choose a reason for hiding this comment

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

In addition, nearly all of the icons have a transparent motive so that the background shines through
e.g.
image

The white "G" is not transparent:
image

This is important to have a good consistency to the other icons in the theme.

The icon also seems to have a gradient which is not that common in the Material Design Pattern. It would be great if you could check, if it also works without the gradient.

@PKief
Copy link
Member

PKief commented Nov 23, 2019

Have you made sure that the following icon works with a dark background?

image

Because of the grey color that you use here.

In addition, the distances between the squares are too small. They are very difficult to see in the small view of File Explorer in VS Code.

@hacknug
Copy link
Contributor Author

hacknug commented Nov 24, 2019

All good. Will open individual PRs with the changes later and we can take it from there. Thank you!

@hacknug hacknug closed this Nov 24, 2019
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] Add .netlify folder icon
2 participants