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

Fix icon sizing for themes with custom font-sizes #242257

Merged
merged 2 commits into from
Feb 28, 2025

Conversation

mjbvz
Copy link
Collaborator

@mjbvz mjbvz commented Feb 27, 2025

No description provided.

@mjbvz mjbvz self-assigned this Feb 27, 2025
@mjbvz mjbvz added this to the February 2025 milestone Feb 27, 2025
@mjbvz mjbvz requested a review from joyceerhl February 27, 2025 23:33
Copy link

⚠️ This PR originates from a fork. Due to security restrictions, pipelines from forks are no longer triggered automatically. Learn more.

If the changes appear safe, you can manually trigger the pipeline by commenting /AzurePipelines run.

Copy link
Collaborator

@joyceerhl joyceerhl left a comment

Choose a reason for hiding this comment

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

Screenshot 2025-02-27 at 3 39 17 PM

The default theme icons are tiny 🤏

@mjbvz
Copy link
Collaborator Author

mjbvz commented Feb 28, 2025

Good call. I don't think we can get the icons always looking consistent but here's what it should look like after a few more tweaks:

Screenshot 2025-02-27 at 4 27 23 PM

This theme defines its icons to be on the larger size. They even look a little oversized to me in the explorer. I've fixed this to be more reasonable and not overflow the widget but if I force these icons to be a better looking size, a bunch of our other icon themes break

Copy link
Collaborator

@joyceerhl joyceerhl left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a ton!

@mjbvz mjbvz merged commit cad9007 into microsoft:main Feb 28, 2025
7 checks passed
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.

2 participants