Add some padding for font icons #265
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
joadoumie
requested changes
Jan 6, 2025
Collaborator
joadoumie
left a comment
joadoumie
left a comment
There was a problem hiding this comment.
Looks good. Can take or leave my nits (ironically, I learned these nits from you on a few of my PRs).
src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.WindowsServices/Helpers/ServiceHelper.cs
Outdated
Show resolved
Hide resolved
src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.WindowsServices/Helpers/ServiceHelper.cs
Outdated
Show resolved
Hide resolved
src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.WindowsServices/Helpers/ServiceHelper.cs
Outdated
Show resolved
Hide resolved
Owner
|
|
joadoumie
approved these changes
Jan 9, 2025
zadjii-msft
added a commit
that referenced
this pull request
Jan 17, 2025
When we changed icons to be a pair of IconData in an IconInfo, I forgot this type check. Oops. Now it works again. Originally in #265 Regressed in #286 (targets #308, which targets #299) `IconData` and `IconInfo` are unfortunately, not trivially marshallable into the host process. This creates a collection of cases where an app can crash and take the host down, because we'll try to inquire something about the icon. It broke reloading when an extension crashed. It broke backing out of a crashed extension. Previously: #218 Closes #235
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Random nits, part the first.
Emoji render just a little too big currently, and they get clipped.
This PR fixes that, by making emoji icons have just a bit of negative padding, to give them space to not get clipped.
Is also replaces the icons on the Services page, because the icon on the list items was the icon of the thing to do to the service (i.e. running services would have a "stop" icon, which is insane). Of course, this made the emoji problem much more apparent.