Fixes 11135 - resizeable issue with border-box #1451

ilanc · 2015-02-19T11:42:17Z

If you apply .resizeable() to an element which has border-box sizing and
this element has a border then the container will "shrink" by this
border size immediately as the resizing drag begins.

Unit tests all passing.

If you apply .resizeable() to an element which has border-box sizing and this element has a border then the container will "shrink" by this border size immediately as the resizing drag begins.

alsoResize with border-box sizing

razvanphp · 2015-02-20T21:01:13Z

+1 tested, it works great, thank you!

ilanc · 2015-02-21T10:56:03Z

Any idea how I clear that "CLA error"? I've signed the declaration thing twice but no luck. Also I emailed 'license@jquery.com' and the email bounced.

scottgonzalez · 2015-02-21T13:23:42Z

Your git config doesn't have your full name, so it's being rejected. You can follow the instructions from the details link to fix the problem.

scottgonzalez · 2015-02-23T20:43:10Z

ui/resizable.js

@@ -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),


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

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);

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.

razvanphp · 2015-03-02T19:56:43Z

@ilanc can you please fix this CLA thingy? This bug is around for a few years, I really want to see it merged as soon as possible..

ilanc · 2015-03-03T09:00:47Z

Hi @razvanphp. I'm having trouble doing this. I've changed my user settings and attempted to --reset-author as per the advice on the CLA link but nothing changes on github when I do a git push -f origin 11135-resizeable-borderBox - it just says "Everything up-to-date".
NB there are 2 commits in the pull request.

It's such a minor fix - feel free to just duplicate the fix in your own branch and ignore my pull request. I'll sort this out if I ever make any future pull request.

razvanphp · 2015-03-20T22:59:47Z

I have duplicated this fix in my own branch. I also signed the CLA.

Should I create another pull request or can this one be fixed?

thisjustin · 2015-05-24T19:10:42Z

@ilanc @razvanphp @scottgonzalez what's the status on this? It's not clear if a pull req with a fix has been submitted / accepted.

Working using the above fix and a patch for our site but would really love to see this get into a future release and don't want to add to confusion by opening duplicate pull req.

scottgonzalez · 2015-05-24T19:57:44Z

I still feel strongly that we shouldn't be mixing .css( "width" ) and .width(). I was hoping @mikesherov would chime in since he has more experience with the sizing logic in resizable.

ilanc · 2015-05-25T11:11:32Z

Hi guys. I haven't been following this thread. Use @ilanc if you want me to get notified by email. I thought that @razvanphp was going to resubmit and that this would have been added to the main branch by now.

razvanphp · 2015-05-25T11:28:26Z

as @thisjustin mentioned, I didn't want to add more confusion by creating a new pull request. Also it is clear to me that the CLA thing is not the strongest argument against this PR. Maybe @ilanc can still fix this by a force push.

@scottgonzalez if you look at the next line you commented, you will see that before this fix, .css( "left") and .width() were mixed up, for left and top, so I don't understand your concern. More to that, this is a bug fix, not a cleanup refactoring.

i.e.
https://github.com/jquery/jquery-ui/blob/1.11.4/ui/resizable.js#L991
and
https://github.com/jquery/jquery-ui/blob/1.11.4/ui/resizable.js#L992

ilanc · 2015-05-25T12:10:03Z

This is what I dislike about git - it will take the rest of the day for me to figure out how to change the author from my netreturn adddress to my gmail address, just so that the CLA check passes. And I know what's going to happen - it'll still fail even after I figure it out. @razvanphp can you explain what I have to do to update the author info given that I have 2 commits? The advice on the CLA link only describes fixing 1 commit and doesn't seem to work for 2.

ilanc · 2015-05-25T12:13:53Z

I tried adding my netreturn email address to my github profile instead and even set it as my primary email address. I then resigned the CLA. Unless there is something I need to do in order to get the CLA check to re-run, this appears to not have worked either.

ilanc · 2015-05-25T12:18:08Z

Am i supposed to use filter-branch or rebase -i? as per:

razvanphp · 2015-05-25T12:18:34Z

@ilanc now that you have your netreturn email registered for CLA, you only have to push a change to the branch so that the CLA check runs again. For this, I suggest you fix the spacing, by using tabs instead of spaces, because this seems to be standard for jquery repos.

ilanc · 2015-05-25T12:19:56Z

Ok thanks will give it a go

razvanphp · 2015-05-25T12:25:37Z

https://contribute.jquery.org/style-guide/js/#spacing

You also need to fix the merge conflicts, btw.

ilanc · 2015-05-25T13:07:18Z

@razvanphp - now I can't commit?

c:\dev\jquery-ui>git commit
warning: CRLF will be replaced by LF in ui/resizable.js.
The file will have its original line endings in your working directory.
Invalid commit message, please fix the following issues:

