Skip to content

Commit

Permalink
Resizable: containment plugin restricts resizing within container
Browse files Browse the repository at this point in the history 
Fixes #7018
Fixes #9107
Closes gh-1122
  • Loading branch information
@dekajp @mikesherov
dekajp authored and mikesherov committed Dec 15, 2013
1 parent bae1a25 commit 20f0646
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 11 deletions.
31 changes: 31 additions & 0 deletions tests/unit/resizable/resizable_options.js
View file
Expand Up @@ -123,6 +123,37 @@ test("aspectRatio: 'preserve' (ne)", function() {
equal( target.height(), 70, "compare minHeight");
});

test( "aspectRatio: Resizing can move objects", function() {
expect( 7 );

// http://bugs.jqueryui.com/ticket/7018 - Resizing can move objects
var handleW = ".ui-resizable-w",
handleNW = ".ui-resizable-nw",
target = $( "#resizable1" ).resizable({
aspectRatio: true,
handles: "all",
containment: "parent"
});

$( "#container" ).css({ width: 200, height: 300 });
$( "#resizable1" ).css({ width: 100, height: 100, left: 75, top: 200 });

TestHelpers.resizable.drag( handleW, -20 );
equal( target.width(), 100, "compare width - no size change" );
equal( target.height(), 100, "compare height - no size change" );
equal( target.position().left, 75, "compare left - no movement" );

// http://bugs.jqueryui.com/ticket/9107 - aspectRatio and containment not handled correctly
$( "#container" ).css({ width: 200, height: 300, position: "absolute", left: 100, top: 100 });
$( "#resizable1" ).css({ width: 100, height: 100, left: 0, top: 0 });

TestHelpers.resizable.drag( handleNW, -20, -20 );
equal( target.width(), 100, "compare width - no size change" );
equal( target.height(), 100, "compare height - no size change" );
equal( target.position().left, 0, "compare left - no movement" );
equal( target.position().top, 0, "compare top - no movement" );
});

