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

add resizable.preserveAspectRatio option #260

Merged
merged 1 commit into from
Oct 3, 2015
Merged

add resizable.preserveAspectRatio option #260

merged 1 commit into from
Oct 3, 2015

Conversation

tclift
Copy link
Contributor

@tclift tclift commented Aug 17, 2015

This maintains the aspect ratio of the interactable as it was at resizeStart.

I tested this by modifying the html_svg demo and:

  • Adding preserveAspectRatio: true to the resizable options.
  • Changing the DemoNode's width to 100px (to give it a 1:2 aspect ratio, which is a bit different to that after the margin, border and padding are added).
  • Changing the parseInts to parseFloats so that rounding doesn't mess up the adjustments.
    wasn't sure if you wanted me to add a demo or test somewhere. I build the docs using dr.js, but the page that lists all the demos seems to be missing (not in the repo, I guess). Also most of the existing tests didn't seem to be working.

It is possible to upset the ratio by externally changing the width and height. E.g. the demo page sets a minimum width/height that will cause the ratio to be upset. I figure the onus for this is on the user.

As per resizable.square, resizing from e.g. the bottom right corner is actually an interaction with the right edge (with the bottom edge being linked during the resize), rather than a combination of both edges (i.e. moving up and down will not affect the size, only left and right).

Resolves #169.

@taye
Copy link
Owner

taye commented Aug 20, 2015

Thanks for working on this! I'll look through the code as soon as I can.

}

if (this.options.resize.square && this.options.resize.preserveAspectRatio) {
throw new Error('resizable.square and resizable.preserveAspectRatio cannot be used together');
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer to avoid throwing custom errors. It would be better to simply give preserveAspectRatio preference by checking for it before square at line 2007.

@taye
Copy link
Owner

taye commented Aug 21, 2015

There's a problem with resizing elements that don't have an aspect ratio of 1 from the top/left – either the width or height is incorrect depending on which edge is dominant.

@taye
Copy link
Owner

taye commented Sep 20, 2015

@tclift Have you had time to look at the problems I mentioned?

@taye
Copy link
Owner

taye commented Sep 23, 2015

Have you had a chance to look into the problems I mentioned?

… ratio of the interactable as it was at `resizeStart`. Note that, as per `resizable.square`, resizing from e.g. the bottom right corner is actually an interaction with the right edge (with the bottom edge being linked during the resize), rather than a combination of both edges (i.e. moving up and down will not affect the size, only left and right).
@tclift
Copy link
Contributor Author

tclift commented Sep 23, 2015

Updated to take comments into account. I'll make up a sample on CodePen to test resizing from the top left. Locally it works for me. Unless I've misunderstood I suspect the problem might be externally adjusting the width or height during the resize, such as doing a Math.min in a resizing callback.

@tclift
Copy link
Contributor Author

tclift commented Sep 24, 2015

Well, I didn't notice a problem with resizing from the top left, but found something else... the aspect ratio is based on the "rect" at resize start. This rect is based on getClientRects, which includes any borders and padding. If the borders and padding are fixed size, that is going to mess with the true aspect ratio of the element. I.e. keeping the aspect ratio of the resize rect constant doesn't result in a constant element aspect ratio. The effect is small if the borders and padding are small, however.

Here's a demo. Uncomment the border in the CSS and/or add some padding to see the effect.

Not sure what the best option is from here. I'll have a think about it. What are your thoughts?

@taye
Copy link
Owner

taye commented Oct 3, 2015

What I was seeing was actually just a rounding error in the resizemove listeners of the demo I was using.

I think it's fine if borders cause problems. It can be avoided with a box-sizing: border-box CSS rule or by using a custom rectChecker that handles the border appropriately.

Thank for implementing this!

taye added a commit that referenced this pull request Oct 3, 2015
add `resizable.preserveAspectRatio` option
@taye taye merged commit 93274c0 into taye:master Oct 3, 2015
@tclift tclift deleted the preserve-aspect-ratio branch October 6, 2015 03:34
@davidgfnet
Copy link

Hey, from what I see in the source doesn't seem feasible to implement a dynamic value for preserveAspectRatio (so each object can decide whether to preserve the aspect ratio or not).
Cheers!

@taye
Copy link
Owner

taye commented Aug 10, 2016

@davidgfnet you can work around this by changing the setting in a custom action checker.

If you're using browserify, you can listen to the internal before-start signal and change the interactable settings from a listener function. It should be something like:

var interactAutoStart = requre('interact.js/src/autoStart/index.js');

interactAutoStart.signals.on('before-start', function (arg) {
  // check the interaction details and change settings as necessary
});

The arg to the listener comes from here.

{
  pointer,
  event,
  eventTarget,
  dx,
  dy,
  duplicate,
  interaction,
  interactingBeforeMove,
}

@davidgfnet
Copy link

I'll check it :) But I guess it would be easier to modify the interface to accommodate the feature, right?

@taye
Copy link
Owner

taye commented Nov 28, 2016

@davidgfnet I'll consider allowing a function which returns true/false to be given as the value for the preserveAspectRatio option. What do you think?

@davidgfnet
Copy link

Superb!

@zbjornson
Copy link
Contributor

A function would be nice, but meanwhile I implemented dynamic behavior (shift key = preserve aspect ratio) with this:

// interactable...
.actionChecker(function (pointer, event, action, interactable) {
	interactable.options.resize.preserveAspectRatio = event.shiftKey;
	return action;
});

@bruceCzK
Copy link

bruceCzK commented Sep 3, 2024

A function would be nice, but meanwhile I implemented dynamic behavior (shift key = preserve aspect ratio) with this:

// interactable...
.actionChecker(function (pointer, event, action, interactable) {
	interactable.options.resize.preserveAspectRatio = event.shiftKey;
	return action;
});

Not working any more, is there any workaround?

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.

5 participants