ngImgCrop icon indicating copy to clipboard operation
ngImgCrop copied to clipboard

Aspect ratio, crop data, original image data, and fix for hidden canvas

Open iblank opened this issue 10 years ago • 20 comments

Quick note: I don't have Gulp installed, so I didn't create the compiled files that way. I manually pieced together the compiled, unminified version of the JS file though.

Here is a list of updates:

  • aspect ratio
    • passed in as a string (width + 'x' + height)
    • defaults to 1x1 (square)
    • adjusted the square and circle classes to work as rectangle and ellipses classes
  • passes crop data and original image data, as attributes
    • crop data = { width, height, x, y }
    • original data = { width, height }
    • crop data is 2-way binding, so that users can initiate the canvas to a specific crop
      • if initiated out of bounds, it adjusts values to fit
    • X and Y coordinates are based on left-top corner
  • fixes issue with canvas redrawing incorrectly after being hidden
    • when the canvas is hidden it's clientHeight is set to 0, which basically tells the code to redraw everything to minimum values
    • added an if statement to prevent redrawing when the clientHeight is 0
  • initializes the crop to the maximum possible from the middle of the image
    • this is useful in projects where using the cropping tool is optional to the user, and creating a cropped image (to fit a particular aspect ratio) is required on the back-end
  • updated the test HTML file to show the recent changes
  • updated the README.md to reflect some of the recent changes

iblank avatar Jan 26 '15 22:01 iblank

I apologize for the long description and change list. I have been making these changes over time to a private repository, and finally got around to contributing them back here.

iblank avatar Jan 26 '15 22:01 iblank

Tested the new option for setting the aspect ratio of the result image and works like a charm.

Just one thing, initializing the crop to the maximun size is not very useful in my case. Im using ngImgCrop in a mobile app, and it's a bit tricky for the user to resize the crop when it's size is the maximun. The touch is not as precise as the mouse, and it's a bit difficult to hit the resize points when they are in the border of the container layer. I think it would be good to parametrize this option.

jonmikelm avatar Feb 16 '15 12:02 jonmikelm

Sounds like a good suggestion to me. I haven't looked at it in a while, but I believe the original default was half the width of the canvas. This could be the standard default again, with an option to maximize the selection.

iblank avatar Feb 17 '15 18:02 iblank

I decided to change the default crop selection to 75% of the closest side (based on aspect ratio). I think 50% worked well when the crop selection was always an even square or circle, but that it could be too small in cases where the aspect ratio of the crop and image differ too much. If you set the 'maximize-crop' attribute to true, it'll set the selection to 100%.

iblank avatar Feb 18 '15 21:02 iblank

I love all of the changes you have made, i do wish @alexk111 gets the time to review and hopefully merge these. Thanks for your work!

FyerPower avatar Feb 25 '15 14:02 FyerPower

No problem @gamegenius86. We needed all this functionality in our own project, but it's nice to hear it's helping others.

iblank avatar Feb 25 '15 16:02 iblank

A side note on the latest commit... the img-crop-result directive approach is meant to replace the canvas.toDataURL() function, since it's not supported in IE9. It's a better solution when you actually do all the cropping work on the backend. I've found that it's considerably faster as well, since it's not generating a new image every time the crop is adjusted.

Sample usage:

<div img-crop-result image="image.imagePath" crop-data="image.cropData" width="60%"></div>

If submitting an already cropped image to your backend is required, then I would stick to the original result-image attribute to generate that image.

iblank avatar Feb 25 '15 16:02 iblank

Seems to be really great work ! Is it possible to import it without waiting the pull request to be merged?

Capellaro avatar Feb 26 '15 07:02 Capellaro

great job thank you.

generics avatar Mar 10 '15 11:03 generics

@AlexCapi you can import my forked version here: https://github.com/iblank/ngImgCrop

iblank avatar Mar 10 '15 17:03 iblank

@iblank Thanks for your great work on this. I created a pull request this morning, which uncomments out the section of your code handling the EXIF rotation and also fixes an issue that is currently present in the original repo where large images taken from devices are not rending in the edit window.

hbbh avatar Mar 23 '15 09:03 hbbh

@alexk111 can you accept this?

daanl avatar Mar 30 '15 11:03 daanl

:+1:

STRML avatar May 04 '15 19:05 STRML

@alexk111 can you accept this?

daanl avatar May 18 '15 11:05 daanl

Thanks for very helpful feature @iblank . I found that crop-data is caculated base on scale image, not the original one. Because in my case, I upload the original image with the crop data, but because I display the image in responsive view so the upload crop-data can not apply to the original image. Can you give me some hints to fix this case? Thanks.

tinquang avatar Jul 09 '15 03:07 tinquang

I'm sorry, I don't exactly follow what your issue is. Are you saying that the crop data you pass in is not scaling correctly on the outputted image? The crop box should be using the same scale factor as the original, so that the output matches what you see. The crop data you pass in needs to be based on the actual dimensions of the original. Maybe you can post a screen capture that illustrates what's happening?

iblank avatar Jul 09 '15 13:07 iblank

ok, let me explain, for example I have a portrait image with original size is 960x1280, but when I show it up for cropping, I set container style limit height to 400, so the display image will be 300x400, and when I make a crop, the crop-data return crop information of the display image, not the original one, that mean the maximum crop size (circle) will be 300x300, and because I will upload the image file(not the base64 data) with the crop-data, the result image will has wrong size and wrong position.

tinquang avatar Jul 10 '15 02:07 tinquang

Ok, it sounds to me like the issue is likely caused by the use of max-height... ngImgCrop reads the width and height of the image, and the width and height of the canvas in order to scale everything correctly. If it doesn't get a number from any of those values it'll set the scale to 100%, regardless of what the scale actually should be (in your example it should be 320%).

I tried recreating your issue in jsfiddle: http://jsfiddle.net/ta27w58c/2/ It loads an image with crop data into a responsive canvas, which has a max-height set on it. The canvas and result image are inside a resizable div. Everything seems to function as expected on my end, so I'm not sure how to replicate your issue.

iblank avatar Jul 10 '15 19:07 iblank

Thanks for your quick support @iblank . In my case, there are few things that different from your test on jsfiddle: I turn on cropexif to auto rotate image, and I let user crop image on angular ui modal. Currently I sovled my problem by add 1 more function to cropHost factory and use it instead of your crop-data, but when I have time, I'll be back to test it again with crop-data, maybe there is something wrong on my side when apply your code. Many thanks for your support.

tinquang avatar Jul 13 '15 02:07 tinquang

@alexk111 Is this project dead or are you still active? Please have a look at some of the PRs, especially this one.

CallToPower avatar Feb 17 '16 15:02 CallToPower