sekoyo / react-image-crop

A responsive image cropping tool for React
ISC License
3.77k stars 339 forks source link

Cropped area not accounting for zoom of containing elements properly #591

Open c0d3ster opened 1 month ago

c0d3ster commented 1 month ago

Bug Description: I am using ReactCrop as an element/node on a React Flow canvas. The React Flow library allows the entire canvas area to be zoomed on scroll. Whenever the zoom !== 1, it leaves me unable to crop the larger than zoom (when < 1) or able to crop larger than the image (when > 1). I am also scaling the image down from naturalWidth to a set max width with CSS, but the completed crop is properly being calculated so shouldn't be any issues there.

Screenshot 2024-05-29 at 11 02 44 AM

Expected Behavior: The croppable area selection should always be a maximum of the width and height of the image.

Actual Behavior: The croppable area selection is only able to go to a maximum of width * zoom or height * zoom. If I try to manually divide by the zoom value during setCrop it causes another issue where the crop area expands while moving the selection.

Minimum Reproducible Example A minimum reproducible example can be created by adding transform: scale(50%) to the div containing the ReactCrop component then trying to move the crop selection. the selection will only be able to crop up to 50% in the top left of the image

Possible Solutions There seems to be an issue with the containCrop or clamping logic within the resizeCrop function, so I'm wondering if we can either calculate any zoom value internally using window/document properties or if we can pass in a prop for zoom. This would be different from the scale prop because I still want the image to display without any whitespace and at 1.0 scale.

Ericxgao commented 1 month ago

I am also seeing this issue.

sekoyo commented 3 weeks ago

I used to have a zoom prop, perhaps something like that can fix it 🤔

/** A non-visual prop to keep pointer coords accurate when a parent element is scaled.
Not to be confused with the `scale` prop which scales the image itself. Defaults to 1. */
zoom?: number

https://github.com/sekoyo/react-image-crop/blob/9.1.1/src/ReactCrop.tsx#L358

https://github.com/sekoyo/react-image-crop/blob/9.1.1/src/ReactCrop.tsx#L436

Are you able to make an example of the issue on React Flow (haven't used or heard of it before)?

c0d3ster commented 3 weeks ago

React Flow is a library used for node based workflows. The way the zoom works under the hood is utilizing transform: scale with CSS, so you can just wrap a ReactCrop element in a scaled div to see the same effect. Here is the result of doing that with the CodeSandbox example: https://codesandbox.io/p/sandbox/react-image-crop-demo-with-react-hooks-forked-with-zoomed-out-container-gqjwpg

It would be possible to make a CodeSandbox with React Flow in particular, but that would require more boilerplate since you need to define all your node types. In my current project, the behavior is slightly different than the example above as the cropped output is accurate to the crop area visually. It just won't let you set the crop area to larger than the zoom amount. In other words, the screenshot I attached above is the maximum selectable area. I believe bringing that zoom prop back would do the trick in both cases though.

sekoyo commented 3 weeks ago

Sorry I misread - looks like it's only an issue with the crop preview.

So you can modify it like:

async function canvasPreview(
  image: HTMLImageElement,
  canvas: HTMLCanvasElement,
  crop: PixelCrop,
  scale = 1,
  rotate = 0,
  appScalePc = 100,
) {
  const ctx = canvas.getContext('2d')

  if (!ctx) {
    throw new Error('No 2d context')
  }

  const scaleX = (image.naturalWidth / image.width) * (100 / appScalePc)
  const scaleY = (image.naturalHeight / image.height) * (100 / appScalePc)

  // ....
}

And if your transform: 'scale(50%)' is 50%, then pass 50 to appScalePc

Ericxgao commented 2 weeks ago

Would it still be possible to expose that zoom prop?

Ericxgao commented 2 weeks ago

I fixed this with the following subclass - would it be possible to expose a scale or zoom parameter as it seems like it does this?

interface ScaleCompatibleReactCropProps {
  scale?: number
}

class ScaleCompatibleReactCrop extends ReactCrop {
  props: ReactCropProps & ScaleCompatibleReactCropProps

  getBox() {
    const box = super.getBox()
    const { scale } = this.props

    if (scale !== undefined) {
      box.width /= scale
      box.height /= scale
    }

    return box
  }
}