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

Remove formik Field components from UI layer #1064

Merged
merged 2 commits into from
Jul 21, 2022
Merged

Remove formik Field components from UI layer #1064

merged 2 commits into from
Jul 21, 2022

Conversation

zephraph
Copy link
Contributor

When we first started integrating Formik into the console we really didn't have much of a separation between the UI of a form element and its actual logical behavior. Given that, we added the Field component in our form UI components. This has a few downsides:

  1. It necessitates that the UI layer includes formik as a dependency if we ever ship it separately
  2. It requires that any UI form component only be rendered inside the context of a formik form

This tight coupling has already caused a few minor issues. I'm working on richer table selection behavior to address #1060 and I ran into it but it's not the first time I've ran across it.

Also, we've got #804 where we're actively considering migrating away from Formik. This would be the first required step either way.

@zephraph zephraph requested a review from david-crespo July 21, 2022 17:20
@vercel
Copy link

vercel bot commented Jul 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
console-ui-storybook ✅ Ready (Inspect) Visit Preview Jul 21, 2022 at 5:45PM (UTC)

Comment on lines +18 to +19
/** HTML type attribute, defaults to text */
type?: string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this configurable so we could pass down number, etc

Copy link
Collaborator

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

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

Awesome! I did a search for formik in libs/ui and found the FormikDecorator, which we were using to make Storybook stories not break due to missing Formik context. I think that can go too, taking this even further negative LoC. Love it!

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.

2 participants