-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(web): image editor - panel and cropping #11074
Conversation
I feel like I'm ready. The cropping interface is ready. |
#10989 just got accepted into main few hours ago, mobile-only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First off, a big thank you for the PR. Having an editor would be a great addition to Immich!
let animationFrame: ReturnType<typeof requestAnimationFrame>; | ||
let canvasCursor: string; | ||
const getAssetUrl = (id: string, checksum: string) => { | ||
return `http://localhost:2283/api/assets/${id}/original?c=${checksum}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the original asset can't be displayed by the browser? Also this URL should be changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only partially resolved. There is still the possibility that a browser can't display the original asset. This will also be an issue for videos, live photos and panoramas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we even allow the editor to be opened for live photos, panoramas, and gifs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it makes sense to disable the editor for these types of assets. If there are tools added in the future that can be used for more asset types (like rotate), we can always reconsider this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is still the possibility that a browser can't display the original asset
And then what to show? now there is an error message when opening the editor and an error loading the image. An error message, a black editor screen, and a toolbar on the right.
Should I close the editor automatically in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it makes sense to disable the editor for these types of assets
done
// Vertical lines | ||
const thirdWidth = crop.width / 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge fan of the grid lines personally, maybe only show them while changing the crop area?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the lines visible only when the cropping rectangle is changed. But then, when the cropping rectangle is the same size as the image, it is poorly visible
I can't imagine how to solve this in a simple way. I added small circles on the corners for more visibility.
But maybe as long as the rectangle is the same size as the image, draw these lines?
Although if this is just a problem for me, then everything is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think this is better. The dots only seem to get cut off at the edges, maybe there's a way to extend them? I also noticed that cropping immediately stops when the cursor leaves the canvas. Can we allow cropping to continue when that happens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will require some reworking, but I will try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't easy, but it's done. video
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing that might be worth looking into down the road is using sharp to do the actual cropping on the original asset. So always show the crop tool on the jpeg/webp thumbnail and use it to get the cropping coordinates, but when send those to the server which can actually perform the operation. I believe using sharp it would be possible to keep the metadata, etc.
We try to work together by discussing our roles. But the difference so far is that I cannot rotate the image, and it is not saved. |
Co-authored-by: Michel Heusschen <[email protected]>
|
Ok, now I don't see any errors. Yet |
but I still don't understand the error I'm getting here https://github.com/immich-app/immich/actions/runs/10328229151/job/28597174488 |
Head branch was pushed to by a user without write access
I saw this in the release notes, but it doesn't look like it is available. Is it disabled, or hidden? |
@eygraber Yeah, it's hidden for now. I think we'll launch it when the server side is ready. |
* cropping, panel * fix presets * types * prettier * fix lint * fix aspect ratio, performance optimization * improved tool selection, removed placeholder * fix the mouse's exit from canvas * fix error * the "save" button and change tracking * lint, format * the mini functionality of the save button * fix aspect ratio * hide editor button on mobiles * strict equality Co-authored-by: Michel Heusschen <[email protected]> * Use the dollar sign syntax for stores inside components * unobtrusive grid lines, circles at the corners * more correct image load, handleError * more strict equality * fix styles. unused and tailwind Co-Authored-By: Michel Heusschen <[email protected]> * dont store isShowEditor * if showEditor - hide navbar & shortcuts * crop-canvas decomposition (danger) I could have accidentally broken something.. but I checked the work and it seems ok. * fix lint * fix ts * callback function as props * correctly disabling shortcuts * convenient canvas borders • you can use the mouse to go beyond the boundaries and freely change the crop. • the circles on the corners of the canvas are not cut off. * -the editor button for video files, -save button * hide editor btn if panoramic || gif || live * corners instead of circles (preview), fix lint&format * confirm close editor without save * vertical aspect ratios * recovery after merge. editor's closing shortcut * fix format * move from canvas to html elements * fix changes detections * rotation * hide detail panel if showing editor * fix aspect ratios near min size * fix crop area when changing image size when rotate * fix of fix * better layout - grouping https://github.com/user-attachments/assets/48f15172-9666-4588-acb6-3cb5eda873a8 * hide the button * fix i18n, format * hide button * hide button v2 --------- Co-authored-by: Michel Heusschen <[email protected]> Co-authored-by: Alex Tran <[email protected]>
Any news on this? |
Yes -which github issue to track and vote on for server side of this? |
#11658 seems to be a potential server-side implementation. |
Any update on this?? |
Another attempt to create a photo editor. I hope it's the final one. #10883
Video
I made a sidebar with tools. so far, there is a cropping and adjustment tool (placeholder).
Cropping works - you can freely choose the size or use the preset.
I checked on different pictures: horizontal, square, vertical. I didn't notice any problems.
Only the desktop version works. I haven't adapted it to mobile screens.
This is only a web interface, there is no server part of this functionality yet.
TODO:
[ ] Adaptation to mobile screens.[x] Canvas performance optimization.