add done argument to onEntering and onExiting

[aholachek]

Hi,
I've been missing the functionality from V1 which passed in a callback to componentWillLeave that could be called on conclusion of the exit animation to signal that the element could be removed from the DOM. Having to provide a timeout in advance, or a listener on the DOM node itself, as one currently has to do for the V2 version, is not quite as convenient for JS animations.

In this pr, I added done function arguments to the onEntering and onExiting callbacks so that those functions could directly signal when transitions were done.

Right now, since if you are using the done callback you neither need to provide a timeout prop nor a addEndListener prop, I removed the required from the timeout prop. If you think this PR is a good direction to go in I would love some advice about whether this was the right thing to do with props.

[taion]

won't e.g. onEntered get called twice if the user both manually triggers the transition end and the timeout fires? i think, then, that this should hook into that weird "next callback" mechanism. i wish i clearly remembered how that actually worked well enough to give you a real suggestion on implementation, though

[silvenon]

CI is failing because the commit message doesn't adhere to the preferred format; to make this easier for contributors, I pushed some changes to master branch which will install a git hook and lint messages of your subsequent commits.

To lint the message of your last commit run npx commitlint -e. To see more options run npx commitlint --help.

[mvasin]

@aholachek Have a look at the issue #389 – @jquense is strictly against adding done to onEntering/onExiting, and we agreed to add transition status param to addEndListener (see my PR #396). But after I discovered the workaround I decided not to break the API and close my pull request.

I'm not sure this PR is also worth keeping open unless Jason changes his mind.

@aholachek Will the workaround I've mentioned work for you?

[aholachek]

@mvasin thank you for looking into this problem, the workaround is ok for single elements (where I have access to the in prop value) but I don't understand how I would get it working for children of the TransitionGroup (where the in prop is not accessible if I understand correctly).

If I could figure out how to get it working for TransitionGroup, I would use your workaround instead of what I'm currently doing, which is triggering events on dom nodes from my JS animation. However, even if I understood how to get it to work, I do wish there were a clean, self-evident way for an external animation function to trigger a removal on an element after it has finished, without needing to use the workaround. It would be a little less confusing.

Thank you again for looking in to how to do this! I will close my pr since it seems like it's not the right direction to go in.

[abernier]

@mvasin thank you for looking into this problem, the workaround is ok for single elements (where I have access to the in prop value) but I don't understand how I would get it working for children of the TransitionGroup (where the in prop is not accessible if I understand correctly).

If I could figure out how to get it working for TransitionGroup, I would use your workaround instead of what I'm currently doing, which is triggering events on dom nodes from my JS animation. However, even if I understood how to get it to work, I do wish there were a clean, self-evident way for an external animation function to trigger a removal on an element after it has finished, without needing to use the workaround. It would be a little less confusing.

Thank you again for looking in to how to do this! I will close my pr since it seems like it's not the right direction to go in.

Could also be safier to removeEventListener once done :

addEndListener={(node, done) => {
  function reacttransitionend() {
    node.removeEventListener('reacttransitionend', reacttransitionend);
    done();
  }
  node.addEventListener('reacttransitionend', reacttransitionend);
}}

Working demo : https://codepen.io/abernier/pen/pYKjPr?editors=0010