search

freedomofpress / securedrop

GitHub repository for the SecureDrop whistleblower platform. Do not submit tips here!
https://securedrop.org/
Other
3.62k stars 687 forks source link

Admin logo image upload: Resize images server-side if they are too large #2807

Open redshiftzero opened 6 years ago

redshiftzero commented 6 years ago

Description

We don't optimize uploaded images server-side so this makes for a slow experience for sources and journalists if a news organization elects to use a too large logo image. The large image may be transferred to the client which will then be resized to a smaller size anyway. It would make a lot more sense to do resizing server-side. What do we think (cc: @toast) about the following flow:

  1. Admin uploads image(s), which may or may not be the correct size.
  2. We resize the image(s) server-side and display them to the user. (we could use PyImageOptimizer as suggested in #2801 but since it looks like that dependency is not widely used / has not seen recent activity, and the functionality is simple I would advocate we implement this functionality in the application code ourselves).
  3. The uploaded image is shown, and admins can resubmit if they don't like how it has been resized.

We should avoid adding additional JavaScript such that the journalist interface degrades gracefully without js, so this means that we can't have the kind of image cropping interface we are used to from other websites.

Related issue: #2801

User Stories

As a SecureDrop source or journalist, I don't want to wait for a large logo image to unnecessarily download so that my Tor Onion Service browsing experience is faster.

kushaldas commented 6 years ago

Pillow (recomended) and ImageMagick are two options.

huertanix commented 6 years ago

I think this workflow works in lieu of using fancier JS workflows. The resize should keep the aspect ratio of the image so it doesn't look warped. I think it should resize the width down to the max width available, and let whatevs the resulting height is be the height of the resized image.

aydwi commented 6 years ago

@huertanix @kushaldas I agree with this. Image can be resized down to the constraint (250px square?) without changing the aspect ratio. A certain portion of the image can be cropped out before performing this operation if one dimension is very large than the other. I'm working on implementing this using Pillow

huertanix commented 6 years ago

I would caution against auto-cropping when no cropping tools are available in the UI for the user to fall back on if they don't like the auto-crop.

aydwi commented 6 years ago

That is a good point, but I was suggesting that suppose an image is too wide, cropping away some part of its extremes and then resizing. But it doesn't seem too necessary

kushaldas commented 6 years ago

@aydwi Any update on this?

aydwi commented 6 years ago

Yes, I wrote a script using Pillow to resize the image to a maximum dimension of 1000px, and crop before resizing which can improve the view in certain cases.

I could use some help with how to integrate this feature in a Flask based app.

aydwi commented 6 years ago

@kushaldas I've created a PR 😄