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

feat: Add C3 image #2655

Merged
merged 12 commits into from
Nov 9, 2024
Merged

feat: Add C3 image #2655

merged 12 commits into from
Nov 9, 2024

Conversation

StandUp2001
Copy link
Contributor

Description

Add a new icon for c3 in fileIcons array in src/core/icons/fileIcons.ts

Contribution Guidelines

@github-actions github-actions bot added the 🏞️ icons PR with new icons label Oct 31, 2024
Copy link
Contributor

github-actions bot commented Oct 31, 2024

Preview

Thank you for creating a pull request. This preview shows you how your icon will look on the different themes:

Generated preview

Check how your icon fits in a 16x16 grid with our Pixel Perfect Checker by following this link.

You can find more information on how to contribute in the contribution guidelines.

@StandUp2001 StandUp2001 changed the title Add C3 image feat: Add C3 image Oct 31, 2024
Copy link
Member

@lucas-labs lucas-labs left a comment

Choose a reason for hiding this comment

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

Thanks for the pr!

If vscode integration exists for c3 (e.g. an extension that adds color highlighting, etc), they surely defined a languageId for the c3 language.

So, we should also add the mapping to src/core/icons/languageIcons.ts

icons/c3.svg Show resolved Hide resolved
@StandUp2001
Copy link
Contributor Author

Thanks for the pr!

If vscode integration exists for c3 (e.g. an extension that adds color highlighting, etc), they surely defined a languageId for the c3 language.

So, we should also add the mapping to src/core/icons/languageIcons.ts

Yeah they have one but not officially in vscode yet. So don't know if you already want it in the languageIcons.ts or later when it is out to the extension tab?
C3 VSCode

@StandUp2001
Copy link
Contributor Author

StandUp2001 commented Nov 3, 2024

Sorry for all those commits. At this moment I think I am done with it.
Pls let me know if otherwhise

@PKief
Copy link
Member

PKief commented Nov 4, 2024

From my point of view the "3" is too thin compared with the "c". Personally, I don't like the current look and feel - as it's not even aligned.

image

Can we try to bring the same styling to both the "c" and the "3" here?

@PKief
Copy link
Member

PKief commented Nov 8, 2024

I've created this one similar to the C-Sharp icon:

c3

@okineadev
Copy link

I've created this one similar to the C-Sharp icon:

c3

It looks more like a than a C3

Better to just change the thickness of the number

@lucas-labs
Copy link
Member

Yeah, the only way to fit both the C and the 3 is to not use left and right margins. IMO, this shouldn't be an issue here, at least as an exception. I find bottom and top margins more important, as they help to avoid icons to be too close to each other when displayed vertically in the file tree.

The last version by @StandUp2001 does that (almost not using horizontal margins) and it look pretty good IMO (although, it's not pixel perfect, but that can be fixed). What do you guys think?

image

@okineadev
Copy link

image

Let there be this icon

@lucas-labs
Copy link
Member

lucas-labs commented Nov 9, 2024

I aligned it to the grid. As I said, it lacks horizontal margins in favor of a better shape.

image

Here's how it looks next to the rest of the family:

image

@lucas-labs lucas-labs requested review from PKief and okineadev November 9, 2024 04:32
@PKief PKief merged commit b0be8b6 into material-extensions:main Nov 9, 2024
5 checks passed
Copy link
Contributor

github-actions bot commented Nov 9, 2024

Merge Successful

Thanks for your contribution! 🎉

The changes will be part of the upcoming update on the Marketplace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏞️ icons PR with new icons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants