-
Notifications
You must be signed in to change notification settings - Fork 119
Optionally replace stroke color #68
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
default behaviour remains unchanged
@anncwb Hi - Please could you run this trivial update through the workflow for a patch update |
@anncwb I need this too. Can you take a look at this pr? Thanks. |
+1 |
I believe this is not in the scope of this plugin. The expected logic should be handled by an SVGO plugin |
Thanks for the reply - it seems colours are replaced by this module /after/ the SVGO stage. How do you propose this is solved by the SVGO options? |
I have published a forked version which supports disabling the replacement of the stroke colour. |
This should be merge |
1 similar comment
This should be merge |
content = content.replace(/stroke="[a-zA-Z#0-9]*"/, 'stroke="currentColor"') | ||
if (options.replaceStrokeWithCurrentColor) { | ||
// fix cannot change svg color by parent node problem | ||
content = content.replace(/stroke="[a-zA-Z#0-9]*"/, 'stroke="currentColor"') |
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.
You can replace multiple strokes at once instead of just one.
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.
Please use the regular expression /\bstroke="[^"]*"/gi to perform the replacement.
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.
This seems like a sensible change, however my intention in the PR is to have the replacement optional, and retain all existing functionality as-is. I don't want to increase the scope in this PR as it has been waiting to be merged for almost 3 years.
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.
I completely understand your intention to keep this PR focused. It's unfortunate that this project seems to have stalled in maintenance. For practical progress, I've chosen to propose these changes in another contributor's active PR while maintaining full compatibility with existing implementations.
This is annoying, been playing with svgo options and it drove me mad, only to find out the issue was in this plugin all along. |
Hey @anncwb, could you please take a moment to review that MR and merge it? If there are any additional changes or adjustments needed, I'm more than willing to collaborate further to ensure its seamless integration into the project. I hope @digitalacorn also could help. Here is a little example of inconsistency, that the default behavior of the plugin produces: in the USA SVG flag, it makes white stripes black. And it's not possible to fix it, rather than passing Black USA flag (the stroke was replaced with Proof that not only white color is used as a stroke: Is there any chance you can merge the changes and deploy a new version of your plugin? |
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.
Please use the regular expression /\bstroke="[^"]*"/gi to perform the replacement.
Expose an option to enable or disable the replacement of the stroke colors within the SVG content.
Default behaviour remains the same