Skip to content
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

refactor(form-controls): use functional components and react hooks #641

Merged
merged 10 commits into from
Nov 25, 2020

Conversation

mshaaban0
Copy link
Collaborator

@mshaaban0 mshaaban0 commented Nov 13, 2020

Purpose of PR

Refactor form controls to use functional components and react hooks, why:

Note: This shouldn't introduce any breaking changes, so if you see anything please flag it.

PR Checklist

  • I have read the relevant readme.md file(s)
  • All commits follow our Git commit message convention
  • Tests are added/updated/not required
  • Tests are passing
  • Storybook stories are added/updated/not required
  • Usage notes are added/updated/not required
  • Has been tested based on Contentful's browser support
  • Doesn't contain any sensitive information

@netlify
Copy link

netlify bot commented Nov 14, 2020

Deploy preview for forma-36-react-components ready!

Built with commit a41cdca

https://deploy-preview-641--forma-36-react-components.netlify.app

Copy link
Contributor

@gui-santos gui-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Thanks a lot @mshaaban0

Copy link
Collaborator

@denkristoffer denkristoffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the extensive work ❤️ I've added a few questions and thoughts. The biggest thing is that I think we should make sure to wrap event handlers in useCallback for good measure.

@mshaaban0 mshaaban0 force-pushed the refactor/form-controls-PUL-1251 branch from 783e2c0 to c264b46 Compare November 20, 2020 15:35
@mshaaban0
Copy link
Collaborator Author

@denkristoffer Thanks for all the useful comments, addressed 👍

@denkristoffer
Copy link
Collaborator

Great!

@mshaaban0 mshaaban0 force-pushed the refactor/form-controls-PUL-1251 branch 2 times, most recently from 22b81a1 to 60c2800 Compare November 24, 2020 16:46
@mshaaban0 mshaaban0 force-pushed the refactor/form-controls-PUL-1251 branch from 60c2800 to 8d49f55 Compare November 25, 2020 10:25
Copy link
Contributor

@burakukula burakukula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! 🎉

@mshaaban0 mshaaban0 merged commit 5c0f8fc into master Nov 25, 2020
@mshaaban0 mshaaban0 deleted the refactor/form-controls-PUL-1251 branch November 25, 2020 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants