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

When ratio is set, direction should be updated #161

Merged
merged 1 commit into from
May 5, 2019

Conversation

RaananW
Copy link
Contributor

@RaananW RaananW commented May 2, 2019

if dragging in one direction but asking to preserve ratio the direction should actually be one of the corners

if dragging in one direction but asking to preserve ratio the direction should actually be one of the corners
@xieziyu xieziyu changed the base branch from master to dev May 5, 2019 11:26
@xieziyu xieziyu merged commit c0497f1 into xieziyu:dev May 5, 2019
@xieziyu
Copy link
Owner

xieziyu commented May 5, 2019

@RaananW
This PR will make n and s handles stop to work if rzAspectRatio is set

@RaananW
Copy link
Contributor Author

RaananW commented May 5, 2019

Hey,

I checked it locally and it worked, but I might have missed something (it didn't push it?) . I'll check and let you know

@RaananW
Copy link
Contributor Author

RaananW commented May 6, 2019

I didn't commit a small change to resizeTo, which makes it work:

    let tmpX = Math.round(p.x / this._gridSize.x) * this._gridSize.x;
    let tmpY = Math.round(p.y / this._gridSize.y) * this._gridSize.y;

    // if aspectRatio is set, change should be max on both
    if (this._aspectRatio) {
      tmpX = tmpY = (Math.abs(tmpX) >= Math.abs(tmpY)) ? tmpX : tmpY;
    }

I have noticed, however, that it is eventually better to keep the single-direction approach, because this implementation will not work if dragging south, but making a big move on e or w.

I still believe that the direction SHOULD be updated, but direction should also have "original handler", to know which one was triggered. So, if I am using "s", originalHandler would be "s" (or s: true), but direction would be s: true, e: true , because the box is going to change in both directions.

@xieziyu
Copy link
Owner

xieziyu commented May 7, 2019

Yes, I agree. The original _direction was designed to mean "dragging a handler in this direction" which is different from "the box is changing in these directions". So it's better to introduce another variable for this purpose.

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.

2 participants