Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9227 closed bug (duplicate)

Poor Performance of animateClass with children - 500 elements > 1 second pause

Reported by: BenEllis Owned by:
Priority: minor Milestone: none
Component: ui.effects.core Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:

Description

My use case is I'm displaying a conversation history (100s - 1000s of lines) and the user wants the ability to minimize, restore and maximize the view. Other controls inside the container change size.

Below is a contrived example of what I'm trying to achieve,

http://jsfiddle.net/BenEllis/4Gwe9/3/

When using

$('div.outer').addClass('myanimatedclass', {
    children: true,
    duration: 1000
});

if div.outer has many children the performance is slow. Could the performance be improved by using the CSS rules to build a selector of children that would be affected by the classes being added / removed?

Change History (5)

comment:1 Changed 6 years ago by Scott González

Resolution: duplicate
Status: newclosed

Duplicate of #9226.
You've presented the exact same fiddle and use case as before. This seems like a bad example. For instance, the fact that you want to hide the input isn't really tied to how that should animate. Please don't intentionally create duplicate tickets.

comment:2 Changed 6 years ago by BenEllis

Why does a performance issue require a use-case for a feature that already exists? I would have thought having 100+ child elements in fairly common?

I've done an initial working implementation to address the performance problem, it only selects children affected by the change of the classes before getting all the styles. This has really improved the performance when there are >100 children.

This works perfectly for me (IE9, FF 20.0.1, Chrome 26.0.1410.64 m), though there may be a few bugs for CSS rules I've not considered as I've manually parsed the CSS rules. I'm guessing there is a better selector or helper methods in jQuery that would do a better job than I've done.

$.effects.animateClass = function( value, duration, easing, callback ) {
	var o = $.speed( duration, easing, callback );

	return this.queue(function () {
	    
	    var getSelectors = function () {
	        var retVal = '';
	        $.each(classAnimationActions, function (i, action) {
	            if (value[action]) {
	                var classNames = value[action].match(/[^,]+(?=,)|[^,]+(?=$)/gim);
	                $.each(classNames, function (i, className) {
	                    var sheets = document.styleSheets;
	                    for (var sheetIndex in sheets) {
	                        var rules = sheets[sheetIndex].rules || sheets[sheetIndex].cssRules;
	                        for (var r in rules) {
	                            if (rules[r].type == window.CSSRule.STYLE_RULE && rules[r].selectorText.indexOf("." + className) != -1) {
	                                var currentrule = rules[r].selectorText;
	                                currentrule = currentrule.substring(currentrule.indexOf("." + className) + ("." + className).length);
	                                currentrule = /[^) ].*/gim.exec(currentrule);
	                                if (currentrule) {
	                                    retVal = retVal + ',' + currentrule;
	                                }
	                            }
	                        }
	                    }
	                });
	            }
	        });
	        
	        return retVal.substring(1);
	    }; 
	    
		var animated = $( this ),
			baseClass = animated.attr( "class" ) || "",
			applyClassChange,
	        allAnimations = o.children ? animated.find(getSelectors()).addBack() : animated;

		// map the animated objects to store the original styles.
		allAnimations = allAnimations.map(function() {
			var el = $( this );
			return {
				el: el,
				start: getElementStyles( this )
			};
		});

		// apply class change
		applyClassChange = function() {
			$.each( classAnimationActions, function(i, action) {
				if ( value[ action ] ) {
					animated[ action + "Class" ]( value[ action ] );
				}
			});
		};
		applyClassChange();

		// map all animated objects again - calculate new styles and diff
		allAnimations = allAnimations.map(function() {
			this.end = getElementStyles( this.el[ 0 ] );
			this.diff = styleDifference(this.start, this.end);
			return this;
		});

		// apply original class
		animated.attr( "class", baseClass );

		// map all animated objects again - this time collecting a promise
		allAnimations = allAnimations.map(function () {
		    if (this.diff != {}) {
		        var styleInfo = this,
		            dfd = $.Deferred(),
		            opts = $.extend({}, o, {
		                queue: false,
		                complete: function() {
		                    dfd.resolve(styleInfo);
		                }
		            });

		        this.el.animate(this.diff, opts);
		        return dfd.promise();
		    }
		});

		// once all animations have completed:
		$.when.apply( $, allAnimations.get() ).done(function() {

			// set the final class
			applyClassChange();

			// for each animated element,
			// clear all css properties that were animated
			$.each( arguments, function() {
				var el = this.el;
				$.each( this.diff, function(key) {
					el.css( key, "" );
				});
			});

			// this is guarnteed to be there if you use jQuery.speed()
			// it also handles dequeuing the next anim...
			o.complete.call( animated[ 0 ] );
		});
	});
};
Last edited 6 years ago by BenEllis (previous) (diff)

comment:3 in reply to:  2 Changed 6 years ago by Scott González

Replying to BenEllis:

Why does a performance issue require a use-case for a feature that already exists? I would have thought having 100+ child elements in fairly common?

Because if it's not something people are doing for real, or it's not something people *should* be doing, then it doesn't matter how poorly it performs.

comment:4 Changed 6 years ago by BenEllis

I am using this in a real commercial project. Why shouldn't I be doing it?

comment:5 in reply to:  4 Changed 6 years ago by tj.vantoll

Replying to BenEllis:

I am using this in a real commercial project. Why shouldn't I be doing it?

Simultaneously animating hundreds of elements is a bad idea. There's only so much the CPU can handle and there are better (and far less resource intensive) ways of accomplishing the same effect. If you need further support try #jquery on Freenode, the forums, or Stack Overflow.

Note: See TracTickets for help on using tickets.