Skip to content
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

Add the preserveCurrentColor option to the removeAttrs plugin. #1000

Merged
merged 3 commits into from Feb 24, 2019

Conversation

roblevintennis
Copy link
Contributor

@roblevintennis roblevintennis commented Jul 11, 2018

Hello, I just coded this up as a follow up for the issue I just logged #999 .. I've added some explanations in my diff to hopefully make it easy to review.

Also, nothing output when I ran: npm run lint and npm test returns 307 passing (1s). Let me know if you like this and/or if there's any conventions I missed. Thanks for your time!


@@@

{"attrs":"(fill|stroke)", "preserveCurrentColor": true}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume that the falsy case is covered simply by the fact that: removeAttrs.01.svg removeAttrs.02.svg both pass. Also, if this diff isn't clear, I simply copied one of those and added the fill="currentColor" on line #11 above.


if(!(preserveCurrentColor && value === 'currentColor')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only potentially remove if we have NOT enabled this option AND the value is not currentColor. This is essentially same as what we did for svgstore: https://github.com/FWeinb/grunt-svgstore/pull/63/files#diff-239be4703ce423ab0e43053cdf58384bR176

@GreLI GreLI merged commit 0fe5368 into svg:master Feb 24, 2019
@GreLI
Copy link
Member

GreLI commented Feb 24, 2019

SVGO v1.2.0 has been just released!

@roblevintennis
Copy link
Contributor Author

Awesome @GreLI thanks! I can point back to SVGO proper 🙌

@roblevintennis roblevintennis deleted the removeAttrs-preserves-currentColor branch February 25, 2019 21:14
@roblevintennis roblevintennis restored the removeAttrs-preserves-currentColor branch February 25, 2019 22:10
@roblevintennis
Copy link
Contributor Author

I had to restore branch until my company (and probably me) can properly update our package.json and what not (it's currently pointed to my fork which I obviously want to change but need to find a spare cycle)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants