search

Khan / react-components

khan.github.io/react-components/
MIT License
1.01k stars 99 forks source link

Use CSSCore instead of jQuery in TimeoutTransitionGroup #22

Closed moretti closed 9 years ago

moretti commented 9 years ago

@spicyj Thanks, I've updated it in my fork

moretti commented 9 years ago

@spicyj changed to 4 spaces, sorry. Tough day :smiley:

joelburget commented 9 years ago

@moretti as a heads-up I think this looks good now, but I'll need to figure out how to make this work with our internal javascript bundling / module resolver.

moretti commented 9 years ago

Thanks @joelburget, updated with an arrow function

devgeeks commented 9 years ago

Is this likely to get pulled in?

nelix commented 9 years ago

I'm pretty eager to have this too.

ariabuckles commented 9 years ago

Hey all,

Thanks for the pull request and support. I'd like to get this pulled in if we can.

Unfortunately, in the current state I can't merge this yet. Reaching into the react internal files only works if the entire app is using browserify/webpack (or in node/io.js), and doesn't work if people are using react as a single file [minified] library. Currently this is how we integrate react, which is not likely to change but might be possible.

Secondarily, this depends on React not changing their internal implementation, which is unideal.

Since we're only using CSSCore for adding/removing classes, maybe we could implement this without the CSSCore dependency or jQuery? Or pull in the appropriate code into react-components?

I'm also open to looking into how we integrate React such that such requires work, but that is likely to be on a longer timeframe (and requires modifying a lot more than just react-components).

Thanks, Aria

devgeeks commented 9 years ago

As a Browserify user, that hadn't occurred to me, but I would be happy to make a new PR with no dep on CSSCore (straight up vanilla-js).

ariabuckles commented 9 years ago

@devgeeks awesome; I'd be happy to merge that!

sophiebits commented 9 years ago

It may be easier to just embed CSSCore.js into react-components:

https://github.com/facebook/react/blob/master/src/vendor/core/CSSCore.js

(Perhaps with the calls to invariant changed.)

ariabuckles commented 9 years ago

(I'm okay with that too!)

devgeeks commented 9 years ago

Maybe that's the approach I will use. Basically include CSSCore, but with normal error handling instead of invariant.

ariabuckles commented 9 years ago

Hi! Thanks for doing this! I'll add a couple of comments inline.

ariabuckles commented 9 years ago

Er... on #31 :). I'm going to close this PR in favor of #31. Thanks everyone for contributing and the discussion.