TimeoutTransitionGroup is now using timeouts read from css

[davidreher]

I stumbled over your implementation of TransitionGroup and saw the text at the top of the file. Since I did not want to use hard coded timeouts in my application, I implemented the suggested changes.

Unfortunately I had to use jQuery for that to have easy support of cross browser compability (especially referring to transition, -webkit-transition ...)

[jaredly]

personally I'd like to avoid adding a jquery dependency (I don't think jQuery is used anywhere else in the codebase)

[davidreher]

I will try to work on that ... but you refer to it as needed in the documentation so I thought that would not be a big deal ...

[ariabuckles]

Hi @davidreher!

Thanks for taking the time to send us this :heart: :smile:.

Unfortunately I'm probably not going to merge this right now :disappointed:. There are a lot of moving parts, and I'm sort of the reluctant maintainer of this (I know a lot of the components in this repo well, but TimeoutTransitionGroup isn't one of them) [if someone else at KA is reading this and wants to own this PR, go for it!]. I don't know our uses of this well and am hesitant to make major changes that could affect parts of the site I'm less familiar with.

If this is really important to you, I'm pretty susceptible to persuasion (and time; unfortunately I only have a few cycles for open source things), and I can give this more thorough consideration if you feel this is important enough to merit it and make the following changes:

1. I'd like to see a compelling argument for why this is the right approach. I'm not sure reading from CSS is the right thing over, say, pulling all the CSS into javascript-land with inline styles / RCSS / react-style. How bad is reading styles off the DOM when they're loaded, and does this cause lag for the first animation?
2. I'd like the changeset to be in the same style as the source file--no blank lines with indentation, no spaces between function and (, standard indentation (putting } else { on the same level as if, not one level greater, JSX attributes indented as in the source. It should also follow the style guides at https://github.com/Khan/style-guides .
3. I'd like the diff to be minimal. This is sort of related to the above; reviewing a change that is half semantics half style is challenging. The change should only touch relevant lines.
4. The change should be easy to follow. This is probably the most subjective and maybe hardest, but the idea is that the diff view itself should convey what is changing and why. If there are both refactoring changes (_wrapChild) and semantics changes, those should be in separate commits so that it's easy to see that the refactorings are simple and correct and the changes are simple and correct. Helper functions for several lines that are related (I suspect a lot of the logic in componentDidMount could be pulled out into helper functions), and comments for things that aren't obvious at the point at which you're reading them (knownCssTimings) also go a long way here
5. It needs to be correct and non-scary. This one falls out of the others pretty naturally--once the change is minimal, looking it over for correctness and scary things is much easier for everyone involved :). It's hard to look at this in-depth before then, but one thing that's probably worth looking into is componentDidMount: doing props-based logic in componentDidMount probably means we need to do the same logic in componentDidUpdate

(I'm not sure my stance on the jQuery thing; we literally had a pull request a couple weeks ago to remove it, and I'm not sure how to balance not having dependencies vs features. Maybe the right answer would actually be a different/separate component that depends on jQuery. Sorry about that mess!)

This response will probably come across more harshly than I intend, and I apologize for that! It's awesome that you're using our code and extending it to do what you need. Go fork and prosper! I just probably won't be able to merge this here because of time :disappointed:. But I wanted to provide a list of some suggestions for if you'd find those helpful or if getting this merged is really important to you. Otherwise, I fully recommend forking and using your own copy, and celebrating :smile:.

Again, thanks for taking the time to submit this upstream :heart:. It's nice to see that other people are using and working with this code, and I hope it's of some use to you!