Adding objectFit property #51
Conversation
There was a problem hiding this comment.
I think that some JS logic is missing regarding the image size handling based on objectFit. See this part of the code on react-easy-crop: https://github.com/ValentinH/react-easy-crop/blob/bba2b53bab1d409b719b9e48d156a7c35f8755bd/src/Cropper.tsx#L374-L385
In addition, why not also supporting cover?
|
Completely missed that part! I initially started on this to get a feel for the package. I have some time tonight, I'll take another look and also look at the cover option. |
|
@drijnkels any news on this? |
|
I spent a little time on it but got distracted by work. The cover option is added but I have a hard time understanding why renderedMediaSize is being recalculated everytime computeSizes is called. @ValentinH could you explain this? I'm not seeing any issues in the svelte version that is not doing the recalculation. |
In the react version, |
| @@ -17,6 +17,8 @@ | |||
| export let crossOrigin: HTMLImgAttributes['crossorigin'] = null | |||
| export let restrictPosition = true | |||
| export let tabindex: number | undefined = undefined | |||
| export let objectFit: ObjectFit = 'contain' | |||
| export let mediaObjectFit: ObjectFit = objectFit | |||
There was a problem hiding this comment.
I don't think mediaObjectFit should be exported as it's an property that is computed internally.
| @@ -29,6 +31,7 @@ | |||
| let rafDragTimeout: number | null = null | |||
| let rafZoomTimeout: number | null = null | |||
|
|
|||
| const imageObjectFitClass = {"contain": 'image_contain', "cover": "horizontal-cover", "horizontal-cover": "image_horizontal_cover", 'vertical-cover': "image_vertical_cover"}; | |||
There was a problem hiding this comment.
| const imageObjectFitClass = {"contain": 'image_contain', "cover": "horizontal-cover", "horizontal-cover": "image_horizontal_cover", 'vertical-cover': "image_vertical_cover"}; | |
| const imageObjectFitClass = {"contain": 'image_contain', "cover": "image_horizontal_cover", "horizontal-cover": "image_horizontal_cover", 'vertical-cover': "image_vertical_cover"}; |
| // const newZoom = zoom - (e.deltaY * zoomSpeed) / 200 // Can give 1.01 due to rounding errors | ||
| const newZoom = (e.deltaY > 0) ? zoom - (zoomSpeed / 2) : zoom + (zoomSpeed / 2); |
There was a problem hiding this comment.
I don't think that this change is intended as part of this PR.
Adds the objectFit property to the Cropper as requested in #14.
Similar implementation as the react-easy-crop version apart from the cover option. Also fixes a small JSDoc issue.