Reset width and height for an image when swapping src #4877
Conversation
| @@ -369,6 +369,7 @@ | |||
| setSrc: function(src, callback, options) { | |||
| fabric.util.loadImage(src, function(img) { | |||
| this.setElement(img, options); | |||
| this._setWidthHeight(); | |||
There was a problem hiding this comment.
Umm... did you forget to pass options to _setWidthHeight or is this intentional? Just wondering.
There was a problem hiding this comment.
@scriptspry Great question. I just tried using setSrc with width and height specified in options, and the provided options were not set. I ended up needing to use loadImage directly, which shouldn't be necessary. I vote for options to be passed in to this._setWidthHeight() or better, don't call _setWidthHeight at all and let setElement take care of it.
There was a problem hiding this comment.
There was some reason this was done.
I'll try to recover it.
It could be that setSrc will intentionally reset the image from scratch, and if you need to set it with different widht/height/scropX/cropY you do that in the callback.
What would be wrong with this approach?
There was a problem hiding this comment.
@asturur Hi, nothing wrong with the approach of changing the width and height in the callback, it's just when I looked at the fabric source code I thought for sure not passing options along was a bug. I almost created a PR to fix it, then I noticed this comment and wanted to follow up.
* ok * added simple test * working test
close #4733
needs tests