Issue 442/basic information form

andycwilliams commented 10 months ago

This PR:

Resolves #442

Just a draft for now to make sure I'm on the right track.

1. Adds (basic) styling to BasicInfo.jsx 2. Adds logic from #515

Screenshots (if applicable):

Future Steps/PRs Needed to Finish This Work (optional):

1. Ensure this component is properly implementing the useCivicProfile hook 2. Ensure the labeling (such as all the ids and names) is consistent and adheres to any standards (if necessary)

Issues needing discussion/feedback (optional):

1. What styling is needed for each form as well as overall Civic Profile section

xscottxbrownx commented 10 months ago

I haven't looked at this deeply yet...

But, personally don't like the date format - not a big deal though. Still logical.

When does submit button become active? Seems like it's disabled because isSuccess is false, but can't switch isSuccess to true without the button it seems.

And to hit on what Tim was saying more about the test...

I was able to get it passing by matching the label of each <TextField>: const firstNameField = getByRole('textbox', { name: 'Legal first name' }); worked.

xscottxbrownx commented 10 months ago

I'm not really sure why but two things:

1) the margin between the first input and second is like double the others on mobile 2) the clear function doesn't seem to clear the date field

leekahung commented 10 months ago

Think the direction of this PR is pretty good. With regards to date formatting, think we could follow what HMIS utilize.

andycwilliams commented 10 months ago

But, personally don't like the date format - not a big deal though. Still logical.

Do you mean YYYY-MM-DD? Because that's what HMIS states to use.

the margin between the first input and second is like double the others on mobile

Fixed.

the clear function doesn't seem to clear the date field

Yeah I haven't messed with that too much because I'm pretty sure all input must be rendered as string (again, per HMIS). But dates are entered as objects, so I assume that that will just have to be stringified? I've procrastinated this part a bit and need a bit of advice on the next step.

I was able to get it passing by matching the label of each <TextField>: const firstNameField = getByRole('textbox', { name: 'Legal first name' }); worked.

Still haven't finished the test. I think I need to resolve the above issue first.

timbot1789 commented 6 months ago

@andycwilliams any updates on this one?

andycwilliams commented 6 months ago

@andycwilliams any updates on this one?

This is next on my to-do list

andycwilliams commented 6 months ago

The Clear button should be good now. Still troubleshooting BasicInfo.test.jsx not full passing. Something to do with line 38: const dateOfBirthField = getByRole('textbox', { name: 'Date of birth' });

The old screenshots seem to be broken so here are some new ones.

andycwilliams commented 5 months ago

Opening up this PR now. That final test still isn't passing, so my hope right now is to comment that one out and create a separate issue to address it.

Also please ignore my above mis-click.

leekahung commented 5 months ago

Opening up this PR now. That final test still isn't passing, so my hope right now is to comment that one out and create a separate issue to address it.

Think we could make an exception for this one. We could comment on the unit test for the basic information form for now, but like what you've mentioned we'll have to open an issue/bug report to fix it.

Can approve this after the merge conflict resolution.

codeforpdx / PASS