-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Explicitly set foreground color to black in svg icons #26731
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
Explicitly set foreground color to black in svg icons #26731
Conversation
Ideally this change would have been made in a way that regenerating the icons with make_icons.py maintains the explicit 'style="fill:black"', but I see that backend_svg.py has a cutout for black text and explicitly saves some output by not outputting the color in that case, so I guess it's not really worth the trouble. @QuLogic do you know if this will interfere with gtk's "symbolic" icons though? |
I see the filter for black text in
I assume we don't want to change that. I was not familiar with |
I modified the script. It feels a little hacky since it performs a purely syntactic search and replace with no svg model underlying it, but I think it does the job. I tested it in my system where the svg backend produces very different icon code from what is currently committed, although the icons appear visually indistinguishable. So it should definitely work on systems that produce the svg code that is currently committed. |
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.
Subject to a confirmation/comment about the GTK aspect.
In case this turns out to be a problem for gtk, I guess the "base" files could not include the explicit foreground color but just have a xml marker-comment as to where the wx backend should insert the foreground color ( |
From my testing, it appears that this does not break GTK3 dark mode. GTK4 dark mode is already broken because it uses some other method that hasn't been implemented by us. |
@jmoraleda Can you squash? (Not compulsory.) |
…ipt adds style="fill:black;" to monochromatic icons
b7b8a15
to
3b643d4
Compare
Thanks for the PR! |
Currently the monochromatic svg icons do not explicitly specify foreground color, so they appear black, which is the svg default. This PR explicitly sets the foreground color of the monochromatic icons to black.
The rationale for this change is that explicitly setting the foreground color, enables changing the color through string replacement. In particular this permits adding dark-theme support in backends that use svg icons by doing a string replacement
fill:black;
->fill:white;
.