Skip to content

Fixes 11135 - resizeable issue with border-box #1451

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fixes 11135 - resizeable issue with border-box #1451

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ed4d43
Fixes 11135 - resizeable issue with border-box
Feb 19, 2015
9fa0867
Fixes 10948 and 8932
Feb 19, 2015
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions ui/resizable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,15 +487,18 @@ $.widget("ui.resizable", $.ui.mouse, {
if ( this.position.left !== this.prevPosition.left ) {
props.left = this.position.left + "px";
}
if ( this.size.width !== this.prevSize.width ) {
props.width = this.size.width + "px";
}
if ( this.size.height !== this.prevSize.height ) {
props.height = this.size.height + "px";
}

this.helper.css( props );

if (this.size.width !== this.prevSize.width) {
props.width = this.size.width + "px";
this.helper.width(this.size.width);
}
if (this.size.height !== this.prevSize.height) {
props.height = this.size.height + "px";
this.helper.height(this.size.height);
}

return props;
},

Expand Down Expand Up @@ -994,8 +997,8 @@ $.ui.plugin.add("resizable", "alsoResize", {
$(exp).each(function() {
var el = $(this);
el.data("ui-resizable-alsoresize", {
width: parseInt(el.width(), 10), height: parseInt(el.height(), 10),
left: parseInt(el.css("left"), 10), top: parseInt(el.css("top"), 10)
width: parseInt(el.css("width"), 10), height: parseInt(el.css("height"), 10),
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure a proper fix would always use consistent methods. I don't think we should be mixing css( "width" ) and .width().

cc @mikesherov
Copy link
Author

Choose a reason for hiding this comment

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

I considered that - you could for example use .css('width') inside _mouseStart() and then wouldn't need to change _applyChanges() (around line 490) - but I didn't know whether it would have wider issues elsewhere. .width() and .css('width') are used inconsistently throughout the code. It would be quite a job to figure out whether these are intentional or not - and what consequences there would be for making things consistent. Stylistically I prefer .css('width') because you can setup a props object and do this.helper.css(props);
Copy link
Author

Choose a reason for hiding this comment

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

I put some notes together on width() vs css('width') vs outerWidth() here:
http://jsfiddle.net/ilancopelyn/n911jymr/1/
Have a play around with it and see what you think - there are some subtleties with trying to be consistent.
Copy link

Choose a reason for hiding this comment

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

good job
left: parseInt(el.css("left"), 10), top: parseInt(el.css("top"), 10)
});
});
};
Expand Down