Widget: Increase performance for large sortable/draggable collections

Fixes gh-2062
This commit is contained in:
Michał Gołębiowski-Owczarek 2024-11-19 22:13:41 +01:00
parent f76bbcd512
commit c2e25190ef
No known key found for this signature in database
3 changed files with 46 additions and 29 deletions

View File

@ -155,27 +155,27 @@ QUnit.test( "Classes elements are untracked as they are removed from the DOM", f
var widget = $( "#widget" ).classesWidget(); var widget = $( "#widget" ).classesWidget();
var instance = widget.classesWidget( "instance" ); 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" ); "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" ); "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 is tracking 3 ui-core-span elements" );
widget.find( "span" ).eq( 0 ).remove(); 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" ); "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" ); "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" ); "After removing 1 span from dom 2 ui-core-span elements are tracked" );
widget.find( "span" ).remove(); 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" ); "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" ); "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" ); "No ui-core-span elements are tracked after removing all spans" );
} ); } );

View File

@ -8,6 +8,7 @@
}, },
"env": { "env": {
"es6": true,
"browser": true, "browser": true,
"jquery": true, "jquery": true,
"node": false "node": false

View File

@ -301,10 +301,10 @@ $.Widget.prototype = {
this.uuid = widgetUuid++; this.uuid = widgetUuid++;
this.eventNamespace = "." + this.widgetName + this.uuid; this.eventNamespace = "." + this.widgetName + this.uuid;
this.bindings = $(); this.bindings = new Set();
this.hoverable = $(); this.hoverable = $();
this.focusable = $(); this.focusable = $();
this.classesElementLookup = {}; this.classesElementLookup = Object.create( null );
if ( element !== this ) { if ( element !== this ) {
$.data( element, this.widgetFullName, this ); $.data( element, this.widgetFullName, this );
@ -355,7 +355,7 @@ $.Widget.prototype = {
this._destroy(); this._destroy();
$.each( this.classesElementLookup, function( key, value ) { $.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 // We can probably remove the unbind calls in 2.0
@ -368,7 +368,7 @@ $.Widget.prototype = {
.removeAttr( "aria-disabled" ); .removeAttr( "aria-disabled" );
// Clean up events and states // Clean up events and states
this.bindings.off( this.eventNamespace ); $( Array.from( this.bindings ) ).off( this.eventNamespace );
}, },
_destroy: $.noop, _destroy: $.noop,
@ -450,16 +450,12 @@ $.Widget.prototype = {
currentElements = this.classesElementLookup[ classKey ]; currentElements = this.classesElementLookup[ classKey ];
if ( value[ classKey ] === this.options.classes[ classKey ] || if ( value[ classKey ] === this.options.classes[ classKey ] ||
!currentElements || !currentElements ||
!currentElements.length ) { !currentElements.size ) {
continue; continue;
} }
// We are doing this to create a new jQuery object because the _removeClass() call elements = $( Array.from( currentElements ) );
// on the next line is going to destroy the reference to the current elements being this._removeClass( elements, classKey );
// 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 );
// We don't use _addClass() here, because that uses this.options.classes // 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 // for generating the string of classes. We want to use the value passed in from
@ -509,7 +505,7 @@ $.Widget.prototype = {
return elements; return elements;
} ) } )
.some( function( elements ) { .some( function( elements ) {
return elements.is( element ); return elements.has( element );
} ); } );
if ( !isTracked ) { if ( !isTracked ) {
@ -525,12 +521,24 @@ $.Widget.prototype = {
function processClassString( classes, checkOption ) { function processClassString( classes, checkOption ) {
var current, i; var current, i;
for ( i = 0; i < classes.length; i++ ) { for ( i = 0; i < classes.length; i++ ) {
current = that.classesElementLookup[ classes[ i ] ] || $(); current = that.classesElementLookup[ classes[ i ] ] || new Set();
if ( options.add ) { if ( options.add ) {
bindRemoveEvent(); 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 { } 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; that.classesElementLookup[ classes[ i ] ] = current;
full.push( classes[ i ] ); full.push( classes[ i ] );
@ -553,9 +561,7 @@ $.Widget.prototype = {
_untrackClassesElement: function( event ) { _untrackClassesElement: function( event ) {
var that = this; var that = this;
$.each( that.classesElementLookup, function( key, value ) { $.each( that.classesElementLookup, function( key, value ) {
if ( $.inArray( event.target, value ) !== -1 ) { value.delete( event.target );
that.classesElementLookup[ key ] = $( value.not( event.target ).get() );
}
} ); } );
this._off( $( event.target ) ); this._off( $( event.target ) );
@ -583,6 +589,7 @@ $.Widget.prototype = {
}, },
_on: function( suppressDisabledCheck, element, handlers ) { _on: function( suppressDisabledCheck, element, handlers ) {
var i;
var delegateElement; var delegateElement;
var instance = this; var instance = this;
@ -600,7 +607,11 @@ $.Widget.prototype = {
delegateElement = this.widget(); delegateElement = this.widget();
} else { } else {
element = delegateElement = $( element ); 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 ) { $.each( handlers, function( event, handler ) {
@ -637,12 +648,17 @@ $.Widget.prototype = {
}, },
_off: function( element, eventName ) { _off: function( element, eventName ) {
var that = this;
eventName = ( eventName || "" ).split( " " ).join( this.eventNamespace + " " ) + eventName = ( eventName || "" ).split( " " ).join( this.eventNamespace + " " ) +
this.eventNamespace; this.eventNamespace;
element.off( eventName ); element.off( eventName );
$.each( element, function( _i, node ) {
that.bindings.delete( node );
} );
// Clear the stack to avoid memory leaks (#10056) // Clear the stack to avoid memory leaks (#10056)
this.bindings = $( this.bindings.not( element ).get() );
this.focusable = $( this.focusable.not( element ).get() ); this.focusable = $( this.focusable.not( element ).get() );
this.hoverable = $( this.hoverable.not( element ).get() ); this.hoverable = $( this.hoverable.not( element ).get() );
}, },