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.

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

Align property names in AddParticipantForm #515

Conversation

alex-yau-ttd
Copy link
Contributor

@alex-yau-ttd alex-yau-ttd commented Sep 2, 2024

Match property names in AddParticipantForm to the input names in AddParticipantDialog. This mismatch prevented defaults to be set on the contact inputs, which was slowing down developer iteration.

Manual testing

Still working like normal

chrome_TbqOM9q6bJ.mp4

With a default value set

const formMethods = useForm<AddParticipantForm>({
  defaultValues: { siteIdType: 0, firstName: 'hello reviewer' },
});
chrome_1DIV00rS6U.mp4

Match property names in `AddParticipantForm` to the input names in `AddParticipantDialog`
@@ -185,6 +182,9 @@ export type AddParticipantForm = {
siteName?: string;
jobFunction: string;
crmAgreementNumber: string;
firstName: string;
lastName: string;
email: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe keep them at the top to preserve the order as before since it doesn't look like its in alphabetical order

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved them to the bottom to align with UpdateParticipantForm (and the UI itself). I'm not too concerned about preserving order, so I think I'll merge as-is. thanks!

@alex-yau-ttd alex-yau-ttd merged commit d6f8d16 into main Sep 3, 2024
3 checks passed
@alex-yau-ttd alex-yau-ttd deleted the ajy-UID2-3878-Align-property-names-in-Add-Participant-Form branch September 3, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants