-
Notifications
You must be signed in to change notification settings - Fork 9.3k
Refactor IconPathConverter::IconWUX() to not set icon size #19806
Copy link
Copy link
Labels
Area-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Help WantedWe encourage anyone to jump in on these.We encourage anyone to jump in on these.Issue-TaskIt's a feature request, but it doesn't really need a major design.It's a feature request, but it doesn't really need a major design.Product-TerminalThe new Windows Terminal.The new Windows Terminal.
Milestone
Metadata
Metadata
Assignees
Labels
Area-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Help WantedWe encourage anyone to jump in on these.We encourage anyone to jump in on these.Issue-TaskIt's a feature request, but it doesn't really need a major design.It's a feature request, but it doesn't really need a major design.Product-TerminalThe new Windows Terminal.The new Windows Terminal.
Type
Fields
Give feedbackNo fields configured for Task.
I hit this problem pretty regularly whenever I'm working with icons (especially in the settings UI).
IconPathConverter::IconWUX()is a really powerful function that we use throughout the codebase to convert an icon path/code into an actual icon UI. However, we explicitly set the size of the icon on the way out. This was great when it was introduced because it meant that the icons in the dropdown, tabs, etc. all had the correct size. But as the project has grown to use those icons elsewhere (namely in the Settings UI), this has become a huge pain. Icons aren't always drawn to be the same size, especially if they're coming from different sources (i.e. hardcoded font icon vs evaluated icon in profile).I'd update it whenever I hit the problem, but it's realistically a fundamental enough change that it will have side effects. I'd rather have it be its own change and audit the app to make sure it doesn't break anything.
References
terminal/src/cascadia/UIHelpers/IconPathConverter.cpp
Line 318 in 3ec372c