-
Notifications
You must be signed in to change notification settings - Fork 41
Refactor: Migrate to function components #380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…to their own states
f6a75ed to
38fbc5b
Compare
38fbc5b to
a7534ee
Compare
|
@KoolADE85 |
88bef0c to
224ea62
Compare
Hey @AnnMarieW that's a great call! I started this and realized it will be another fairly big diff to look at. So I'm thinking I'll do that in a separate PR. |
BSd3v
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refactor looks like the way I had envisioned it.
I dont know if its necessary to do the check on the gridApi switch with all the other props in the useEffect as it only needs to run when the gridApi gets loaded.
… already - adjusting tests in the event the grid takes a bit to render - fixed issue with selectedRows triggering uneccessarily
BSd3v
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. All tests are passing.
This PR is a pure refactor from class to function components.
There is no functionality change. All automated tests continue to pass, and all example apps in
docs/have been manually confirmed as working.