dojo.fx.chain/combine destructive

[dante]

dojo.fx.combine mangles passed _Animations:

var anim1 = new dojo._Animation(); ... var anim4 = dojo.fx.combine([anim1,anim2,anim3]); anim1.play(); // === anim4.play(); === anim2.play() etc.

the code looks like it only clobbers the first animation, but affects all.

if fx.combine would return a new _Animation of the combined effects, the issue would be resolved?

dojo.fx.chain should probably return a new _Animation as well:

var anim4 = dojo.fx.chain([anim1,anim2,anim3]); anim1.play(); // === anim4.play()

this does seem to be entirely intuitive behavior. (seems like anim1 should still be anim1, and anim4 should be the resulting animation of the chain, or even more simple: a 'null' animation that connects to onEnd of each, and plays each other animation in order)

[bill]

This should go to Eugene I think. Reassigning...

[dante]

as a testcase, you can use dojox/fx/tests/test_easing.html ... there is a commented out test in the addOnLoad code (in rev > [10073])

[peller]

Kind of a dup of #3477, though I think this might be a bit clearer. There are related issues, such as the way the onEnd event is fired -- not at all what I expected.

[BryanForbes]

I added two tests to the fx tests in [12085]. These fail because of how combine and chain are currently implemented.

[BryanForbes]

Fix for chain and combine bugs.

[BryanForbes]

I added a patch to fix the chain and combine bugs. The only way I could think to fix these was to create a duck-typed Animation object and return that. The reason being that there's no good way to make either chain or combine work with just the animations given to the functions.

With chain, we're running into the problem that the returned animation is the first animation. This works fine for playing, but not for connecting to the onEnd handler. It also doesn't handle pausing, stopping, etc. the chained animation.

With combine, we were using a "placeholder" animation that doesn't keep track of if the animations it is combining have ended, it just gives a rough guess (and a poor one as you can see by the test I uploaded in [12085]). It may only be milliseconds off, but when you rely on that calculation to do something like destroy a dialog that you're doing an animation on it's critical so you don't get "node doesn't exist" errors. The other problem that combine had was animations with differing lengths. It set up the "placeholder" animation with the duration of the first animation, but if the second animation's duration was longer it would fire "onEnd" before the combine was truly done.

[elazutkin]

(In [12098]) dojo.fx: new versions of dojo.fx.combine() and dojo.fx.chain() based on Bryan Forbes' patch. Minor cleanup, new fx test. Fixes #4083. Thank you, Bryan!