immich
immich copied to clipboard
func: added should delete orginal after crop functionality
Fixed #11800
- Added new functionality of asking user to delete orginal image after cropping the image.
@mertalev Please add someone to review
The linked issue doesn't seem related to this PR. Also, the edited image is lossy and loses some qualities of the image, like HDR. I think we should make that more robust before suggesting the user to delete the original.
The cropping functionality is acting the same as previously, as I have not changed anything regarding the cropping functionality.
I have just added the "delete the original" functionality after the cropping is done. For, cropping image quality functionality, you can create a new bug card, and I can start working on it.
Also this functionality has nothing to do, within cropping functionality, so this just acts as a adds on as a functionality over the cropping functionality.
You haven't changed it, sure. But the premise of the feature is that the user can effectively replace the original with the edited version. I don't think we should suggest this until the edited image has the same quality as the original.
Ok, understood. I hope we can merge this after the crop image quality fix, as you mentioned. So, I have updated the code, where the image quality is maintained now after cropping.
Any update on this @Clement83
Any update on this @Clement83
Hi @yashrajjain726, just to clarify, I'm just an Immich enthusiast, not a contributor.
I noticed a typo and left a comment to help.
By the way, it's all good for me :)
Great @Clement83 , As it was assigned to you, I thought you would be reviewer.
@mertalev Any update on this ?
Sorry, I'm not a mobile dev and won't be the best person to review the implementation details.
My suggestion is to split out the lossless conversion to its own PR. In particular, it'd be nice to compare the output images before and after the change, and to check its color processing, metadata, etc.
Thanks for the PR. However, this is not the functionality I envision in the app. For future reference, you can chat with us on Discord before opening a PR to avoid situations where work got put in but doesn't get merged. Again, sorry for the confusion.
Great @alextran1502 , not an issue. Can we close #11800 also, if not required ?