Add aspect ratio support to the resize directive about imagetools HOT 11 CLOSED

I was going to propose that the API mirrored an existing popular one (like Cloudinary has).

Yep that makes total sense. It looks like the two main differences are that they have a shorthand ar for the aspect ratio parameter, and they value can be either a string like 16:9 or a float.

I should have a little time this morning, I'll add that in and submit another PR once it's tested

from imagetools.

Just opened a discussion for potentially adopting more ideas from services like Cloudinary!

from imagetools.

Hey @tonyfsullivan! Thanks for the idea, I definitely see the benefit. Two things I'm not sure about though:

1. Most people won't have the float representation of aspect ratios memorized and realizing that 1.7777 means 16:9 is a lot of mental gymnastics at a glance.

2. I'm unsure about the a new aspectRatio directive since it'd be mutually exclusive with width or height anyway, but I don't have a better idea right now anyway.

3. If we're going forward with the aspect directive, would the aspect ratio be calculated on based on the given size?
   Example:
   16:9 and a given width of 500 would mean resize the height to be 0.5625 times the width.
   Now say we have been given the height instead and an aspect ratio of 16:9. Would this mean resize the width to be 0.5625 times the height? Most people would find this confusing since 16:9 is associated with landscape and 9:16 with portrait images.

Maybe you have some idea regarding this!

from imagetools.

All excellent questions! Here's my take on each question given my use case, but there may very well be a different API to cover more scenarios

1. Totally agree that float values like 1.77777 aren't ideal. I hadn't thought through the exact implementation and got to a float just because I thought about doing something like ./image.png?w=1920&aspect=${16/9} but that could actually run into issues with dynamic impmort variables. The more common format of 16:9 might actually be a great API design for it as long as the colon character isn't a problem, ./image.png?w=1920&aspect=16:9
2. Agree here as well, a separate aspectRatio directive separate from resize isn't ideal. I was thinking aspect could be an extra optional parameter that the existing resize directive uses when both height and width aren't specified.
3. I should have included more details in my original issue here, sorry! At a minimum, I'd expect the resize directive to use aspect when only one dimension is given. I.e. width + aspect allows the resize directive to calculate height, and height + aspect allows it to calculate width.

Here's the use cases I can think of to help clarify what I had in mind, from input string to imageConfig objects

• ./image.png?width=1920&height=1080&aspect=4:3 -> aspect ignored, { width: 1920, height: 1080 }
• ./image.png?width=1920&aspect=16:9 or ./image.png?height=1080&aspect=16:9 -> { width: 1920, height: 1080 }
• ./image.png?width=960;1920&aspect=16:9 -> [{ width: 960, height: 540 }, { width: 1920, height: 1080 }]

One question would be if aspect could be used alone without height or width. I'd expect that to basically crop the image down as large as possible, deciding which axis to crop on by comparing the aspect ratio to the original photo's metadata from Sharp. This would be like saying "I don't care how big it is, just give me the photo in a 16:9 ratio."

from imagetools.

The more common format of 16:9 might actually be a great API design for it as long as the colon character isn't a problem, ./image.png?w=1920&aspect=16:9

Yeah that was my thinking too, I'll have a look at it later. Maybe the colon causes issues because it's used at the start of the url. But we'll see.

One question would be if aspect could be used alone without height or width. I'd expect that to basically crop the image down as large as possible, deciding which axis to crop on by comparing the aspect ratio to the original photo's metadata from Sharp. This would be like saying "I don't care how big it is, just give me the photo in a 16:9 ratio."

Yeah so that would be the only reason for having it as a separate directive. I like the possibility though, since it would make the position and fit directives more useful.

I'll have a look and do some tinkering later today, maybe it's just a simple fix. I'll post an update here as soon as I have more!

from imagetools.

Excellent, thanks! I'd be happy to take a stab at a fix here for a PR, I know there's an active next branch with some very cool work that may be more pressing.

I did a little digging and realized that Google actually uses colons in their query params for font imports. hopefully that means it shouldn't be an issue with Rollup or Vite either.

from imagetools.

This is very good news!
Using colons seems to be fine, from my testing and your research so that is great news!
If you're willing to take a stab you're more than welcome of course, my time is a bit limited currently so this would be really awesome!
The file that handles all the resizing business is https://github.com/JonasKruckenberg/imagetools/blob/main/packages/core/src/transforms/resize.ts
It's currently not all that elegant so if you need pointers or anything I'd be glad to help.

from imagetools.

This issue should be closed?

Seems I was a bit late to joining the discussion as the PR has already been merged. I was going to propose that the API mirrored an existing popular one (like Cloudinary has).

Looking over the PR I'm not sure how well that covers an existing project of mine where I used sharp directly to crop a region out of input images with a fit strategy and different aspect ratio to maintain before resizing.

If I ever get some time to port it to vite and try out the imagetools plugin I'll be sure to provide some feedback with examples (and perhaps a PR 😀 ).

from imagetools.

This issue should be closed?

In theory yes, though I have some issues with lerna (again 🙄) so it's not yet released. I'll close it once the version is out to npm!

I was going to propose that the API mirrored an existing popular one (like Cloudinary has).

This looks interesting and if I or anyone else has some time left this might be cool to add, should not be too difficult I think!

If I ever get some time to port it to vite and try out the imagetools plugin I'll be sure to provide some feedback with examples (and perhaps a PR 😀 ).

Sure!

from imagetools.

Should be very easy to archive as it's just a matter of changing the parseAspect function and adding the alias 👍🏻

from imagetools.

This feature has been shipped in [email protected], [email protected] and [email protected] respectively.

from imagetools.