- π³π±Netherlands eelkeblok Netherlands π³π±
Linking π Fix coding standard issues + add GitLab CI to the project Needs review as it fixes the test that I guess needs to be extended to fulfil #15.
- π³π±Netherlands eelkeblok Netherlands π³π±
I've found this patch can actually also run into a floating point issue; on certain image sizes, the dimensions of the crop area may work out to be slightly larger than the preview image used for the cropping, in which case Cropper will decide to show the default crop area. I'll see if I can work up a reproduction case and an updated patch.
When granting credit, please also include @idebr at iO since he actually found this issue.
- π³π±Netherlands eelkeblok Netherlands π³π±
Here's a file that can be used to reproduce the problem mentioned above. It works with the default 400 wide cropping thumbnail. I didn't dive into the exact mechanics how the width ends up being larger that the available width, but it obviously is some sort of rounding error. Intuitively, I'd say a simple round should be enough to get it back to 400 instead of 400-point-something, I can't imagine the rounding error would be large enough to get it up to 401. The alternative would be to do a floor, but that would work out to 399 (as long as we're sticking to the default cropping preview for the example) in a significant number of cases.
Also, I am wondering why we need extra lines of code to pass this information to the cropper; it would seem this is already happening somewhere else, so isn't it possible to leverage that in this particular case. The solution seems a bit "smelly", but maybe I am misunderstanding the problem.
- π«π·France dqd London | N.Y.C | Paris | Hamburg | Berlin
Thanks for the awesome and helpful review @eelkeblok - Seems this still neds work. Just wanted to let know that some of the required issues linked have been committed know. So wrk can go on in here. +1
- π³π±Netherlands eelkeblok Netherlands π³π±
I updated MR !3 with a fix for the problem because I needed a solution for the time being. Leaving on Needs work for the feedback about whether we need an additional place to set these dimensions, or whether we should be looking into where the dimensions are applied when it does work and whether we can make adjustments to make that work. It seems that we are now introducing some duplicate code that could have similar bugs. I tried finding out how it works for other situations, but couldn't figure it out in the limited time I had available.