Add employer page #56
Conversation
There was a problem hiding this comment.
Looks like this is the "old" (storybook changes their syntax every year it seems 🙄) storybook format, let's update to the new format/syntax
There was a problem hiding this comment.
@brandonlenz Which format/syntax are you referring to? I originally changed it to match ssn and the other existing stories but since it expects a form to already exists it throws an error. The examples I see on the storybook site don't use a form provider, so I updated it from the old version with this versions equivalents, StoryFn instead of story etc; which I also got from online examples where people were using storybook with a react-hook-form subcomponent. The storyFn is needed because to simulate it as part of a form and those hook calls to make a temporary form need to happen in a function. Open to hear what else your suggesting or if you have a better way to set it as a form as I tried to not spend too long on it so could have easily overlooked something.
|
|
||
| export const yupAddress = () => | ||
| object().shape({ | ||
| address: string() |
There was a problem hiding this comment.
With a single address format, let's give standard 2 address lines.
You can also remove all the min/max length requirements on these
There was a problem hiding this comment.
Hmm, I was always taught any text input should have a max, even if its large. I'm down to get rid of any mins, but should we at least have a max of 255 if we want it to be an extreme max?
There was a problem hiding this comment.
Sure let's go with 255 for now!
| .city { | ||
| width: 10rem !important; | ||
| margin-right: 2rem !important; | ||
| } | ||
|
|
||
| .city_error { | ||
| padding-right: 2rem; | ||
| } | ||
|
|
||
| .state { | ||
| width: 8rem !important; | ||
| } | ||
|
|
||
| .zipcode { | ||
| width: 8rem !important; | ||
| } |
There was a problem hiding this comment.
Are the !importants really needed? Its usually code smell, so definitely want to avoid that especially this early on a project where we have the luxury and time of doing things right, even if it means banging heads against scss (again 😢)
There was a problem hiding this comment.
Ahh I recall we added this back on DOL for a reason, but to be honest I don't remember what it is I just kept it the same. I would assume that we have less stuff here so probably the same conflict won't occur.
| const validationSchema = yup.object().shape({ | ||
| address: yup.object().shape({ | ||
| address1: yup.string().required(tCommon('validation.required')), | ||
| address2: yup.string().optional(), | ||
| city: yup.string().required(tCommon('validation.required')), | ||
| state: yup.string().required(tCommon('validation.required')), | ||
|
|
||
| zipcode: yup | ||
| .string() | ||
| // eslint-disable-next-line security/detect-unsafe-regex | ||
| .matches(/^\d{5}(-\d{4})?$/, tCommon('validation.notZipCode')) | ||
| .required(tCommon('validation.required')), | ||
| }), | ||
| }) |
There was a problem hiding this comment.
Does this need to be defined again here when you have src/validations/address.ts?
There was a problem hiding this comment.
They aren't the same schemas. I kept how we had it on DOL because I liked that the address only storybook had its own generic schema just for display and then when we used the object we made it more customized, min/max length. Any objection to keeping it that way?
| export const Default = Template.bind({}) | ||
| Default.args = { | ||
| name: 'sample_phone', | ||
| label: 'Enter your phone number:', | ||
| } |
There was a problem hiding this comment.
So my note about the older syntax for stories can be seen in this example. The documentation now provides updated syntax (SB 7), while these examples are from storybook 6 and before. SB changes teh syntax every year it seems like, but lets go ahead and keep it all uniform with the latest style: https://storybook.js.org/docs/react/writing-stories/introduction
StoryFn isn't used anymore, and the Template.bind is also not used.
e.g this part would be something like
| export const Default = Template.bind({}) | |
| Default.args = { | |
| name: 'sample_phone', | |
| label: 'Enter your phone number:', | |
| } | |
| export const PhoneNumberField: Story = { | |
| render: () => // Your StoryFn contents here, or a call to it | |
| } |
Yes, the name PhoneNumberField is already taken, so import PhoneNumberField as PhoneNumberFieldComponent in this file (see ssn.stories.tsx). This naming convention matching the storybook story name displays the example without nesting it, which we should do if our only story is the default story
| import { UntouchedRadioValue } from 'constants/formOptions' | ||
|
|
||
| export type YesNoInput = boolean | UntouchedRadioValue | ||
| // export type CheckboxInput = boolean | UntouchedCheckboxValue |
| @@ -0,0 +1,63 @@ | |||
| const common = { | |||
There was a problem hiding this comment.
Just for consistency
| const common = { | |
| export default { |
| }, | ||
| } | ||
|
|
||
| export default common |
There was a problem hiding this comment.
| export default common |
| // Work Location | ||
|
|
There was a problem hiding this comment.
nit
| // Work Location | |
| // Work Location |
##Resolves #13
Changes proposed in this pull request:
Add employer page with the following:
Full-time vs part time
Address
Phone number
Alternate phone number
Self employment status
Ownership status
Separation reason/change in employment
Start and end dates