Resizable: Refactored resize handler. Fixed #5545, #5817, #7605. #694

Closed
wants to merge 4 commits into
from

Projects

None yet

3 participants

@eromba
eromba commented Jul 10, 2012

This pull request includes a refactoring of Resizable's resize handler that fixes 3 reported issues. The fixes build upon one another, so I am including them together in one request. I've included appropriate test cases for each issue, as well.

Resizable now modifies only the CSS properties that are actually necessary to perform the resize (instead of always setting top, left, width, and height regardless; see #7605). This makes the plugin behave more predictably in dynamic layouts.

Furthermore, the plugin's resize event is now altogether more useful:

  • The event is triggered only when the element's size is modified (as opposed to every mousemove; see #5545)
  • The ui callback parameter now correctly reports the element's current size (see #5817)
@scottgonzalez scottgonzalez commented on an outdated diff Jul 16, 2012
ui/jquery.ui.resizable.js
if (!this._helper && this._proportionallyResizeElements.length)
this._proportionallyResize();
- this._updateCache(data);
-
- // calling the user callback at the end
- this._trigger('resize', event, this.ui());
+ // Call the user callback if the element was resized
+ if ( ! $.isEmptyObject(props) ) {
@scottgonzalez
scottgonzalez Jul 16, 2012 jQuery member

Can you please change this to use a flag inside each fo the conditionals above? $.isEmptyObject() wasn't added until jQuery 1.4, and if we land this change it'll need to go into 1-8-stable as well. Otherwise this will need to wait until 1.10 when we introduce breaking changes.

@eromba
eromba commented Jul 18, 2012

This should now be ready to go for both master and 1.8-stable.

@mikesherov
Member

Hi @eromba, thanks again for contributing this patch. We recently rewrote the test suite for resizable. Can you rebase this please?

@eromba
eromba commented Nov 8, 2012

I'm closing this pull request since I'll be moving the rebased commits to a new, dedicated branch.

@eromba eromba closed this Nov 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment