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: added icon to language #186

Merged
merged 5 commits into from
Mar 8, 2022
Merged

feat: added icon to language #186

merged 5 commits into from
Mar 8, 2022

Conversation

JuanM04
Copy link
Contributor

@JuanM04 JuanM04 commented Mar 6, 2022

Changes

  • What does this change?
  • Be short and concise. Bullet points can help!
  • Before/after screenshots can be helpful as well.

Testing

Docs

@JuanM04 JuanM04 requested a review from a team as a code owner March 6, 2022 15:42
@changeset-bot
Copy link

changeset-bot bot commented Mar 6, 2022

🦋 Changeset detected

Latest commit: 308cda8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
astro-vscode Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Princesseuh
Copy link
Member

Good change! The code looks good to me, however there seem to be a problem with the alignment of the icon, at least on my setup?

image

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 7, 2022

I've taken the icon from here: vscode-icons/vscode-icons#2774

@antonyfaris
Copy link
Member

I've taken the icon from here: vscode-icons/vscode-icons#2774

The viewbox property has a different value. Maybe using the original size (32x32) will fix it?

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 7, 2022

@Princesseuh I've updated the icon, try now

@Princesseuh
Copy link
Member

Unfortunately doesn't seem to change anything:
image

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 7, 2022

Ok, I've found this icon, maybe this one works

@Princesseuh
Copy link
Member

Unfortunately, same thing with that one haha. Does it work fine on your side? Maybe it's an issue with my particular setup

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 7, 2022

I can replicate it but I don't know how to fix it. In only happens with the Seti theme, so maybe we could submit an icon there (?)

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 8, 2022

It seems that we aren't the only ones with alignment issues: microsoft/vscode#141335 (comment)

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

IMO a misaligned icon is better than no icon! I'd merge this and we can fix the alignment later?

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 8, 2022

@natemoo-re Let me do a quick try by removing the margins with figma. If it stays the same, I'll merge it

@JuanM04
Copy link
Contributor Author

JuanM04 commented Mar 8, 2022

Plot twist: it didn't work

@JuanM04 JuanM04 merged commit 2f87d79 into withastro:main Mar 8, 2022
@JuanM04 JuanM04 deleted the icon branch March 8, 2022 22:02
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.

4 participants