-
Notifications
You must be signed in to change notification settings - Fork 59
Form Validation #239
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
Form Validation #239
Conversation
…f notifs disabled.
| @@ -1,8 +1,8 @@ | |||
| import clone from 'lodash/cloneDeep' | |||
| import { Field, FieldArray } from 'formik' | |||
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.
I'm just realizing that the amount of files in the lib/components/user directory is getting quite large. Can some subfolders be created to help organize all these files?
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.
Good point. Perhaps this? In this PR or a separate PR?
user/
- (name needed)
- form-navigation-buttons
- link-button`
- link-menu-item
- stacked-pane-display
- sequential-pane-display
- login
- after-signin-screen
- awaiting-screen
- before-signin-screen
- nav-login-button
- nav-login-button-auth0
- nav-login-button.css
- with-loggedin-user-support
- account
- account-setup-finish-pane.js
- existing-account-display.js
- favorite-locations-pane.js
- new-account-wizard.js
- notification-prefs-pane.js
- phone-verification-pane.js
- terms-of-use-pane.js
- user-account-screen.js
- verify-email-screen.js
- trip
- saved-trip-editor.js
- saved-trip-list.js
- saved-trip-screen.js
- trip-basics-pane.js
- trip-notifications-pane.js
- trip-summary-pane.js
- trip-summary.js
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.
Might as well do a separate PR. For the (name needed) I'd just keep those in the root /user folder.
| validateOnBlur | ||
| validationSchema={validationSchema} | ||
| onSubmit={isCreating ? this._handleSaveNewTrip : this._handleSaveTripEdits} | ||
| onSubmit={this._handleTrip} |
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.
I think this is supposed to be _handleSaveTrip (!).
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.
Wow good catch!
| * Save changes and return to the planner. | ||
| * @param {*} userData The user edited state to be saved, provided by Formik. | ||
| */ | ||
| _handleExitAndSave = async userData => { |
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.
Minor tweak: could you change this name to _handleSaveAndExit (just to match its behavior)?
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.
Updated in 6902c2b
landonreed
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. Conditional approval on a couple of handler name changes.
|
🎉 This PR is included in version 1.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR converts the forms for user accounts and saved trips into Formik, and adds validation using yup.
Previous form state management code is removed, as all state management is handled by Formik.
The main design change resulting from the Formik conversion is the component change for selecting the monitored trip days.

The following fields are validated: