-
Notifications
You must be signed in to change notification settings - Fork 0
feat(onboarding) - add new step to select country #187
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(onboarding) - add new step to select country #187
Conversation
…at-lets-you-select-a
src/flows/Onboarding/hooks.ts
Outdated
return parseJSFToValidate(values, selectCountryForm?.fields, { | ||
isPartialValidation: true, | ||
}); |
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.
Do we need to parse the country values?
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 did for what I thought it was consistency, I've removed it
src/flows/Onboarding/hooks.ts
Outdated
const [internalCountryCode, setInternalCountryCode] = useState<string | null>( | ||
null, | ||
); | ||
const { data: countries, isLoading: isLoadingCountries } = useCountries(); |
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.
Can we build a hook that aggregates all the logic to build the country dropdown? It could do what useCountries already does and in the select callback run createHeadlessForm for the local schema
src/flows/Onboarding/hooks.ts
Outdated
@@ -554,6 +637,13 @@ export const useOnboarding = ({ | |||
* @returns Validation result or null if no schema is available | |||
*/ | |||
handleValidation: (values: FieldValues) => { | |||
if (stepState.currentStep.name === 'select_country') { | |||
const parsedValues = parseJSFToValidate( |
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.
Same question, do we need to parse the country value?
case 'basic_information': { | ||
if (!internalEmploymentId) { | ||
const notLoadedEmployment = |
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.
We should be consistent in the naming, so booleans should start with is
, eg isEmploymentLoaded
Co-authored-by: João Almeida <[email protected]>
I added a new step to select country and remove the countryId from the props.
We could discuss later if we could have countryId as optional to preselect the country fields
You can select a country and the onboarding starts as before
If you decide to load an onboarding and you change countries, in the basic information step we don't update the employment, we just create a new one.