Only stop if the transition on this element ended #1301
Conversation
This way it doesn't end when some random element elsewhere on the page has an ending transition. It must be noted that this is not a complete fix. If you have multiple transitions on the element, it still ends when the first ends. I ran into this not working (the transition ends almost immediately, because some unrelated transition ends). This fixes it for me, but I was wondering whether this cornercase was considered, and whether the other cornercase (multiple transitions) was considered. I think it's very hard to get this correct, and think it might be better to make the timeout explicit, or at least allow to set it explicit, and fall back to this behaviour. But I'm really interested in feedback on this one.
|
Ah yes, #669 though. |
|
Note that endListener can also be called immediately without arguments if CSS transitions are not supported: https://github.com/facebook/react/blob/5476f91/src/addons/transitions/ReactTransitionEvents.js#L77-L79 |
|
@markijbema Can you add the missing |
|
Hey Ben, You're right that I didn't test this PR adequately. I extracted it from some internal fiddling around. Unfortunately I don't have a working setup at the moment to develop on react itself. I believe these two changes should fix both comments, but as said, I cannot currently test it. |
|
@chenglou - you've been looking at this transition stuff, can you tell if this is still needed? |
|
I believe this is still correct. |
|
👍 |
|
And I'm finally coming back around to this, sorry! @markijbema Could you sign the CLA? https://code.facebook.com/cla. Then this should be good to go. |
|
Done. |
Only stop if the transition on this element ended
|
Oy, that took too long, but it's in. Thanks for your infinite patience! |
|
👍 |
6 participants
This way it doesn't end when some random element elsewhere on the page has an ending transition.
It must be noted that this is not a complete fix. If you have multiple transitions on the element, it still ends when the first ends.
I ran into this not working (the transition ends almost immediately, because some unrelated transition ends). This fixes it for me, but I was wondering whether this cornercase was considered, and whether the other cornercase (multiple transitions) was considered.
I think it's very hard to get this correct, and think it might be better to make the timeout explicit, or at least allow to set it explicit, and fall back to this behaviour. But I'm really interested in feedback on this one.