Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added checking _direction when applying styles #117

Merged
merged 2 commits into from
Oct 31, 2018

Conversation

Yamazaki93
Copy link
Contributor

Hi,
I've included changes here I think could solve #116, couldn't find the dev branch so opened the PR to master.

Copy link
Owner

@xieziyu xieziyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will break [rzAspectRatio] function when only vertical or horizontal handles set. It still needs to apply height or width changes.

I will create a dev branch, then you can submit PR to it.

@Yamazaki93
Copy link
Contributor Author

Ah yes you are absolutely right, will change that and update the PR

@Yamazaki93 Yamazaki93 changed the base branch from master to dev October 29, 2018 20:37
@Yamazaki93
Copy link
Contributor Author

Yamazaki93 commented Oct 29, 2018

Ok, so the reasoning behind ed0737a :
If aspect ratio is set -> the user wants resizable size aspect ratio should be maintained -> both width and height need to be set.
If apsect ratio is not set -> the user doesn't want aspect ratio to be maintained -> only setting width or height depending on handles available
So if only w or e handle is available and aspect ratio is not set, dragging them inherently changes the aspect ratio. We then don't set the height, allowing the other styles to apply, does that sound about right?
Thanks!

@xieziyu xieziyu merged commit 963d30b into xieziyu:dev Oct 31, 2018
@xieziyu
Copy link
Owner

xieziyu commented Oct 31, 2018

Thanks, I will publish it soon.

@Yamazaki93 Yamazaki93 deleted the fix-size-locking branch October 31, 2018 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants