Add originalSize option #19
Conversation
Add -d option to make source maps work in Chrome/Firefox. From documentation (npx webpack --help): -d shortcut for --debug --devtool eval-cheap-module-source-map --output-pathinfo
Rationale: There is already an eslint config in this repo. Adding eslint as a dependency means that developers do not need to install it globally and everybody is using the same version.
src/crop.js
Outdated
| checkRatio (previewImageSize) { | ||
| if (this.originalSize) { | ||
| const expectedRatio = this.originalSize.width / this.originalSize.height | ||
| const actualRatio = previewImageSize.width / previewImageSize.height | ||
| const percentageChange = ((actualRatio - expectedRatio) / expectedRatio) * 100 | ||
| if (Math.abs(percentageChange) > 1) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn('srcissors: Expected and actual image ratio differ by more than 1%: ' + | ||
| `${expectedRatio} vs ${actualRatio}`) | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I wonder if we should actually just log and move on when this happens. I see that if correctly used, one never has an issue here. But it could lead to pretty bad results if ignored.
Have you thought about showing that error in the UI instead of loading the image in this case?
There was a problem hiding this comment.
Hmm, I agree that console.warn is not super useful, but I don't really have a better idea. Showing an error in the UI directly is a bit difficult because srcissors has a very light UI so far, so I guess the way to achieve this would be to invoke an error callback or event and let the library client deal with it. Is this what you had in mind?
There was a problem hiding this comment.
Could we emit a ratio-mismatch (or something better named) event in this case? Like srcissors already has for ready and load and change?
There was a problem hiding this comment.
What happens if the preview is the wrong size? Will scissors still produce valid crops?
And why not just throw an error?
There was a problem hiding this comment.
Throwing an error sounds like a good solution to me as well.
src/crop.js
Outdated
| const percentageChange = ((actualRatio - expectedRatio) / expectedRatio) * 100 | ||
| if (Math.abs(percentageChange) > 1) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn('srcissors: Expected and actual image ratio differ by more than 1%: ' + |
There was a problem hiding this comment.
The 'Expected' in the message confused me a bit.
Maybe: scrissors: Displayed image has a different image ratio than the one configured in 'originalRatio' ...?
Add `originalSize` option, which allows to display a downscaled version of the image in the cropping interface, but use the original size for crop attributes.
a3c1d0d to
24c4d6a
Compare
|
Thank you for the reviews! I changed the behavior to throw an error if the aspect ratio does not match and improved the error message as suggested by Lukas. |
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Related to livingdocsIO/livingdocs-planning#4095 / livingdocsIO/livingdocs-planning#1084
Motivation
For very large images, it is either not possible¹ or not desirable (for perf reasons) to load them in full resolution in the cropping interface. In order to support this use case, a new option
originalSizeis introduced, which allows to detach the coordinate system of all cropping options (crop.{x,y,width,height}, minResolution, minWidth, etc.) from the pixel resolution of the preview image.¹ e.g. imgix has a limitation of 8192px per dimension
Considered alternative: Introduce a scale factor that defines the relation between original and preview size. This was not done because it would introduce more complexity: All affected cropping options would have to be scaled and rounded to the nearest integer, which could also introduce subtle changes without any user action.
Changelog
originalSizeoption, which allows to display a downscaled version of the image in the cropping interface, but use the original size for crop attributes.