Thanks HHChaos for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

It looks like there are inconsistencies to when this event fires. Initially, it only fires after moving the thumb and letting go of the mouse a single time, but if you then drag the image around it fires constantly as the image is moving. It looks like there are discrepancies between when the CroppedRegion property is updated during ManipulationDeltas vs. ManipulationCompleteds between the image and the thumbs.

It feels like there should there be two events, one which occurs as the region is changing do to the deltas, and another when the manipulations are completed? We could call them CropRegionManipulationDelta that's fired constantly as the thumbs or image are moving vs. CropRegionManipulationCompleted which fires only after a thumb or image manipulation with the mouse has finished?

We of course also need to document these behaviors and add an event section to the documentation here.

Thanks!

This is because dragging the thumb does not necessarily cause the CroppedRegion to change, although it seems that the CroppedRegion has changed. CroppedRegion only changes when the image is moved or zoomed (it can be understood that a change in the viewport will cause a CroppedRegion change). I want to know what you think about this. Is it acceptable?

@HHChaos yeah I understand the rationale of the current implementation. I'm just commenting from actually playing with the event from the development side, it doesn't seem as useful when the two scenarios (resizing area vs. changing image size/pos) behave differently.

I feel like based on the events we do have, it shouldn't be too hard to split apart into two sets of events for when there's any change vs. when it's completed?

I'm pretty sure this'll help developers in different scenarios too when they're providing immediate feedback on the cropped region's size with the delta or having to perform another step after the user's finished their manipulation.

From looking at the code in ImageCropper.Events.cs, I think we have enough flexibility to make these distinctions as we have the underlying ManipulationDelta/Completed events for the Thumbs or the Images. We'd just have to map those to our own versions for the CroppedRegion update and just tighten it up a bit.

@HHChaos also, if it helps, I did this initial review all on a live stream starting here. We jump to another PR for a bit while the app compiles, but you can see me testing the scenario out first-hand live here and discussing with some other community members about separating these concerns.

@HHChaos yeah I understand the rationale of the current implementation. I'm just commenting from actually playing with the event from the development side, it doesn't seem as useful when the two scenarios (resizing area vs. changing image size/pos) behave differently.

I feel like based on the events we do have, it shouldn't be too hard to split apart into two sets of events for when there's any change vs. when it's completed?

I'm pretty sure this'll help developers in different scenarios too when they're providing immediate feedback on the cropped region's size with the delta or having to perform another step after the user's finished their manipulation.

From looking at the code in ImageCropper.Events.cs, I think we have enough flexibility to make these distinctions as we have the underlying ManipulationDelta/Completed events for the Thumbs or the Images. We'd just have to map those to our own versions for the CroppedRegion update and just tighten it up a bit.

@michael-hawker Yes, I think you are right. I will make some changes.