FEATURE: Adds pixel snapping support to aspect ratio cropping

[patricekaufmann]

Description: This aims to prevent cropping issues by cropping areas that will lead to rounding errors.

What I did

This change adds a new configuration option to the ImageEditor.

    image:
      ui:
        inspector:
          editorOptions:
            crop:
              aspectRatio:
                pixelSnapping: true

With this setting set to true, image cropping that is handled in the Ui will snap to pixels ranges. For example, it will no longer be possible to select a 170px x 96px crop area when the ratio is set to 16:9 as 16 is not a divider of 170.

Usually it would lead to rounding and sometimes losing pixels in the process. This is especially annoying when creating thumbnails in different resolutions as you would sometimes end up with 160px x 90px and 320px x 179px images.

Large values in custom aspect ratios, for example 400:300, are also supported as it gets calculated the same way as 4:3.

How I did it

Modify the response from react-image-crop by manipulating the values in cropArea before calling the onComplete callback passed to the ImageCropper component.

I also tried to implement onChange behaviour which worked nicely on CodePen with a react-image-crop at version 9. We're still using version 2 in here and the onChange callback is a bit buggy in version 2 so I skipped that part.

How to verify it

Crop an image with 16:9 aspect ratio and display the final resource with and height. You will get width and height values that are not multiples of the aspect ratio's values, for example 1922x1080 (When calculating paddings to reserve space for the image, you will end up with 56.21313% instead of 56.25% which could also break pixel perfect display for fixed 16:9 images.)

When cropping the image with snapping enabled, it now is 56.25% as expected. I attached a video to display the aspect ratio problem:

ufEPMFVFuN.mp4

[markusguenther]

Thanks for providing this improvement.
Resolved some linter issues. Will test soonish :)

[grebaldi]

Hi @patricekaufmann,

thanks a lot for this superb change! I was able to test it locally and it works like a charm 👌

I added one minor nitpicky comment, but other than that the code LGTM.

I would argue for setting the pixelSnapping option to true by default with the next major :)

[markusguenther]

Thanks @patricekaufmann, for your patients and the nice feature :)

[mhsdesign]

I would argue for setting the pixelSnapping option to true by default with the next major :)

i agree, is there any place where we dont want this?