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 support for resizing images #60

Open
kaycebasques opened this issue Aug 21, 2020 · 11 comments
Open

Add support for resizing images #60

kaycebasques opened this issue Aug 21, 2020 · 11 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kaycebasques
Copy link

kaycebasques commented Aug 21, 2020

On web.dev we never need images larger than 1600px. I frequently have to ask our contributors to resize their images (in order to keep the repository size down). Are you open to adding image resizing to image-actions or should we look for other solutions?

Edit: We never need images within our main content to be more than 1600px. There are a few exceptions we'd need to specify but I can share more details later.

@crstauf
Copy link

crstauf commented Aug 21, 2020

Related #20.

@benschwarz
Copy link
Member

benschwarz commented Aug 22, 2020

Hey @kaycebasques, actually — yes!

Since this was suggested back on #20, I've wondered how it could best work.

In my mind, it'd be great to be able to specify a directory, or path glob, along with resize settings. Some examples of that might be:

  • Resize all images inside /images/avatars to 200x200
  • Resize any image inside /blog/*/header*.jpg to 800x300

Something that gives me pause is being able to specify how cropping and other image resizing operations might be specified. However this gets implemented, my primary goal is to keep any required configuration super simple… but I feel we'll still be able to cover most common scenarios.

What do you think of all that?

@tunetheweb
Copy link
Contributor

tunetheweb commented Aug 22, 2020

Think when you get into cropping you're asking for trouble.

One thing I'd like is just to ensure no one uploads a print quality 6000 x 3000 image when a 600 x 300 would do!

What about a max-width per folder?

E.g.

  • * - 600px
  • images/banners - 1600px
  • images/fullsize - 0
  • images/**.XL.jpg - 0

Or:

max-width_default: "600"
max-width_custom:
 1600:
  - "images/banners/"
max-width_exclusion:
  - "images/fullsize"
  - "images/**.XL.jpg"

Os something like that.

This would scale all images to 600px max width (with the appropriate height would be chosen to maintain aspect ratio) by default, however images/banners/banner1.jpg would instead be scaled to maximum of 1600px width, and images/fullsize/banner1.jpg would not be scaled at scaled at all and neither would images/folder/subfolder/photo.XL.jpg.

@crstauf
Copy link

crstauf commented Aug 22, 2020

An option to comment-only or keep the PR from merging if an image is found larger than max width would be nice.

Personally I'm hesitant to auto-resize images, but definitely see value in automating alerts and requests to contributors.

@kaycebasques
Copy link
Author

Thanks for looking into this. Sorry for missing #20 (I recall searching for related issues… don't know how I missed that).

Here's our use case for web.dev:

  • Our images are stored alongside our Markdown. E.g. the content for https://web.dev/optimize-fid is at /src/site/content/en/blog/optimize-fid/index.md and its hero image is at /src/site/content/en/blog/optimize-fid/hero2.jpg and one of the main content images that the Markdown references is at /src/site/content/en/blog/optimize-fid/lighthouse.jpg.
  • For hero images we always need those to be 3200px wide and 960px tall. At minimum, image-actions should allow us to specify that it should ignore hero images, perhaps with a glob like /src/site/content/**/hero.jpg (we don't currently enforce that all hero images should have the filename hero.jpg but we can start).
  • For main content images we just want to enforce a maximum width of 1600px but we want the height to remain proportional, e.g. if the original image is 3200px wide and 1600px tall it should be resized to 1600px wide and 800px tall.
  • Ideally image-actions would replace the main content images in-place, rather than just warning that the images are too large. Our goal here is to minimize the work that our contributors need to do. Having to manually resize images is one of our contributors' top gripes ("this seems like it should be automated…").

@bazzadp's proposed config would probably work for us. Here's my first impression of an intuitive config:

max-width: 
  default: 1600
  input:
    - '/src/site/content/**'
  ignore:
    - '/src/site/content/**/hero.jpg' # or perhaps just 'hero.jpg'

I'm assuming it would automatically detect common image filename extensions.

@tunetheweb
Copy link
Contributor

@kaycebasques alternatively if you also wanted to enforce the 3200px maximum width for hero images you could have this config like this under my proposal:

max-width: 
  default: 1600
  input:
    - '/src/site/content/**'
  custom:
    3200: 
      - '/src/site/content/**/hero.jpg' # or perhaps just 'hero.jpg'

That is provide a 3200 setting rather than ignoring it completely. Wouldn't enforce a 960px tall though and maybe overkill for your needs but throwing it out there.

@tunetheweb
Copy link
Contributor

tunetheweb commented Sep 9, 2020

What would also be really cool (maybe as a stage two) would be to create alternative sizes/formats for scrset or picture elements:

alternatives:
  input:
    - **XL.jpg
  alt1:
    name: LG
    format: jpg
    width: 1600px
  alt2:
    name: MD
    format: jpg
    width: 800px
  alt3:
    name: SM
    format: jpg
    width: 400px
  alt4:
    name: LG
    format: png
    width: 1600px
  alt5:
    name: MD
    format: png
    width: 800px
  alt6:
    name: SM
    format: png
    width: 400px

So if you fed it a /src/site/content/article1/hero-XL.jpg it would automatically create the following images:

  • /src/site/content/article1/hero-LG.jpg
  • /src/site/content/article1/hero-MD.jpg
  • /src/site/content/article1/hero-SM.jpg
  • /src/site/content/article1/hero-LG.png
  • /src/site/content/article1/hero-MD.png
  • /src/site/content/article1/hero-SM.png

@kaycebasques
Copy link
Author

@bazzadp that config for enforcing 3200px for the hero would work for us (probably)

I think I've represented our use case and requirements accurately but at this point I should CC @robdodson (our TL)

@robdodson
Copy link

I think as a start being able to request that all images that match a particular glob path can be proportionally resized would be good.

@benschwarz benschwarz added enhancement New feature or request help wanted Extra attention is needed labels Sep 14, 2020
@benschwarz
Copy link
Member

benschwarz commented Sep 14, 2020

Thanks for sharing your config ideas @kaycebasques & @bazzadp. Unfortunately GitHub Actions doesn't support array objects so we'd need to come up with another way to specify configurations.

In general, I really like where this discussion is headed. Thanks for sharing your ideas 😎

What would also be really cool (maybe as a stage two) would be to create alternative sizes/formats for scrset or picture elements:

Yes! How great would that be!??

Maybe transcoding isn't something you'd want actively committed to a repo, but instead used in a more advanced pipeline that optimised images then packages/deploys in a following build step? 🤔 Either way. Sounds like a great addition.

@petems
Copy link

petems commented Sep 19, 2020

Hah, I was literally building something myself for this and realized that the awesome work in this action would be perfect for a few tweaks to reszing (Since sharp already has a super in-depth resizing feature: https://sharp.pixelplumbing.com/api-resize)

I was doing it in-place with mogrify: https://github.com/petems/img-resizer-inplace-limit

I was going to fork and hack it together myself, but if I can help in any way for this I'd be interested! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Development

No branches or pull requests

6 participants