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

Updated vscode file icons with stable and insider versions (1.35) #2092

Merged
merged 6 commits into from Jun 23, 2019

Conversation

KingDarBoja
Copy link
Member

@KingDarBoja KingDarBoja commented Jun 15, 2019

Fixes #2087

Changes proposed:

  • Add
  • Delete
  • Fix
  • Prepare

Since the new vscode icon is the same for all platforms (check this response), I replaced the vscode2 icon with the insiders version just in case someone wants to use it.

Also, this PR is not ready yet because I have to change the folder icon too.

I would like to ask if I could change the vs icon too for the new one.

UPDATE: Gonna need some help with the folder color for the vscode and vscode2 @JimiC .

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #2092 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2092   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          78      78           
  Lines        5959    5959           
  Branches      138     138           
======================================
  Hits         5959    5959

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04e636f...9f0a6c9. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #2092 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2092   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          78      78           
  Lines        5959    5959           
  Branches      138     138           
======================================
  Hits         5959    5959
Impacted Files Coverage Δ
src/iconsManifest/supportedExtensions.ts 100% <ø> (ø) ⬆️
src/iconsManifest/supportedFolders.ts 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10f7c1f...03c00ff. Read the comment docs.

@KingDarBoja KingDarBoja marked this pull request as ready for review June 15, 2019 18:09
Copy link
Member

@JimiC JimiC left a comment

Choose a reason for hiding this comment

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

Do not replace. Add new.

@KingDarBoja
Copy link
Member Author

Do not replace. Add new.

So just append another number for the new versions like 3 & 4 or what? I don't get the idea.

@JimiC
Copy link
Member

JimiC commented Jun 15, 2019

Just like a list. New gets added on top. Old gets pushed one place down the list.

Copy link
Member

@robertohuertasm robertohuertasm left a comment

Choose a reason for hiding this comment

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

It looks good to me. @KingDarBoja are you still waiting for the folder backgrounds color to be changed or should we merge?

Copy link
Member

@JimiC JimiC left a comment

Choose a reason for hiding this comment

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

Vscode second one should not be replaced by insiders icon

@JimiC
Copy link
Member

JimiC commented Jun 16, 2019

@robertohuertasm Don't merge until I resolve the icons. I'm out of town and it's difficult for me to coordinate this by phone.

@robertohuertasm robertohuertasm added this to the Next milestone Jun 16, 2019
@KingDarBoja
Copy link
Member Author

@robertohuertasm Let's wait for the folder background to be fixed by JimiC.

And of course, I would fix the vscode2 icons to not use the insiders version and instead, provide them under a different key, like as vscode-insiders. If the vscode-team is not okay with that, I would just wipe them out and just keep the vscode ones.

Cheers!

@JimiC
Copy link
Member

JimiC commented Jun 17, 2019

I properly updated the vscode to the new icon.
@KingDarBoja I love the idea of adding the insiders version but now it's not the time to do it. I kept the icon though in case someone wants to use it via the customization feature. I intend (after the release of v9) to add a feature that detects the editor used and load the corresponding vscode icon. Until then we keep it as it is.

Copy link
Member

@robertohuertasm robertohuertasm left a comment

Choose a reason for hiding this comment

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

Great work @KingDarBoja! 🚀

@robertohuertasm robertohuertasm merged commit ba9e350 into vscode-icons:master Jun 23, 2019
@KingDarBoja KingDarBoja deleted the updated-vscode-icon branch June 23, 2019 18:27
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 Change] VSCode Icon
3 participants