-
Notifications
You must be signed in to change notification settings - Fork 178
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
feat(app): deprecate formik, migrate to react-hook-form #14424
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #14424 +/- ##
==========================================
+ Coverage 67.77% 67.86% +0.09%
==========================================
Files 2518 2519 +1
Lines 71968 73051 +1083
Branches 9244 9722 +478
==========================================
+ Hits 48774 49577 +803
- Misses 20990 21201 +211
- Partials 2204 2273 +69
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
some initial comments testing the forms
...src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/index.tsx
Show resolved
Hide resolved
d66505c
to
19d8e5b
Compare
...src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx
Outdated
Show resolved
Hide resolved
...src/organisms/Devices/RobotSettings/AdvancedTab/AdvancedTabSlideouts/RenameRobotSlideout.tsx
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/FormModal.tsx
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/form-fields.ts
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/form-fields.ts
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/form-fields.ts
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/form-state.ts
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/form-state.ts
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/ConnectModal/index.tsx
Outdated
Show resolved
Hide resolved
app/src/organisms/Devices/RobotSettings/ConnectNetwork/types.ts
Outdated
Show resolved
Hide resolved
tried this branch on a dev kit and rename worked as expected. |
> | ||
<ConnectModalComponent {...props} /> | ||
</Formik> | ||
<form onSubmit={handleSubmit(onSubmit)}> |
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 form isn't submitting on click - tracked down the issue, the id
(below in line 90) needs to be on this form element to associate the submit button in FormModal
with this form.
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.
thank you for thoroughly testing and catching this Brent!
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.
everything works, looks good!
closes RAUT-960
Overview
Deprecate formik and update to react-hook-form. This migration has already been done in PD.
Test Plan
The following areas had components that were migrated, please check them to make sure they still work as expected:
Changelog
Review requests
see test plan
Risk assessment
low-ish, only a few components in the app were using formik