Skip to content

Commit

Permalink
Widget: Increase performance for large sortable/draggable collections
Browse files Browse the repository at this point in the history
  • Loading branch information
mgol committed Nov 19, 2024
1 parent f76bbcd commit c2e2519
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 29 deletions.
18 changes: 9 additions & 9 deletions tests/unit/widget/classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,27 +155,27 @@ QUnit.test( "Classes elements are untracked as they are removed from the DOM", f
var widget = $( "#widget" ).classesWidget();
var instance = widget.classesWidget( "instance" );

assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 3,
assert.equal( instance.classesElementLookup[ "ui-classes-span" ].size, 3,
"Widget is tracking 3 ui-classes-span elements" );
assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 3,
assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].size, 3,
"Widget is tracking 3 ui-core-span-null elements" );
assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 3,
assert.equal( instance.classesElementLookup[ "ui-core-span" ].size, 3,
"Widget is tracking 3 ui-core-span elements" );

widget.find( "span" ).eq( 0 ).remove();
assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 2,
assert.equal( instance.classesElementLookup[ "ui-classes-span" ].size, 2,
"After removing 1 span from dom 2 ui-classes-span elements are tracked" );
assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 2,
assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].size, 2,
"After removing 1 span from dom 2 ui-core-span-null elements are tracked" );
assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 2,
assert.equal( instance.classesElementLookup[ "ui-core-span" ].size, 2,
"After removing 1 span from dom 2 ui-core-span elements are tracked" );

widget.find( "span" ).remove();
assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 0,
assert.equal( instance.classesElementLookup[ "ui-classes-span" ].size, 0,
"No ui-classes-span elements are tracked after removing all spans" );
assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 0,
assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].size, 0,
"No ui-core-span-null elements are tracked after removing all spans" );
assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 0,
assert.equal( instance.classesElementLookup[ "ui-core-span" ].size, 0,
"No ui-core-span elements are tracked after removing all spans" );
} );

Expand Down
1 change: 1 addition & 0 deletions ui/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
},

"env": {
"es6": true,
"browser": true,
"jquery": true,
"node": false
Expand Down
56 changes: 36 additions & 20 deletions ui/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,10 @@ $.Widget.prototype = {
this.uuid = widgetUuid++;
this.eventNamespace = "." + this.widgetName + this.uuid;

this.bindings = $();
this.bindings = new Set();
this.hoverable = $();
this.focusable = $();
this.classesElementLookup = {};
this.classesElementLookup = Object.create( null );

if ( element !== this ) {
$.data( element, this.widgetFullName, this );
Expand Down Expand Up @@ -355,7 +355,7 @@ $.Widget.prototype = {

this._destroy();
$.each( this.classesElementLookup, function( key, value ) {
that._removeClass( value, key );
that._removeClass( $( Array.from( value ) ), key );
} );

// We can probably remove the unbind calls in 2.0
Expand All @@ -368,7 +368,7 @@ $.Widget.prototype = {
.removeAttr( "aria-disabled" );

// Clean up events and states
this.bindings.off( this.eventNamespace );
$( Array.from( this.bindings ) ).off( this.eventNamespace );
},

_destroy: $.noop,
Expand Down Expand Up @@ -450,16 +450,12 @@ $.Widget.prototype = {
currentElements = this.classesElementLookup[ classKey ];
if ( value[ classKey ] === this.options.classes[ classKey ] ||
!currentElements ||
!currentElements.length ) {
!currentElements.size ) {
continue;
}

// We are doing this to create a new jQuery object because the _removeClass() call
// on the next line is going to destroy the reference to the current elements being
// tracked. We need to save a copy of this collection so that we can add the new classes
// below.
elements = $( currentElements.get() );
this._removeClass( currentElements, classKey );
elements = $( Array.from( currentElements ) );
this._removeClass( elements, classKey );

// We don't use _addClass() here, because that uses this.options.classes
// for generating the string of classes. We want to use the value passed in from
Expand Down Expand Up @@ -509,7 +505,7 @@ $.Widget.prototype = {
return elements;
} )
.some( function( elements ) {
return elements.is( element );
return elements.has( element );
} );

if ( !isTracked ) {
Expand All @@ -525,12 +521,24 @@ $.Widget.prototype = {
function processClassString( classes, checkOption ) {
var current, i;
for ( i = 0; i < classes.length; i++ ) {
current = that.classesElementLookup[ classes[ i ] ] || $();
current = that.classesElementLookup[ classes[ i ] ] || new Set();
if ( options.add ) {
bindRemoveEvent();
current = $( $.uniqueSort( current.get().concat( options.element.get() ) ) );

// This function is invoked synchronously, so the reference
// to `current` is not an issue.
// eslint-disable-next-line no-loop-func
$.each( options.element, function( _i, node ) {
current.add( node );
} );
} else {
current = $( current.not( options.element ).get() );

// This function is invoked synchronously, so the reference
// to `current` is not an issue.
// eslint-disable-next-line no-loop-func
$.each( options.element, function( _i, node ) {
current.delete( node );
} );
}
that.classesElementLookup[ classes[ i ] ] = current;
full.push( classes[ i ] );
Expand All @@ -553,9 +561,7 @@ $.Widget.prototype = {
_untrackClassesElement: function( event ) {
var that = this;
$.each( that.classesElementLookup, function( key, value ) {
if ( $.inArray( event.target, value ) !== -1 ) {
that.classesElementLookup[ key ] = $( value.not( event.target ).get() );
}
value.delete( event.target );
} );

this._off( $( event.target ) );
Expand Down Expand Up @@ -583,6 +589,7 @@ $.Widget.prototype = {
},

_on: function( suppressDisabledCheck, element, handlers ) {
var i;
var delegateElement;
var instance = this;

Expand All @@ -600,7 +607,11 @@ $.Widget.prototype = {
delegateElement = this.widget();
} else {
element = delegateElement = $( element );
this.bindings = this.bindings.add( element );
for ( i = 0; i < element.length; i++ ) {
$.each( element, function( _i, node ) {
instance.bindings.add( node );
} );
}
}

$.each( handlers, function( event, handler ) {
Expand Down Expand Up @@ -637,12 +648,17 @@ $.Widget.prototype = {
},

_off: function( element, eventName ) {
var that = this;

eventName = ( eventName || "" ).split( " " ).join( this.eventNamespace + " " ) +
this.eventNamespace;
element.off( eventName );

$.each( element, function( _i, node ) {
that.bindings.delete( node );
} );

// Clear the stack to avoid memory leaks (#10056)
this.bindings = $( this.bindings.not( element ).get() );
this.focusable = $( this.focusable.not( element ).get() );
this.hoverable = $( this.hoverable.not( element ).get() );
},
Expand Down

0 comments on commit c2e2519

Please sign in to comment.