bcabanes / angular-image-cropper

AngularJS directive for cropping images.
124 stars 62 forks source link

api.zoom() doesn't seem to work for certain values #25

Closed sugataach closed 8 years ago

sugataach commented 8 years ago

Hey bcabanes,

Great directive!

Just having some trouble with the zoom api, it doesn't seem to work for integers less than 3 (including negative numbers).

Ideally, I'd like to pass in zoom(1) or zoom(-1).

See codepen link, try playing around with the settings on the "Zoom API" button.

Codepen Link

sugataach commented 8 years ago

Hmmm some interesting findings:

WORKS FOR

api.zoom(1.01)

DOESN'T WORK

api.zoom(1) api.zoom(2) api.zoom(-1) api.zoom(-1.01) api.zoom(-3)

bcabanes commented 8 years ago

Hi @sugataa, It seems like a bug, thank you very much for the Codepen 👍 link it helps a lot. I'll make a bugfix soon. I think api.zoom() isn't a good idea, api.zoomIn() & api.zoomOut() make more sense.

sugataach commented 8 years ago

Hey, afraid there's still an issue with api.zoomOut, I can't seem to get it to work.

api.zoomOut isn't even registering in the api

Here's a gist of how I ended up "hacking" it...

https://gist.github.com/sugataa/9d7c4986b9ca2071b668

sugataach commented 8 years ago

Basically I changed api.zoomIn(factor) back to api.zoom(factor)

WORKS

to zoomIN -> factor must be bigger than 1 to zoomOUT -> factor must be less than 1

jokeMF commented 7 years ago

I had the same problem and I found the 'bug': the zoom factor is converted twice resulting in a 2x zoom with values close to 1.

In Cropper.prototype.setDimensions zoomInFactor and zoomOutFactor are set:

this.zoomInFactor = 1 + parseFloat(this.options.zoomStep); this.zoomOutFactor = 1 / this.zoomInFactor;

then in Cropper.prototype.bindControls applyZoomIn and applyZoomOut are bound:

this.elements.controls.zoomIn.addEventListener('click', function() { self.applyZoomIn(self.zoomInFactor); }); this.elements.controls.zoomOut.addEventListener('click', function() { self.applyZoomOut(self.zoomOutFactor); });

in these two functions the zoom factor is converted again, causing the factor 2

Cropper.prototype.applyZoomIn = function(zoom) { this.zoomImage(1 + parseFloat(zoom)); }; Cropper.prototype.applyZoomOut = function(zoom) { this.zoomImage(1 / ( 1 + parseFloat(zoom))); };

To make it work properly I replaced both calls by:

this.zoomImage(parseFloat(zoom));