Resizable: modified resizable containment plugin to restrict resizing within container. Fixes #7018 Resizable: resizing can move objects

[dekajp]

Fixes #9107 Resizable: aspectRatio and containment not handled correctly
Fixes #7018 Resizable: resizing can move objects

Just want to check if this approach is ok ? for fiddle link - http://jsfiddle.net/dekajp/c2Rux/14/

adding prevSize and prevPosition in the plugin parameter object. so that if resize breaks then it can be restored.

this PR is not complete - does not include unit testing. ( Fixes #7018 ). also i found some minor code style issues , so i fixed them.

[mikesherov]

The approach seems fine to me. I have some minor nits, which I'll report now just to avoid saying them later:

1. A lot of the comments you're fixing are unnecessary to begin with. Rather than fix them, remove them if they don't mention a ticket number or a browser they're supporting. If they do mention a browser, please augment the comment to include a line: // support: IE. There a lots of examples in the code base.
2. Your style of declaring an empty object and then assigning individual properties should be replaced with a single declaration with the properties defined during the declaration.

[dekajp]

@mikesherov added unit test cases and your suggested changes.

[mikesherov]

ticket-7018 -> #7018

[jzaefferer]

We should avoid ticket references in unit tests. Describe what the test is about instead, like all other tests do.

[scottgonzalez]

Yes. The test name should be descriptive of the test. Ticket references should be in comments.

[dekajp]

Is this a good name aspectRatio:position ? i am very bad in naming variables

[mikesherov]

I was under the impression that test names should reference ticket number along with good description. I thought we were just getting rid of *_tickets.js. It's helpful to see the ticket number in the runner output.

[scottgonzalez]

I'm ok with that, as long as the test name is descriptive. I just don't think the fact that a ticket existed is actually important in the output.

[mikesherov]

@scottgonzalez I'm fine either way, but I do personally think the fact that a ticket existed is important, I always look back at fiddles.

[dekajp]

removed ticket ref from test output, but added #ticket as comment in code.

[mikesherov]

@dekajp, thanks again for all your hard work! I did a review and once you make those minor changes, I can land this!

[dekajp]

@mikesherov thank you !! whats your preference - do you want me to push new changes in separate commit or you want me to update original commit ?

[mikesherov]

@dekajp single updated commit is preferred.

[dekajp]

@mikesherov seems http://bugs.jqueryui.com/ticket/9107 is related to this . here is the fixed fiddle -http://jsfiddle.net/Mm2Ed/3/ ( minor change - continueResize=false in re calculation of left and top coord.

do you want me to add this change in this commit or i can submit a new PR based on this .

[mikesherov]

Add it to this, but edit the commit message. Thanks again for all your hard work!

[dekajp]

I am trying to add a test case - example in fiddle (http://jsfiddle.net/c2Rux/16/) it seems that i can not simulate this

the last drag [-10,-10] does not work !!

[dekajp]

ah !! i think i got the problem , i created some room for mouse move by making the container absolute left:100 ,top:100 after that it worked