-
Notifications
You must be signed in to change notification settings - Fork 4
feat: ui flexibility prototype version 1 #766
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
base: main
Are you sure you want to change the base?
Conversation
b63a4bf to
83dcdaf
Compare
| import { contractorEvents } from '@/shared/constants' | ||
| import { renderWithProviders } from '@/test-utils/renderWithProviders' | ||
|
|
||
| describe('Contractor/Address', () => { |
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 file just has the original unit test copied over to test for regressions
| const composedDefaultValues = { | ||
| street1: formDefaultValues.street1 || defaultValues?.street1, | ||
| street2: formDefaultValues.street2 || defaultValues?.street2, | ||
| city: formDefaultValues.city || defaultValues?.city, | ||
| state: formDefaultValues.state || defaultValues?.state, | ||
| zip: formDefaultValues.zip || defaultValues?.zip, | ||
| } |
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.
Not sure if it's ultimately worth it to have the hook supply the default values and merge them here via props. Only thought was, sometimes composing the default values gets complicated
| error, | ||
| fieldErrors, |
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 provide both the error and fieldErrors via context. They only get our UI alerts if they use the BaseUIComponent below.
Still need to figure out how to navigate this with fieldErrors that need to be applied to the form fields. Do we just leave it to the partner to wire those up? Or do we find a way to normalize all form related errors so they aren't getting them from both the validation schema and the fieldErrors
Additionally, we need to investigate more how to help this be more ergonomic so they're not forced to respect both error and fieldErrors
jeffredodd
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.
Just a note or two
| return ( | ||
| <FadeIn> | ||
| {(error || fieldErrors) && ( | ||
| <Components.Alert label={t('status.errorEncountered')} status="error"> |
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 leaving notes here as I go so MAYBE i'm missing something. But have we thought about how they might re-locate or reposition this inside their app?
This is the first iteration of UI prototyping exploration.
Approach
onSubmithandler through the hook