feat(web): image editor - panel and cropping

[ilyaChuk]

trafficstars

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.

[opbod]

Related: https://github.com/immich-app/immich/pull/10989 (Mobile); https://github.com/immich-app/immich/pull/3271 (Web); https://github.com/immich-app/immich/pull/9575 (Web)

[ilyaChuk]

I feel like I'm ready. The cropping interface is ready. There is a editor tools sidebar on desktop screens. So far, there's only cropping, but she's ready to work with an array of tools. Since last time, I added a "save" button. It is active only when changes have been made in the editor. So far, clicking on it does nothing except close the editor and the panel. I've double-checked everything several times, different aspect ratios of the images, it works stably for me. This is the first time I have had such an experience, but I tried to make the code convenient for other developers as well. video

[opbod]

https://github.com/immich-app/immich/pull/10989 just got accepted into main few hours ago, mobile-only. @martyfuhry does this web-only image editor PR conflict with or enrich the overall setup?

[ilyaChuk]

@opbod

does this web-only image editor PR conflict with or enrich the overall setup?

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.

[ilyaChuk]

@michelheusschen Thank you very much for the review. I fixed all the problems. But I postponed the decomposition stage to the very end. Therefore, there is a possibility that I broke something in the component. I still try to check all the work and as if everything is ok.

[ilyaChuk]

there is Test / Web (pull_request) error

 src/lib/i18n.spec.ts  (50 tests | 1 failed) 6739ms
   ❯ src/lib/i18n.spec.ts > i18n > no missing messages
     → expected { errors: { …(121) }, …(801) } to deeply equal { about: '', account: '', …(799) }
     
     ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/lib/i18n.spec.ts > i18n > no missing messages
AssertionError: expected { errors: { …(121) }, …(801) } to deeply equal { about: '', account: '', …(799) }

is it my fault? I didn't seem to touch anything there.

[alextran1502]

Just try the PR, when we hit Save it doesn't do anything yet, correct?

[ilyaChuk]

@alextran1502

Save it doesn't do anything yet, correct?

Yes, clicking on it does nothing except close the editor and the panel. In the future, this button should send a request to the server. And it will already save the photo somehow. Most likely, this will be my next PR

[alextran1502]

I think it makes sense to include the save functionality in this PR as well, if we merge it to main and have to push out a release, there will be a features that is not usable. Or maybe we can hide the button?

[ilyaChuk]

In the discord, they told me "Definitely a separate PR. Keep things as small as possible". I'm not sure what's the right thing to do. Because the saving functionality is, as far as I can imagine, quite a lot of functionality. Maybe we can not send it to the release yet, but keep it in some other branch? Together with the photo editor for the mobile application #10989.

[alextran1502]

I think we want to keep different editing features in separate PRs, but each should have a finished state. I am fine with hiding the button in this PR and getting it merged.

The mobile app editor is fine since it produces the local file that can be uploaded/shared.

[ilyaChuk]

Ok, then I'll remove this button now. but I don't really understand what you mean by "merged". You're not going to release this, are you? because it's now a bare interface with no real save

[alextran1502]

Merge means getting this into main so that you don't have to keep working around merge conflicts, which happens often since many PRs get merged into main every day.

[ilyaChuk]

Should we have parity with the mobile version in terms of the corners of the cropping rectangle? On the one hand, I want the editor to look the same in both places. But on the other hand, the circles on the corners look more convenient for me to use the mouse. In fact, this is just a visual, the capture boundaries are the same, but it looks less convenient for me. And yet, in the web Google photo - circles 👍 - yes, corners 👎 - no, circles

[ilyaChuk]

if I have not forgotten anything, then I have fulfilled all the wishes from your review. but tomorrow I want to add a pop-up question when trying to close the editor, "are you sure you want to quit? the changes will not be saved" as in google photos. and presets for vertical aspect ratio. and then the PR will be ready

[alextran1502]

if I have not forgotten anything, then I have fulfilled all the wishes from your review. but tomorrow I want to add a pop-up question when trying to close the editor, "are you sure you want to quit? the changes will not be saved" as in google photos. and presets for vertical aspect ratio. and then the PR will be ready

Sounds good to me!

[ilyaChuk]

merge conflict was not easy to resolve. I hope I didn't break anything. But otherwise, everything is ready @alextran1502

[ilyaChuk]

[alextran1502]

I would like to get this PR merged in main before it gets out of hand with the changes. Let's finish up the UI and then have it merged. Then we can implement the logic of actually saving the photo later

[ilyaChuk]

I'm a little tilted because I've done so much, and it really needs to be rewritten from canvas to html elements. It won't take super long. but it's mentally hard. It's ok, I had to come to this in a couple of days. and it will be ready in a few days.

[jrasm91]

I'm happy to help work on this if you want some help finishing it. I also started work on a server API for creating assets from edits that might be possible to plug this into (#11658). I think most of what you have now is good to go, just needs a bit of cleanup and polish.

[ilyaChuk]

@jrasm91, I'm glad you've joined the editor's work too, and thanks for your willingness to help. But there's not much left to do in this PR, I'll finish it up

[ilyaChuk]

I think it's ready. Moved from canvas to html elements.

video

[ilyaChuk]

Thanks to the move away from canvas, it's easier to make rotations. But there is one point of contention for me. Aspect ratios. Should it be relative to the image or relative to the window? I checked, in the mobile version it is now relative to the window. But look at the 1st video. Here I can see the advantage of the aspect ratio being relative to the image. I'll be honest, it's a lot easier for me. Secondly, trying to change the aspect ratio while trying to stay relative to the window results in awkward behaviour: https://github.com/user-attachments/assets/7859c57a-ec89-49ab-bf88-51c78a34656b Yeah, it's got an extra bug. But you get the point. If you want to select the lettering "poison". When you rotate and try to save the relative ratio, the inscription will be selected wrong (perpendicular). I checked it again. in Google photos, the aspect ratio relative to the image. vote 👍 - relative to image 👎 - relative to window

[ilyaChuk]

~~Feels ready 😴~~ no, I found a problem with very long or very wide images. It's not easy to fix. in the morning

[ilyaChuk]

Ok, now I don't see any errors. Yet

[ilyaChuk]

but I still don't understand the error I'm getting here https://github.com/immich-app/immich/actions/runs/10328229151/job/28597174488

[eygraber]

I saw this in the release notes, but it doesn't look like it is available. Is it disabled, or hidden?

[ilyaChuk]

@eygraber Yeah, it's hidden for now. I think we'll launch it when the server side is ready.

[sammyke007]

Any news on this?