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

Disable SVG optimization by default #145

Merged
merged 5 commits into from
Aug 17, 2023

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Aug 16, 2023

Description of the Change

In #79, we added SVG optimization using the SVGO library. This optimization is enabled by default and the only way to turn it off was through a filter. I had some concerns with this feature (around performance and potential issues with stripping params we may want to keep) and so in order to not delay a 2.2.0 release any longer, talked to @jeffpaul and decided that we would keep this feature in place but we would change this to be disabled by default.

A new filter has been added (safe_svg_optimizer_enabled) that returns false by default. Users that want to try out this new optimization feature will have to hook into that filter and return true:

add_filter( 'safe_svg_optimizer_enabled', '__return_true' );

This gives people the opportunity to test this out and decide whether they want this extra optimization or not (and doesn't impact sites that don't care about this). If enough people use and like this feature, in the future we can look to either enable this by default or add an admin setting to turn this on/off.

How to test the Change

Follow the steps outlined in #79, once you add the line of code above. Without that line in place, ensure optimization doesn't happen and everything works as expected.

Changelog Entry

Changed - Don't run SVGO optimization by default and instead make this an opt-in feature.

Credits

Props @dkotter, @jeffpaul

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@dkotter dkotter added this to the 2.2.0 milestone Aug 16, 2023
@dkotter dkotter self-assigned this Aug 16, 2023
@dkotter dkotter requested a review from a team as a code owner August 16, 2023 22:18
@dkotter dkotter requested review from peterwilsoncc, a team and ravinderk and removed request for a team and peterwilsoncc August 16, 2023 22:18
@dkotter dkotter mentioned this pull request Aug 16, 2023
19 tasks
Copy link
Contributor

@ravinderk ravinderk left a comment

Choose a reason for hiding this comment

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

@dkotter It is working as expected.

@dkotter dkotter merged commit aa2caf5 into develop Aug 17, 2023
11 checks passed
@dkotter dkotter deleted the feature/disable-optimizer-by-default branch August 17, 2023 13:55
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.

2 participants