Skip to content

Reworking Resizing / Cropping#139

Merged
colbyfayock merged 6 commits intobetafrom
beta+feat/width-resize
Feb 15, 2024
Merged

Reworking Resizing / Cropping#139
colbyfayock merged 6 commits intobetafrom
beta+feat/width-resize

Conversation

@colbyfayock
Copy link
Collaborator

@colbyfayock colbyfayock commented Jan 25, 2024

Description

This reworks cropping and resizing to support a bigger effort to correctly apply these transformations at the right point within the transformation string.

The current mechanism applies the core width and height to the front of the transformation string, with the idea of providing a "canvas" for someone to work from, but this is unintuitive, where a width and height would be the expected final resize, which may not be the case with other transformations (particularly dynamically generated responsive images).

This removes the widthResize property which applied the transformation the end, where the core width and height resizing now takes place.

To fill in for use cases where it made sense to apply a "base" cropping and resizing effect, a new base* property was added for width, height, crop, gravity, and zoom, to accommodate the situation where one might want to resize, apply transformations, then obtain the true width and height with a final resize as part of a 2-stage crop and resize.

In the example where

Somewhat of a tl;dr
This PR primarily reverses the order in which the parameters are applied, opting for the primary width and height to be the final transformation applied to avoid attempting to more accurately describe what the user's intent is when passing in a width and height. As such, the name of the options for the 1st stage is now base*

Examples

Previously when using an effect with a width:

constructCloudinaryUrl({
  options: {
    width: 100,
    grayscale: true
  }
})

The resulting transformation would yield: c_limit,w_960/e_grayscale

However, it would now yield: e_grayscale/c_limit,w_960

This is more relevant when using the 2-stage resizing, where previously:

constructCloudinaryUrl({
  options: {
    width: 100,
    widthResize: 200
  }
})

The resulting transformation would yield: c_limit,w_100/c_limit,w_200

Where now:

constructCloudinaryUrl({
  options: {
    width: 100,
    baseResize: 200
  }
})

The resulting transformation would yield: c_limit,w_200/c_limit,w_100

Use Case

A big use case around why this is important is limiting the size of an image prematurely.

In an example such as:

constructCloudinaryUrl({
  options: {
    width: 100,
    widthResize: 200
  }
})

The resulting transformation would yield: c_limit,w_100/c_limit,w_200

This is problematic if the original image is greater than or equal to 200, as with the first transformation applied, the image will not be able to be sized organically above 100. If using a crop that allows upscaling, it would not result in using the original image, meaning it would scale to 200 with reduced quality.

The effect is largely the same in what it achieves, but the use case and intent should be more accurate with this PR.

Relevant: cloudinary-community/next-cloudinary#327

@vercel
Copy link

vercel bot commented Jan 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cloudinary-util ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 5:21pm

Copy link

@Baroshem Baroshem left a comment

Choose a reason for hiding this comment

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

Few questions from my side.

Also, do you expect this change to cause some breaking changes?

Copy link

@Baroshem Baroshem left a comment

Choose a reason for hiding this comment

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

Nicely done!

@colbyfayock colbyfayock merged commit 78a6f49 into beta Feb 15, 2024
@colbyfayock colbyfayock deleted the beta+feat/width-resize branch February 15, 2024 15:50
@colbyfayock colbyfayock mentioned this pull request Feb 15, 2024
colbyfayock added a commit that referenced this pull request Feb 15, 2024
This reworks cropping and resizing to support a bigger effort to
correctly apply these transformations at the right point within the
transformation string.

The current mechanism applies the core width and height to the front of
the transformation string, with the idea of providing a "canvas" for
someone to work from, but this is unintuitive, where a width and height
would be the expected final resize, which may not be the case with other
transformations (particularly dynamically generated responsive images).

This removes the widthResize property which applied the transformation
the end, where the core width and height resizing now takes place.

To fill in for use cases where it made sense to apply a "base" cropping
and resizing effect, a new `base*` property was added for width, height,
crop, gravity, and zoom, to accommodate the situation where one might
want to resize, apply transformations, then obtain the true width and
height with a final resize as part of a 2-stage crop and resize.

In the example where

**Somewhat of a tl;dr**
This PR primarily reverses the order in which the parameters are
applied, opting for the primary width and height to be the final
transformation applied to avoid attempting to more accurately describe
what the user's intent is when passing in a width and height. As such,
the name of the options for the 1st stage is now `base*`

**Examples**

Previously when using an effect with a width:
```
constructCloudinaryUrl({
  options: {
    width: 100,
    grayscale: true
  }
})
```

The resulting transformation would yield: `c_limit,w_960/e_grayscale`

However, it would now yield: `e_grayscale/c_limit,w_960`

This is more relevant when using the 2-stage resizing, where previously:

```
constructCloudinaryUrl({
  options: {
    width: 100,
    widthResize: 200
  }
})
```

The resulting transformation would yield: `c_limit,w_100/c_limit,w_200`

Where now:

```
constructCloudinaryUrl({
  options: {
    width: 100,
    baseResize: 200
  }
})
```
The resulting transformation would yield: `c_limit,w_200/c_limit,w_100`

**Use Case**

A big use case around why this is important is limiting the size of an
image prematurely.

In an example such as:
```
constructCloudinaryUrl({
  options: {
    width: 100,
    widthResize: 200
  }
})
```

The resulting transformation would yield: `c_limit,w_100/c_limit,w_200`

This is problematic if the original image is greater than or equal to
200, as with the first transformation applied, the image will not be
able to be sized organically above 100. If using a crop that allows
upscaling, it would not result in using the original image, meaning it
would scale to 200 with reduced quality.

The effect is largely the same in what it achieves, but the use case and
intent should be more accurate with this PR.

Relevant:
cloudinary-community/next-cloudinary#327

BREAKING CHANGE: Removes widthResize in favor of width, baseWidth added to replace original width application
github-actions bot pushed a commit that referenced this pull request Feb 15, 2024
# [@cloudinary-util/url-loader-v5.0.0-beta.1](https://github.com/colbyfayock/cloudinary-util/compare/@cloudinary-util/url-loader-v4.2.0...@cloudinary-util/url-loader-v5.0.0-beta.1) (2024-02-15)

* Reworking Resizing / Cropping (#139) ([89bf6ab](89bf6ab)), closes [#139](#139)

### BREAKING CHANGES

* Removes widthResize in favor of width, baseWidth added to replace original width application
colbyfayock added a commit that referenced this pull request Feb 26, 2024
# Description

* Reworks cropping and resizing to support a bigger effort to correctly
apply these transformations at the right point within the transformation
string. #139

BREAKING CHANGE: Removes widthResize in favor of advanced crop prop
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