Opened 15 years ago
Closed 14 years ago
#4083 closed defect (fixed)
dojo.fx.chain/combine destructive
Reported by: | dante | Owned by: | Eugene Lazutkin |
---|---|---|---|
Priority: | high | Milestone: | 1.1 |
Component: | fx | Version: | 0.9 |
Keywords: | Cc: | Bryan Forbes, alex, tk | |
Blocked By: | Blocking: |
Description
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)
Attachments (1)
Change History (8)
comment:1 Changed 15 years ago by
Owner: | changed from alex to Eugene Lazutkin |
---|
comment:2 Changed 15 years ago by
Cc: | Adam Peller removed |
---|
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])
comment:3 Changed 15 years ago by
Cc: | Bryan Forbes alex tk added |
---|
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.
comment:4 Changed 15 years ago by
Milestone: | 1.0 → 1.1 |
---|
comment:5 Changed 14 years ago by
I added two tests to the fx tests in [12085]. These fail because of how combine and chain are currently implemented.
comment:6 Changed 14 years ago by
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.
comment:7 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
This should go to Eugene I think. Reassigning...