Feat/albas improvements #644
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also organised imports, changed to updating properties, no optional chaining, no nullish coalescing or undefined values unless strictly necessary (will test this later).
…as changed. Parent needs to know that the settings were updated to update view.
…sly checkWithinCropperSizeBounds) - Changed how cropper size bounds are calculated. - Changed naming of scaled cropper state values to internal, as we now check all of them –static too– and they follow different rules. - Added hiding resize squares when static side and changed cursor in non hidden squares. - Deleted resetCropOnAspectRatio change (explained in the rules section of cropperPosition.utils in changes) - Change naming of checkWithinCropperSizeBounds to checkSizeAndPosition, and updated it to cover new behaviour explained in rules (see changes, cropperPosition.utils). - See rules in cropper position and cropper size bounds utils.
… size on resize window
- How it moves when resizing when maintain aspect ratio is true - Can resize when one internal static side is applied
|
I forgot to make and send the videos. I sent changes so cropper can resize when there's one static side (I don't think that needs a video) and changed how it resizes when maintain aspect ratio is true. Before, it would resize from a corner when resizing from a side.
Now, it resizes from the side:
Before, it only resized on x movement when resizing from a corner.
Now, it can resize on x and y movement.
But there's still an area where it doesn't respond. This happens in all corners. I gave up on fixing it.
|
|
Hi there, |
|
Yeah I'm not a fan either. I like some bits though. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.