ngx-image-cropper icon indicating copy to clipboard operation
ngx-image-cropper copied to clipboard

Feat/albas improvements

Open loiddy opened this issue 6 months ago • 2 comments

Hi! Not finished yet. Sending it now so you can start looking at it while I continue working on the next bits.

I changed your state approach to show you what I had in mind. I'm not entirely convinced by it. Just leaving it for now so I can move on with the other stuff.

I think it's important to only use state in image-cropper-component.ts to avoid confusion and not to separate the logic in ngOnChanges as it's already a brain bender. We can move it into a function somewhere, but keep it all together. Not in different functions/class methods.

I cringed at the settingsToUpdate input in my state approach. Seems like it should be a method. Which in a way makes me happy cos having only update transform and update cropper as methods was feeling weird to me. This is just food for thought though. I think we should talk more seriously about state once I send you everything and you read through it.

I deleted all the inputs so we can work on a clear-end result. I'll add them back at the end so developers can use the old and new approach like you said.

I've also changed it back to methods updating state values instead of returning the values which are then passed on to state in main. I want to test both approaches at the end to see if there's a difference. I suspect I might be wrong but I still want to test. I understand what you wanted and I like pure functions too. I just want to see if in this case, where ultimately I'd like the lib to be able to do things like scaling when resizing, simply updating the values is better. Same with undefined values and nullish coalescing or ||.

Which brings me to the next part. I saw you were trying to get rid of hammer. How's it going? Hammer seems to be terrible for performance so that would be awesome! Also you can unify a lot of the pointer events logic and put it in utils, further decluttering image-cropper-component.ts.

Any questions, I'm here. Chao chao for now :)

[EDIT]: Forgot, I'll add your tests back too.

loiddy avatar Jul 27 '24 22:07 loiddy