Improve ImageEditor #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's very common to do `img { width: 100%; height: auto; }` to let
images fill out their container. And the image usually has more pixels
in width than the container has in CSS pixels, to look good on high
resolution screens.
Previously, when clicking an image to edit it djedi would immediately
replace the image with a preview – and set `width: ...; height: ...`
with inline style on it to the actual pixel sizes of the image. This
caused images to blow up in size and overflow their containers.
The reason this was done was to simulate cropping. It worked if images
were assumed to be displayed at their "natural" size, but that's rarely
the case.
This commit uses a canvas to simulate the cropping instead, and only
updates the `src` if the images on the page. This allows previewing the
cropping without breaking the page styling/layout.
When doing this, I found _tons_ of little bugs and quirks. So the commit
ended up being a general refactor of the ImageEditor, making it
generally more stable and easier to understand.
I've also moved the Resize tab into the Image tab, since they are
closely related, and since the "Maintain aspect ratio" both affects the
width/height inputs as well as the crop tool. Another reason for doing
this is because I've hidden the "crop" input field, since it wasn't
editable and not useful.
I’m not 100% sure this is the way to go, so the old behavior is left in place and can be switched to from a variable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's very common to do
img { width: 100%; height: auto; }to letimages fill out their container. And the image usually has more pixels
in width than the container has in CSS pixels, to look good on high
resolution screens.
Previously, when clicking an image to edit it djedi would immediately
replace the image with a preview – and set
width: ...; height: ...with inline style on it to the actual pixel sizes of the image. This
caused images to blow up in size and overflow their containers.
The reason this was done was to simulate cropping. It worked if images
were assumed to be displayed at their "natural" size, but that's rarely
the case.
This commit uses a canvas to simulate the cropping instead, and only
updates the
srcif the images on the page. This allows previewing thecropping without breaking the page styling/layout.
When doing this, I found tons of little bugs and quirks. So the commit
ended up being a general refactor of the ImageEditor, making it
generally more stable and easier to understand.
I've also moved the Resize tab into the Image tab, since they are
closely related, and since the "Maintain aspect ratio" both affects the
width/height inputs as well as the crop tool. Another reason for doing
this is because I've hidden the "crop" input field, since it wasn't
editable and not useful.