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.

Sign up for GitHub

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

bugfix/1912-Support-Page-Forms-Validation #1961

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

bugfix/1912-Support-Page-Forms-Validation #1961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ddf813
Implemented dynamic form validation for each step of the support form
velnachev Oct 17, 2024
4b0fb2e
Merge branch 'master' into bugfix/1912-Support-Page-Forms-Validation
velnachev Oct 17, 2024
7afb98a
code clean up
velnachev Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 23 additions & 18 deletions src/components/client/support-form/SupportForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ import { useMutation } from '@tanstack/react-query'
import { useTranslation } from 'next-i18next'
import React, { useState, useRef } from 'react'
import { FormikHelpers, FormikProps } from 'formik'
import { Stepper, Step, StepLabel, StepConnector, Hidden, Grid, Box } from '@mui/material'

import { Stepper, Step, StepLabel, StepConnector, Grid, Box } from '@mui/material'
import { AlertStore } from 'stores/AlertStore'
import { createSupportRequest } from 'service/support'
import GenericForm from 'components/common/form/GenericForm'
import { ApiErrors, isAxiosError, matchValidator } from 'service/apiErrors'

import Actions from './Actions'
import Roles from './steps/Roles'
import StepIcon from './StepperIcon'
Expand All @@ -24,6 +22,7 @@ import {
SupportRequestResponse,
SupportRequestInput,
} from './helpers/support-form.types'
import ValidationObserver from './ValidationObserver'

const initialValues: SupportFormData = {
person: {
Expand Down Expand Up @@ -108,6 +107,7 @@ export default function SupportForm() {
const formRef = useRef<FormikProps<SupportFormData>>(null)
const [activeStep, setActiveStep] = useState<Steps>(Steps.ROLES)
const [failedStep, setFailedStep] = useState<Steps>(Steps.NONE)
const [currentValidatedField, setCurrentValidatedField] = useState<number | null>(null)

const mutation = useMutation<
AxiosResponse<SupportRequestResponse>,
Expand Down Expand Up @@ -222,21 +222,22 @@ export default function SupportForm() {
onSubmit={handleSubmit}
initialValues={initialValues}
validationSchema={validationSchema[activeStep]}
innerRef={formRef}>
<Hidden mdDown>
<Stepper
alternativeLabel
activeStep={activeStep}
connector={<StepConnector sx={{ mt: 1.5 }} />}>
{steps.map((step, index) => (
<Step key={index}>
<StepLabel error={isStepFailed(index)} StepIconComponent={StepIcon}>
{t(step.label)}
</StepLabel>
</Step>
))}
</Stepper>
</Hidden>
innerRef={formRef}
validateOnBlur={false}
validateOnChange={currentValidatedField === activeStep}>
Copy link
Contributor

Choose a reason for hiding this comment

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

currentValidateField sounds like it pertains to a field, but in here it is used to toggle on/off of individual field validation on a step.

How about validateInidividualFieldsOnStep ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to make the pattern more explicit and readable by making the value that comes out of the ValidationObserver a map such that individual field validation is turned on/off explicitly

<GeneralForm<...>
      ...
      validateOnBlur={validateIndividualFields[activeStep]}
      validateOnChange={validateIndividualFields[activeStep]}>

<Stepper
alternativeLabel
activeStep={activeStep}
connector={<StepConnector sx={{ mt: 1.5 }} />}
sx={{ display: { xs: 'none', md: 'flex' } }}>
{steps.map((step, index) => (
<Step key={index}>
<StepLabel error={isStepFailed(index)} StepIconComponent={StepIcon}>
{t(step.label)}
</StepLabel>
</Step>
))}
</Stepper>
{isThankYouStep(activeStep, steps) ? (
steps[activeStep].component
) : (
Expand All @@ -258,6 +259,10 @@ export default function SupportForm() {
</Grid>
</Box>
)}
<ValidationObserver
setCurrentValidatedField={setCurrentValidatedField}
activeStep={activeStep}
/>
</GenericForm>
)
}
39 changes: 39 additions & 0 deletions src/components/client/support-form/ValidationObserver.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { useFormikContext } from 'formik'
import { useEffect, useRef } from 'react'
import { Steps } from './helpers/support-form.types'

const STEPS_VALIDATION_MAP = {
[Steps.ROLES]: ['roles'],
[Steps.QUESTIONS]: ['benefactor', 'partner', 'volunteer', 'associationMember', 'company'],
[Steps.PERSON]: ['person'],
}

const ValidationObserver = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add some comment stating the need to turn off individual field validation first time the step is submitted but then turning it back on so the user gets immediate feedback when filling the correct/expected value

Copy link
Contributor

Choose a reason for hiding this comment

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

And finally - this new pattern might benefit greatly from a few unit tests. Benefit in terms of readability.

setCurrentValidatedField,
activeStep,
}: {
setCurrentValidatedField: React.Dispatch<React.SetStateAction<number | null>>
activeStep: number
}) => {
const activeStepRef = useRef(0)
const { errors, submitCount } = useFormikContext()

useEffect(() => {
if (activeStepRef.current !== activeStep) {
activeStepRef.current = activeStep
setCurrentValidatedField(null)
}

const value = Object.entries(STEPS_VALIDATION_MAP).find(([, value]) => {
return value.includes(Object.keys(errors)?.[0])
})

if (submitCount > 0 && value) {
setCurrentValidatedField(Number(value?.[0]))
}
}, [errors, submitCount, activeStep])

return null
}

export default ValidationObserver
Loading