Issue 53 add form react hook form component #63

EdenWuyt · 2025-12-05T07:04:35Z

Change Summary

Add form component with react-hook-form and yup.

Change Form

Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.

The pull request title has an issue number
The change works by "Smoke testing" or quick testing
The change has tests
The change has documentation

Note: please check out http://localhost:3000/test/form for simple tests. However I do think the test is incomplete for it does not test complex input types and design.

Other Information

Please carefully read the documentation inside /components/form.tsx.
I think my current implementation is a bit complicated. Since the input field components requires value and onChange props, register does not seem to work (or it works but I jsut dont figure it out).
Not that confident about the implementation. Please assign a robot for me first.

Demo:

form_demo.mp4

Related issue

Resolve Add Form (React Hook Form) component #53

EdenWuyt · 2025-12-05T11:17:44Z

Demo with more test

form_demo_more.mp4

ErikaKK · 2025-12-07T13:21:49Z

client/package.json

    "tailwind-merge": "^3.3.1",
-    "tailwindcss-animate": "^1.0.7"
+    "tailwindcss-animate": "^1.0.7",
+    "yup": "^1.7.1"


sorry we'll be using zod for the schema validation instead of yup because it's more suitable for typescript

Copilot

Pull request overview

This PR introduces a reusable Form component using react-hook-form and zod for validation, addressing issue #53. The implementation provides a declarative API with validation support and flexible error handling options.

Adds Form and ControlledField components with zod schema validation
Includes comprehensive documentation and three example implementations
Adds required dependencies: @hookform/resolvers, react-hook-form, and zod

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.

File	Description
client/src/components/ui/form.tsx	New Form component with react-hook-form integration, ControlledField wrapper, and error handling utilities
client/src/app/test/form/page.tsx	Test page demonstrating three error handling approaches with example form schemas and input fields
client/package.json	Added dependencies for form validation: @hookform/resolvers, react-hook-form, and zod
client/package-lock.json	Lock file updates for new form-related dependencies

Files not reviewed (1)

client/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T16:01:06Z

client/src/components/ui/form.tsx

@@ -0,0 +1,150 @@
+// Usage of Form:
+// import { Form, ControlledField } from "@/components/ui/form"; // Note: both are necessary and not default export!
+// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx


Typo in comment: "implimentation" should be "implementation".

Suggested change

// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx

// <Form schema={zodSchema} // Note: according to the current implementation, numbers are also string. For an example please refer to /app/test/form/page.tsx

Copilot · 2025-12-09T16:01:07Z

client/src/components/ui/form.tsx

+// import { Form, ControlledField } from "@/components/ui/form"; // Note: both are necessary and not default export!
+// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx
+//       onSubmit={submitHandler} // callback function in the form of (data) => handleSumit(data)
+//       orError={errorHandler} // optional, allows flexibility of error message display,


Typo in comment: "orError" should be "onError" to match the actual prop name.

Suggested change

// orError={errorHandler} // optional, allows flexibility of error message display,

// onError={errorHandler} // optional, allows flexibility of error message display,

Copilot · 2025-12-09T16:01:07Z

client/src/components/ui/form.tsx

