reactjs / react-autocomplete

WAI-ARIA compliant React autocomplete (combobox) component
MIT License
2.17k stars 532 forks source link

[fixed] Upgraded React.createClass(...) to ES6 classes #231

Closed ryannjohnson closed 7 years ago

ryannjohnson commented 7 years ago

This PR reconciles https://github.com/reactjs/react-autocomplete/pull/225 and the current master branch.

React complains in the console because it's depreciating its React.createClass(...) syntax and separating its PropTypes module to its own package. This PR updates that.

guillaumepotier commented 7 years ago

I'd love to see this PR merged. But it seems 1) it is not synced with master anymore and 2) it brings more features than React update.

Would it be possible to fix that to hope having it merged soon ?

Thx

ryannjohnson commented 7 years ago

@guillaumepotier It shouldn't have more features than the ES6 classes update. The git diff does look wonky though, since I pulled keyDownHandlers into the constructor to keep it as an object property. Maybe there's a better way?

guillaumepotier commented 7 years ago

No you're right. The wonky git diff made me think at first glance you added keyDownHandlers new features but you're right. Should be good, pulled your branch in my project and it works like a charm, without nasty React console error warning 👍

CMTegner commented 7 years ago

Hi! I really appreciate the effort here! I finally had time to sit down and do the required testing and research on this subject. I've published 1.5.5 which no longer uses React.createClass or React.PropTypes. I feel like I owe you an explanation since it seems like I just ignored your work and did the same thing as you. Here's my after-the-fact feedback:

I hope this can be taken as constructive feedback and not elitism or anything like that. Again, I really appreciate the work you've put in. I figured the path of least resistance was to just push my own changes, since it seemed like it was more important to get the changes out than to spend more time with PRs.

ryannjohnson commented 7 years ago

@CMTegner Lol, this isn't elitism at all and is a great resource to learn from. I'd actually like to thank you for taking the time to point out how this PR (and programming) could have been done better. I'll be referring to this next time I decide to contribute to an open repo.

CMTegner commented 7 years ago

Great, glad I could be of help! :)