-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Allow disabling the icon background #310
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
base: main
Are you sure you want to change the base?
Conversation
TODO docs, waiting for approval that this approach makes sense |
@jeromelaban can you confirm this approach works? |
This approach is compatible with #239 |
@@ -50,6 +50,8 @@ internal class ResizeImageInfo | |||
|
|||
public bool IsAppIcon { get; set; } | |||
|
|||
public bool UseBackgroundFile { get; set; } = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test with a validation for this part?
@MartinZikmund I feel that an additional property is just highlighting an issue with the original implementation and I feel that we should be able to just use the two svg files provided. In my mind the foreground svg is the icon itself and on platforms that support a transparent background only this file would be used (if developer wants a background even on those platforms they can add this to the foreground svg). On platforms that don't support a transparent background, this is where the background svg would be used. Does this make sense, or are there scenarios I'm not thinking about |
Part of: #190
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently the user must create a new
.svg
file likeicon_transparent.svg
that is empty and provide it as theUnoIconBackgroundFile
- as leaving this empty will fallback toicon.svg
.What is the new behavior?
New attribute allows setting
UseIconBackground
tofalse
, which will not use the background file while generating, and will keep the foreground file only instead.I made a follow up PR unoplatform/uno#17855 in Uno.UI to add a prop to control this and then default this to
false
on WASM, desktop and WinUI, where users most commonly don't want a background behind the icon.PR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Run
results.Other information
Internal Issue (If applicable):