Add FormProviders to forms used so they can be more easily used (GCOM-1234) #2109
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
paales
changed the title
Comment on lines
128
to
136
| <Form onSubmit={submit} noValidate sx={sx}> | ||
| <NameFields form={form} key='name' readOnly={readOnly} /> | ||
| <AddressFields form={form} key='addressfields' readOnly={readOnly} /> | ||
| <FormRow key='telephone'> | ||
| <TextFieldElement | ||
| control={form.control} | ||
| name='telephone' | ||
| variant='outlined' | ||
| type='text' | ||
| required={required.telephone} | ||
| validation={{ | ||
| pattern: { value: phonePattern, message: i18n._(/* i18n */ 'Invalid phone number') }, | ||
| }} | ||
| label={<Trans id='Telephone' />} | ||
| InputProps={{ | ||
| readOnly, | ||
| endAdornment: <InputCheckmark show={valid.telephone} />, | ||
| }} | ||
| /> | ||
| </FormRow> | ||
| <ApolloCartErrorAlert error={error} /> | ||
| </Form> | ||
| <FormProvider {...form}> | ||
| {children ?? ( | ||
| <> | ||
| <NameFields /> | ||
| <AddressFields /> | ||
| <TelephoneField /> | ||
| <ApolloCartErrorAlert /> | ||
| <FormAutoSubmit name={['postcode', 'countryCode', 'regionId']} /> | ||
| </> | ||
| )} | ||
| </FormProvider> |
There was a problem hiding this comment.
Please re-add form element (with sx) to prevent breaking change.
| import { | ||
| ChangePasswordDocument, | ||
| ChangePasswordMutation, | ||
| ChangePasswordMutationVariables, | ||
| } from './ChangePassword.gql' | ||
|
|
||
| export function ChangePasswordForm() { | ||
| export function ChangePasswordForm({ children }: PropsWithChildren) { |
There was a problem hiding this comment.
Suggested change
| export function ChangePasswordForm({ children }: PropsWithChildren) { | |
| export type ChangePasswordFormProps = PropsWithChildren | |
| export function ChangePasswordForm(props: ChangePasswordFormProps) { | |
| const { children } = props |
| import { Trans } from '@lingui/react' | ||
| import { ValidatedPasswordElement } from '../ValidatedPasswordElement/ValidatedPasswordElement' | ||
|
|
||
| export function ValidatePasswordFields() { |
There was a problem hiding this comment.
Maybe rename to NewPasswordFields?
There was a problem hiding this comment.
Any particular reason NewPasswordFields is a better name then ValidatePasswordFields?
| import { CreateCustomerAddressDocument } from './CreateCustomerAddress.gql' | ||
|
|
||
| export function CreateCustomerAddressForm() { | ||
| export function CreateCustomerAddressForm({ children }: PropsWithChildren) { |
There was a problem hiding this comment.
Suggested change
| export function CreateCustomerAddressForm({ children }: PropsWithChildren) { | |
| export type CreateCustomerAddressFormProps = PropsWithChildren | |
| export function CreateCustomerAddressForm(props: ChangePasswordFormProps) { | |
| const { children } = props |
Comment on lines
63
to
62
| <NameFields form={form} prefix /> | ||
| <AddressFields form={form} /> | ||
| {children ?? ( | ||
| <> | ||
| <NameFields /> | ||
| <AddressFields /> | ||
| <TelephoneField /> |
There was a problem hiding this comment.
I've added some new logic to NameFields. prefixes had now a false options for prefixes, so the option can be enabled via one prop instead of a prefix and prefixes prop
Comment on lines
19
to
20
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const [_, authenticationError] = graphqlErrorByCategory({ |
There was a problem hiding this comment.
Suggested change
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |
| const [_, authenticationError] = graphqlErrorByCategory({ | |
| const [, authenticationError] = graphqlErrorByCategory({ |
Comment on lines
15
to
16
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const [_, inputError] = graphqlErrorByCategory({ |
There was a problem hiding this comment.
Suggested change
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |
| const [_, inputError] = graphqlErrorByCategory({ | |
| const [, inputError] = graphqlErrorByCategory({ |
Comment on lines
22
to
36
| if (formState.isSubmitSuccessful && data) { | ||
| return ( | ||
| <Alert | ||
| severity='success' | ||
| variant='standard' | ||
| sx={(theme) => ({ | ||
| marginTop: theme.spacings.md, | ||
| marginBottom: theme.spacings.sm, | ||
| })} | ||
| > | ||
| <Trans id='We’ve send a password reset link to your email address!' /> | ||
| </Alert> | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
Should this message not be added back somewhere?
Comment on lines
29
to
27
| const { prefix, form, readOnly, prefixes = [mr, mrs, other] } = props | ||
| const form = useFormContext() | ||
|
|
||
| const prefixes = [mr, mrs, other] | ||
|
|
There was a problem hiding this comment.
Why would you no longer want to be able to pass prefixes in props?
Comment on lines
24
to
19
| export function NameFields(props: NameFieldProps) { | ||
| export function NameFields({ prefix = false }: NameFieldsProps) { |
There was a problem hiding this comment.
Destructure on separate line?
Comment on lines
65
to
48
| {...muiRegister('currentEmail', { | ||
| required: true, | ||
| pattern: { value: emailPattern, message: '' }, | ||
| })} | ||
| InputProps={{ | ||
| readOnly: true, | ||
| }} | ||
| /> | ||
| </FormRow> | ||
|
|
||
| <FormRow> | ||
| <TextField | ||
| key='email' | ||
| variant='outlined' | ||
| type='text' | ||
| autoComplete='off' | ||
| error={formState.isSubmitted && !!formState.errors.email} | ||
| helperText={formState.isSubmitted && formState.errors.email?.message} | ||
| label={<Trans id='New email' />} | ||
| required={required.email} | ||
| {...muiRegister('email', { | ||
| required: true, | ||
| pattern: { value: emailPattern, message: '' }, | ||
| })} | ||
| /> | ||
| <TextField | ||
| key='confirm-email' | ||
| variant='outlined' | ||
| type='text' | ||
| autoComplete='off' | ||
| error={formState.isSubmitted && !!formState.errors.confirmEmail} | ||
| helperText={formState.isSubmitted && formState.errors.confirmEmail?.message} | ||
| label={<Trans id='Confirm new email' />} | ||
| required | ||
| {...muiRegister('confirmEmail', { | ||
| required: true, | ||
| validate: (value) => value === watchNewEmail || i18n._(/* i18n */ "Emails don't match"), | ||
| })} | ||
| /> | ||
| </FormRow> | ||
| <FormProvider {...form}> | ||
| <Form onSubmit={submit} noValidate> | ||
| {children ?? ( | ||
| <> | ||
| <FormRow> | ||
| <TextFieldElement | ||
| key='current-email' | ||
| variant='outlined' | ||
| type='text' | ||
| autoComplete='email' | ||
| autoFocus | ||
| error={formState.isSubmitted && !!formState.errors.currentEmail} | ||
| helperText={formState.isSubmitted && formState.errors.currentEmail?.message} | ||
| label={<Trans id='Current email' />} | ||
| required | ||
| value={email} | ||
| InputProps={{ | ||
| readOnly: true, | ||
| }} | ||
| control={control} | ||
| name='currentEmail' | ||
| validation={{ | ||
| pattern: { value: emailPattern, message: '' }, | ||
| }} | ||
| /> | ||
| </FormRow> | ||
|
|
||
| <FormRow> | ||
| <PasswordElement | ||
| control={control} | ||
| variant='outlined' | ||
| name='password' | ||
| label={<Trans id='Password' />} | ||
| autoComplete='current-password' | ||
| required={required.password} | ||
| disabled={formState.isSubmitting} | ||
| error={Boolean(authenticationError)} | ||
| helperText={authenticationError?.message} | ||
| /> | ||
| </FormRow> | ||
| <PasswordField /> |
There was a problem hiding this comment.
email & confirmEmail field are missing.
| control: Control<TFieldValues> | ||
| // control: Control<TFieldValues> |
There was a problem hiding this comment.
Can this be removed?
| submit: ReturnType<UseFormReturn<TFieldValues>['handleSubmit']> | ||
| // submit: ReturnType<UseFormReturn<TFieldValues>['handleSubmit']> |
There was a problem hiding this comment.
Can this be removed?
Comment on lines
26
to
19
| export type EditBillingAddressFormProps = { sx?: SxProps<Theme> } | ||
| export type EditBillingAddressFormProps = PropsWithChildren<{ sx?: SxProps<Theme> }> |
There was a problem hiding this comment.
We don't use this anywhere. Use more conventional children?: React.ReactNode?
mikekeehnen
force-pushed
the
feature/GCOM-1234
branch
from
3b91d76 to
06e8bea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.