linuxfoundation / lfevents

For the Linux Foundation Events website events.linuxfoundation.org
MIT License
20 stars 4 forks source link

WP SVG Autocrop plugin #455

Closed cjyabraham closed 4 years ago

cjyabraham commented 4 years ago

CNCF has developed SVG Autocrop which optimizes SVG files and trims them to have just a 1 pixel border, among other things. Our editors are in the habit of optimizing every SVG file we upload to LFEvents using SVG Autocrop.

We would like to create a WP plugin which automatically performs this service during upload of SVGs by leveraging the SVG Autocrop API. The plugin will eventually run on cncf.io, the LFEvents sites, and we'd like to release it as a free plugin in the WP plugin directory.

Currently, we use the free Safe SVG plugin which sanitizes the SVG files on upload. The Pro version of the plugin runs the SVGs through an SVGO Optimizer. Both versions offer the ability to hook in our own optimization service using the wp_handle_upload_prefilter hook. So we want to build a plugin that has the Safe SVG plugin as a dependency and uses its hook to run the SVG Autocrop operation on every SVG upload.

A particularly important feature is propagating errors back to the user. If an SVG includes a PNG or JPEG, autocrop fails with an error. It will also fail if the SVG includes a text or tspan tag, meaning that it is including fonts. Also, please disallow uploads if the autocrop service is down. It is running as a serverless app on Google Cloud Functions, however, so should have good uptime.

Develop a Minimum Viable Product for an initial release. From there, we can add on more advanced features.

We will eventually store the code in the CNCF WP SVG Autocrop repo but, for now, just develop it on a branch within the LFEvents repo.

rodmeurer commented 4 years ago

Done @cjyabraham

PR: https://github.com/LF-Engineering/lfevents/pull/457

We activated and tested the plugin at this environment: https://pr-457-lfeventsci.pantheonsite.io

cjyabraham commented 4 years ago

Nice work! A few comments:

Is that coming from the API so needs to be extended there? This is a good test case for a complex svg.

cjyabraham commented 4 years ago

@dankohn a few questions:

  1. If the svg has a jpg/png embedded, should we still allow the upload even though SVG Autocrop can't process it?
  2. Should we still complete the upload on any other error from autocrop? I'm weary of an editor getting stuck one day because they can't get an SVG to upload.
  3. Should we allow the user to explicitly skip the autocrop process? I wonder whether Megan would sometimes want to keep whatever pixel border she has on a given SVG.
dankohn commented 4 years ago
  1. No, we want to reject it. Those SVGs look blurry at high resolution, which eliminates the advantage of using an SVG.
  2. No, then people will disregard. I will rewrite https://github.com/cncf/landscapeapp#images into some sections on the Readme and we can send people there with a link from the error message.
  3. No, the 1 pixel border should always be there. People could always disable the plugin, but if it;s being used, it should autocrop with a 1 pixel border.

I will work on FAQ sections for the Readme.

cjyabraham commented 4 years ago

Ok, then sounds like the way errors are being handled is good for now. @dankohn are you aware of a timeout of 10s in the API?

dankohn commented 4 years ago

This is being addressed on https://github.com/cncf/landscapeapp/issues/511

dankohn commented 4 years ago

@rodmeurer Please create a link from the error message indicating an embedded PNG or JPG to https://github.com/cncf/wp-svg-autocrop#why-cant-my-svg-include-a-png-or-jpg. And please create a link to the error message about tspan to https://github.com/cncf/wp-svg-autocrop#why-cant-my-svg-include-text-or-tspan-tags.

cjyabraham commented 4 years ago

Due to the 1pixel border, deploying this is dependent on this issue.

rodmeurer commented 4 years ago

@cjyabraham @dankohn done.

image

cjyabraham commented 4 years ago

Thanks. I've tweaked a few things to increase the curl timeout and handle another error message: "SVG file embeds a png". There's another issue that I'm tracking with this but I think its cause is upstream.

cjyabraham commented 4 years ago

Waiting to discuss with Megan before I deploy.

cjyabraham commented 4 years ago

As per @dankohn, release plugin on WP directory with this text:

This is a free service but if you will be autocropping more than a thousand SVGs a month, please create your own autocrop server using the code from https://github.com/cncf/svg-autocrop-serverless. If you’re using the service we would appreciate if you would add a link to back to cncf.io as a thank you.

cjyabraham commented 4 years ago

I've submitted the plugin for inclusion in the WP plugin dir.

cjyabraham commented 4 years ago

Plugin now in the WP directory and stored in our github. @rodmeurer let me know if you have a wordpress.org user you'd like me to include in the Contributors section at the bottom.

What's left is for me to activate on the live site.

rodmeurer commented 4 years ago

@cjyabraham you can add this one please https://profiles.wordpress.org/fuerzastudio/

Thank you.

cjyabraham commented 4 years ago

I've added you as a committer and a support rep to the plugin. I'm guessing you have to actually make a commit to the svn repo in order to show up in the "Contributors & Developers" section. Note I have also linked to fuerzastudio in the copy and the plugin is listed as "By fuerzastudio" at the top.

cjyabraham commented 4 years ago

486 is currently blocking deployment of this plugin

cjyabraham commented 4 years ago

this is now activated on the live site