- First line (subject) must indicate the component

Commit message was:

corrected indentation to tab

I tried the following comments with no luck:

Fixes 11135 - corrected indentation to tab
Fixes #11135 - corrected indentation to tab

razvanphp · 2015-05-25T13:17:12Z

Looking at the commit history it must start with Resizable: now.
https://github.com/jquery/jquery-ui/commits/master

If he checks for older commits, you might have to rebase unfortunately:
globalizejs/globalize#320 (comment)

ilanc · 2015-05-25T13:20:01Z

Guys - like I said - I tried. I'm sorry I can't give up a whole day to checking in 2 lines of code. If that means that this stays unfixed for all eternity then so be it. What an utter waste of time.

razvanphp · 2015-05-25T13:25:09Z

@ilanc then you can close this PR and I will create a new one, starting from current master. Thank you for your time!

ilanc · 2015-05-25T13:30:16Z

Sorry - I'm really busy with other stuff. Thanks for trying to help.

AllNamesRTaken · 2015-09-03T09:27:01Z

Great fix! Solved everything for me. All Cred to you ilanc.

szmarci · 2016-03-01T18:17:46Z

@ilanc Thanks for the fix! It worked great, until I added ghost or containment or helper options to the resizable object, then it started to shrink again.

FezVrasta · 2016-07-26T15:49:44Z

Is it possible that this bug is still on production? How is it possible?

lumberman · 2016-09-17T02:01:22Z

Still have this problem.

FezVrasta · 2016-09-17T02:08:07Z

I ended up patching manually my version... this is ridicolus

arschmitz · 2016-09-18T15:35:59Z

We will happily accept a complete, working, mergeable PR for this, so far we have not received one.

FezVrasta · 2016-09-18T16:24:06Z

This one worked just fine.

ucjonathan · 2016-12-02T11:28:53Z

Also used this fix. Would like to see resizable understand border-box without having to use a custom copy and change it.

FezVrasta · 2016-12-02T11:40:25Z

I understand why everybody stopped using jQuery in favor of other solutions... lol

We have a working PR that doesn't get merged for no reasons.

Fixes 11135 - resizeable issue with border-box

6ed4d43

If you apply .resizeable() to an element which has border-box sizing and this element has a border then the container will "shrink" by this border size immediately as the resizing drag begins.

jquerybot added the CLA: Error label Feb 19, 2015

Fixes 10948 and 8932

9fa0867

alsoResize with border-box sizing

scottgonzalez mentioned this pull request Feb 21, 2015

Better error reporting for invalid signatures jquery/jquery-license#17

Closed

scottgonzalez reviewed Feb 23, 2015
View reviewed changes

ilanc closed this May 25, 2015

razvanphp mentioned this pull request May 25, 2015

Resizable: Fix issue with border-box #1563

Closed

mcanepa mentioned this pull request Sep 22, 2021

border-box and dialog resizing problem #1979

Closed

Fixes 11135 - resizeable issue with border-box #1451

Fixes 11135 - resizeable issue with border-box #1451

Uh oh!

Conversation

ilanc commented Feb 19, 2015

Uh oh!

razvanphp commented Feb 20, 2015

Uh oh!

ilanc commented Feb 21, 2015

Uh oh!

scottgonzalez commented Feb 21, 2015

Uh oh!

scottgonzalez Feb 23, 2015

Choose a reason for hiding this comment

Uh oh!

ilanc Feb 24, 2015

Choose a reason for hiding this comment

Uh oh!

ilanc Feb 24, 2015

Choose a reason for hiding this comment

Uh oh!

lqixv Apr 23, 2015

Choose a reason for hiding this comment

Uh oh!

razvanphp commented Mar 2, 2015

Uh oh!

ilanc commented Mar 3, 2015

Uh oh!

razvanphp commented Mar 20, 2015

Uh oh!

thisjustin commented May 24, 2015

Uh oh!

scottgonzalez commented May 24, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

razvanphp commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

razvanphp commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

razvanphp commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

razvanphp commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

razvanphp commented May 25, 2015

Uh oh!

ilanc commented May 25, 2015

Uh oh!

AllNamesRTaken commented Sep 3, 2015

Uh oh!

szmarci commented Mar 1, 2016

Uh oh!

FezVrasta commented Jul 26, 2016

Uh oh!

lumberman commented Sep 17, 2016

Uh oh!

FezVrasta commented Sep 17, 2016

Uh oh!

arschmitz commented Sep 18, 2016

Uh oh!

FezVrasta commented Sep 18, 2016

Uh oh!

ucjonathan commented Dec 2, 2016

Uh oh!

FezVrasta commented Dec 2, 2016

Uh oh!

Uh oh!