test( "containment", function() {
expect( 6 );
var element = $( "#resizable1" ).resizable({
Expand Down
41 changes: 30 additions & 11 deletions ui/jquery.ui.resizable.js
View file
Expand Up @@ -341,14 +341,19 @@ $.widget("ui.resizable", $.ui.mouse, {
el = this.helper, props = {},
smp = this.originalMousePosition,
a = this.axis,
prevTop = this.position.top,
prevLeft = this.position.left,
prevWidth = this.size.width,
prevHeight = this.size.height,
dx = (event.pageX-smp.left)||0,
dy = (event.pageY-smp.top)||0,
trigger = this._change[a];

this.prevPosition = {
top: this.position.top,
left: this.position.left
};
this.prevSize = {
width: this.size.width,
height: this.size.height
};

if (!trigger) {
return false;
}
Expand All @@ -369,16 +374,16 @@ $.widget("ui.resizable", $.ui.mouse, {
// plugins callbacks need to be called first
this._propagate("resize", event);

if (this.position.top !== prevTop) {
if ( this.position.top !== this.prevPosition.top ) {
props.top = this.position.top + "px";
}
if (this.position.left !== prevLeft) {
if ( this.position.left !== this.prevPosition.left ) {
props.left = this.position.left + "px";
}
if (this.size.width !== prevWidth) {
if ( this.size.width !== this.prevSize.width ) {
props.width = this.size.width + "px";
}
if (this.size.height !== prevHeight) {
if ( this.size.height !== this.prevSize.height ) {
props.height = this.size.height + "px";
}
el.css(props);
Expand Down Expand Up @@ -662,7 +667,9 @@ $.widget("ui.resizable", $.ui.mouse, {
position: this.position,
size: this.size,
originalSize: this.originalSize,
originalPosition: this.originalPosition
originalPosition: this.originalPosition,
prevSize: this.prevSize,
prevPosition: this.prevPosition

This comment has been minimized.

Sign in to view

Copy link
@scottgonzalez

scottgonzalez Jul 9, 2014

Member

This changed the public API. We need to revert this part of the change. I've filed http://bugs.jqueryui.com/ticket/10148 to track this.

This comment has been minimized.

Sign in to view

Copy link
@dekajp

dekajp Jul 10, 2014

Author Contributor

@scottgonzalez - Here is the new commit , let me know if this looks ok ? dekajp@e5267d8
};
}

Expand Down Expand Up @@ -779,7 +786,7 @@ $.ui.plugin.add( "resizable", "containment", {
}
},

resize: function( event ) {
resize: function( event, ui ) {
var woset, hoset, isParent, isOffsetRelative,
that = $( this ).resizable( "instance" ),
o = that.options,
Expand All @@ -790,7 +797,8 @@ $.ui.plugin.add( "resizable", "containment", {
top: 0,
left: 0
},
ce = that.containerElement;
ce = that.containerElement,
continueResize = true;

if ( ce[ 0 ] !== document && ( /static/ ).test( ce.css( "position" ) ) ) {
cop = co;
Expand All @@ -800,6 +808,7 @@ $.ui.plugin.add( "resizable", "containment", {
that.size.width = that.size.width + ( that._helper ? ( that.position.left - co.left ) : ( that.position.left - cop.left ) );
if ( pRatio ) {
that.size.height = that.size.width / that.aspectRatio;
continueResize = false;
}
that.position.left = o.helper ? co.left : 0;
}
Expand All @@ -808,6 +817,7 @@ $.ui.plugin.add( "resizable", "containment", {
that.size.height = that.size.height + ( that._helper ? ( that.position.top - co.top ) : that.position.top );
if ( pRatio ) {
that.size.width = that.size.height * that.aspectRatio;
continueResize = false;
}
that.position.top = that._helper ? co.top : 0;
}
Expand All @@ -829,15 +839,24 @@ $.ui.plugin.add( "resizable", "containment", {
that.size.width = that.parentData.width - woset;
if ( pRatio ) {
that.size.height = that.size.width / that.aspectRatio;
continueResize = false;
}
}

if ( hoset + that.size.height >= that.parentData.height ) {
that.size.height = that.parentData.height - hoset;
if ( pRatio ) {
that.size.width = that.size.height * that.aspectRatio;
continueResize = false;
}
}

if ( !continueResize ){
that.position.left = ui.prevPosition.left;
that.position.top = ui.prevPosition.top;
that.size.width = ui.prevSize.width;
that.size.height = ui.prevSize.height;
}
},

stop: function(){
Expand Down

6 comments on commit 20f0646

@jzaefferer
Copy link
Member

Choose a reason for hiding this comment

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

@dekajp @mikesherov one of these resizable commits caused resizable-related failures in the dialog testsuite, for example http://swarm.jquery.org/result/1453910

Even just running the dialog tests locally in Chrome triggers a lot of console errors, along with dozens of failed tests.

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

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

Ok, that's weird. I didn't see these failures when I tested it locally myself. Can we back this out for now?

@dekajp
Copy link
Contributor Author

@dekajp dekajp commented on 20f0646 Jan 7, 2014

Choose a reason for hiding this comment

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

switched to ticket-7018 branch and ran the unit test on dialog on chrome browser in MacOs. I do not see any new failures.https://github.com/dekajp/jquery-ui/tree/ticket-7018 . could be other resizable fixes - let me go thru each fixes

@dekajp
Copy link
Contributor Author

@dekajp dekajp commented on 20f0646 Jan 7, 2014

Choose a reason for hiding this comment

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

@mikesherov @jzaefferer 7485 is where i see the failures in my local machine - 78 failed out of 402
https://github.com/dekajp/jquery-ui/commits/ticket-7485

c03cb80

@dekajp
Copy link
Contributor Author

@dekajp dekajp commented on 20f0646 Jan 7, 2014

Choose a reason for hiding this comment

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

@mikesherov @jzaefferer : confirmed this commit has caused this issue - bae1a25

@dekajp
Copy link
Contributor Author

@dekajp dekajp commented on 20f0646 Jan 7, 2014

Choose a reason for hiding this comment

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

#1162

Please sign in to comment.