+};
+
+// To make input validation effective, an extra wrapper is needed for the input field
+type controlledFieldProps<TSchema> = {


Type name controlledFieldProps does not follow TypeScript naming conventions. Type names should use PascalCase. Consider renaming to ControlledFieldProps.

Copilot · 2025-12-09T16:01:07Z

client/src/components/ui/form.tsx

+    <>
+      <FormProvider {...methods}>
+        <form
+          onSubmit={methods.handleSubmit(onSubmit, (errors) => {
+            if (onError) onError(errors);
+            else alert(getErrorMessage(errors));
+          })}
+          className={cn(
+            "flex w-full flex-col space-y-3 rounded-md border border-[hsl(var(--border))] bg-background px-8 py-8",
+            className,
+          )}
+        >
+          {typeof children === "function" ? children(methods) : children}
+        </form>
+      </FormProvider>
+    </>


The empty fragment wrapper (<></>) around FormProvider serves no purpose and adds unnecessary nesting. Consider removing it to simplify the component structure.

Suggested change

<>

<FormProvider {...methods}>

<form

onSubmit={methods.handleSubmit(onSubmit, (errors) => {

if (onError) onError(errors);

else alert(getErrorMessage(errors));

})}

className={cn(

"flex w-full flex-col space-y-3 rounded-md border border-[hsl(var(--border))] bg-background px-8 py-8",

className,

)}

>

{typeof children === "function" ? children(methods) : children}

</form>

</FormProvider>

</>

<FormProvider {...methods}>

<form

onSubmit={methods.handleSubmit(onSubmit, (errors) => {

if (onError) onError(errors);

else alert(getErrorMessage(errors));

})}

className={cn(

"flex w-full flex-col space-y-3 rounded-md border border-[hsl(var(--border))] bg-background px-8 py-8",

className,

)}

>

{typeof children === "function" ? children(methods) : children}

</form>

</FormProvider>

Copilot · 2025-12-09T16:01:08Z

client/src/app/test/form/page.tsx

+const schema = z.object({
+  firstname: z.string().min(1, "Firstname is required"),
+  lastname: z.string().min(1, "Lastname is required"),
+  middlename: z.string().optional(),
+  age: z
+    .string()
+    .min(1, "Age is required")
+    .regex(/^\d+$/, { message: "Age must be a non-negative integer" })
+    .refine(
+      (val) => {
+        const num = Number(val);
+        return num >= 1 && num <= 120;
+      },
+      {
+        message: "Age must be between 1 and 120",
+      },
+    ),
+});
+
+const extendedSchema = z.object({
+  firstname: z.string().min(1, "Firstname is required"),
+  lastname: z.string().min(1, "Lastname is required"),
+  middlename: z.string().optional(),
+  age: z
+    .string()
+    .min(1, "Age is required")
+    .regex(/^\d+$/, { message: "Age must be a non-negative integer" })
+    .refine(
+      (val) => {
+        const num = Number(val);
+        return num >= 1 && num <= 120;
+      },
+      {
+        message: "Age must be between 1 and 120",
+      },
+    ),
+  frequency: z.enum(["daily", "weekly", "monthly"], {
+    error: () => ({ message: "Invalid frequency" }),
+  }),
+  amenities: z
+    .array(
+      z.enum(
+        ["Audio", "Video", "White Board", "HDMI", "Projector", "Speaker Phone"],
+        {
+          error: () => ({ message: "Invalid amenities" }),
+        },
+      ),
+    )
+    .optional(),
+  recurrenceDate: z.enum(["Monday", "Tuesday", "Wednesday"], {
+    error: () => ({ message: "Invalid frequency" }),
+  }),
+});


The schema and extendedSchema contain duplicated validation logic for firstname, lastname, middlename, and age fields. Consider extracting the common fields into a base schema and extending it using z.object({...}).extend({...}) or composition to reduce code duplication.

Copilot · 2025-12-09T16:01:08Z

client/src/app/test/form/page.tsx

+    )
+    .optional(),
+  recurrenceDate: z.enum(["Monday", "Tuesday", "Wednesday"], {
+    error: () => ({ message: "Invalid frequency" }),


The error message for recurrenceDate says "Invalid frequency" which is inconsistent and misleading. It should say "Invalid recurrence date" or similar to match the field name.

Suggested change

error: () => ({ message: "Invalid frequency" }),

error: () => ({ message: "Invalid recurrence date" }),

Copilot · 2025-12-09T16:01:08Z

client/src/components/ui/form.tsx

@@ -0,0 +1,150 @@
+// Usage of Form:
+// import { Form, ControlledField } from "@/components/ui/form"; // Note: both are necessary and not default export!
+// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx


The comment references "zodSchema" but the parameter is named "schema" in the actual component. This inconsistency could confuse developers using this documentation.

Suggested change

// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx

// <Form schema={schema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx

EdenWuyt · 2025-12-09T18:10:42Z

Copilot suggestion fixed.

EdenWuyt · 2025-12-18T13:09:07Z

No idea why the error comes up

EdenWuyt · 2025-12-23T18:42:56Z

The component is rewritten with shadcn style. Please review again. Thanks. Again, no idea why the type check fails. The error message is like below:

Error: src/app/test/form/page.tsx(16,8): error TS1192: Module '"/home/runner/work/bloom-booking-system/bloom-booking-system/client/src/components/ui/input"' has no default export.
Error: Process completed with exit code 2.

ErikaKK · 2026-01-06T12:59:47Z

No idea why the error comes up

Please pull the latest main and solve the error, the error is because of input component

…Add detailed documentation about the usage of the components. Frontend styling and test to be done.

…e customization. Update tests and documentation.

…ype of value) => void and (type of event) => void.

…id to handle the issue that errors are empty at the beginning. Components remain intact but update the documentation.

… documentation.

…ove redundant empty fragment wrapper.

…ema definition and use schema.extend() to reduce redundancy in test file.

…r message display and api error handling.

EdenWuyt marked this pull request as draft December 5, 2025 07:08

EdenWuyt marked this pull request as ready for review December 5, 2025 07:48

EdenWuyt requested a review from ErikaKK December 5, 2025 11:18

ErikaKK reviewed Dec 7, 2025

View reviewed changes

ErikaKK requested a review from Copilot December 9, 2025 15:57

Copilot started reviewing on behalf of ErikaKK December 9, 2025 15:58 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

EdenWuyt added 16 commits January 6, 2026 14:48

Add Form and ControlledField component with react hook form and yup. …

512f6cd

…Add detailed documentation about the usage of the components. Frontend styling and test to be done.

Modify styling, allow get errors from FormProvider and allow classNam…

031425c

…e customization. Update tests and documentation.

Modify the onChange props type for ControlledField to support both (t…

96b051a

…ype of value) => void and (type of event) => void.

Modify the onChange props type for ControlledField to support both (t…

6fdca17

…ype of value) => void and (type of event) => void.

Add more tests about dropdown input, select input, radio buttons.

3362bd0

Change from using methods.formState.errors to methods.formState.isVal…

dd90f7e

…id to handle the issue that errors are empty at the beginning. Components remain intact but update the documentation.

Change yup to zod.

6dbfd35

Fix Copilot suggestion 1: fix typos and update parameter names in the…

77231fd

… documentation.

Update documentation to a JSDoc style.

ff02c35

Fix Copilot suggestion 2: update props name to fit convention and rem…

e123e42

…ove redundant empty fragment wrapper.

Fix Copilot suggestion 3: correct misleading error message in zod sch…

623855e

…ema definition and use schema.extend() to reduce redundancy in test file.

Move form out of /ui directory. Update component to allow inline erro…

01aa69e

…r message display and api error handling.

Trigger automatic tests.

998fa2f

Rewrite the form component in shadcn style. Update the test page.

571753f

Add documentation.

07213b3

Rebase on the latest origin/main and update input import.

9ad3479

EdenWuyt force-pushed the issue-53-Add_Form_React_Hook_Form_component branch from eabaa1c to 9ad3479 Compare January 6, 2026 14:53

ErikaKK approved these changes Jan 7, 2026

View reviewed changes

EdenWuyt merged commit d84cd61 into main Jan 8, 2026
5 checks passed

	// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx
	// <Form schema={zodSchema} // Note: according to the current implementation, numbers are also string. For an example please refer to /app/test/form/page.tsx

	// orError={errorHandler} // optional, allows flexibility of error message display,
	// onError={errorHandler} // optional, allows flexibility of error message display,

	error: () => ({ message: "Invalid frequency" }),
	error: () => ({ message: "Invalid recurrence date" }),

	// <Form schema={zodSchema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx
	// <Form schema={schema} // Note: according to the current implimentation, numbers are also string. For an example please refer to /app/test/form/page.tsx

Issue 53 add form react hook form component #63

Issue 53 add form react hook form component #63

Uh oh!

Conversation

EdenWuyt commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Summary

Change Form

Other Information

Related issue

Uh oh!

EdenWuyt commented Dec 5, 2025

Uh oh!

ErikaKK Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

EdenWuyt commented Dec 9, 2025

Uh oh!

EdenWuyt commented Dec 18, 2025

Uh oh!

EdenWuyt commented Dec 23, 2025

Uh oh!

ErikaKK commented Jan 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EdenWuyt commented Dec 5, 2025 •

edited

Loading