Fix Git History - #35
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive new registration form system for the site application, along with numerous reusable form components in the shared package. The changes focus on implementing a modern, multi-step registration flow with autosave functionality and proper form validation.
- Adds a complete registration form with 18+ form fields including personal information, education details, and file upload capabilities
- Creates 8 new reusable form components in the shared package for consistent UI/UX across applications
- Implements autosave functionality to prevent data loss during form completion
Reviewed Changes
Copilot reviewed 33 out of 42 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/index.ts | Exports new form components, hooks, and data assets |
| shared/src/hooks/form-autosave.ts | Implements autosave hook with 10-second delay and beforeunload protection |
| shared/src/components/form/*.tsx | Eight new form components for various input types (text, select, radio, checkbox, file upload, etc.) |
| shared/src/assets/*.ts | Static data arrays for schools and majors selection |
| apps/site/src/routes/Register.tsx | Main registration page with complex form logic and UI |
| apps/site/src/routes/Resume.tsx | Simple resume download route |
| shared/src/api/types.ts | Updated API types for new registration endpoints |
| Various config files | Package dependencies, Docker setup, and project configuration updates |
Comments suppressed due to low confidence (2)
apps/site/src/routes/Register.tsx:74
- Using window.open with user-provided file URLs could potentially expose users to security risks. Consider using a more secure approach like downloading the file or opening it in a sandboxed iframe.
headers: {
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,43 @@ | |||
| import { useFormikContext } from "formik"; | |||
There was a problem hiding this comment.
The useFormAutosave hook lacks JSDoc documentation. Consider adding documentation that explains its purpose, parameters, behavior (10-second delay, beforeunload handling), and usage examples.
| backgroundColor="#12131A" | ||
| /> | ||
|
|
||
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> |
There was a problem hiding this comment.
Type assertion 'as string' is used without validation. Consider using optional chaining or a type guard to safely handle the error message, as form.errors[name] could be undefined or a different type.
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> | |
| <FormErrorMessage> | |
| {typeof form.errors[name] === "string" ? form.errors[name] : undefined} | |
| </FormErrorMessage> |
| }} | ||
| /> | ||
|
|
||
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> |
There was a problem hiding this comment.
Type assertion 'as string' is used without validation. Consider using optional chaining or a type guard to safely handle the error message, as form.errors[name] could be undefined or a different type.
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> | |
| <FormErrorMessage> | |
| {typeof form.errors[name] === "string" ? form.errors[name] : null} | |
| </FormErrorMessage> |
| }} | ||
| /> | ||
|
|
||
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> |
There was a problem hiding this comment.
Type assertion 'as string' is used without validation. Consider using optional chaining or a type guard to safely handle the error message, as form.errors[name] could be undefined or a different type.
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> | |
| <FormErrorMessage> | |
| {typeof form.errors[name] === "string" ? form.errors[name] : null} | |
| </FormErrorMessage> |
| )} | ||
| </HStack> | ||
|
|
||
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> |
There was a problem hiding this comment.
Type assertion 'as string' is used without validation. Consider using optional chaining or a type guard to safely handle the error message, as form.errors[name] could be undefined or a different type.
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> | |
| <FormErrorMessage> | |
| {typeof form.errors[name] === "string" ? form.errors[name] : null} | |
| </FormErrorMessage> |
| {label} | ||
| </FormLabel> | ||
| </Checkbox> | ||
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> |
There was a problem hiding this comment.
Type assertion 'as string' is used without validation. Consider using optional chaining or a type guard to safely handle the error message, as form.errors[name] could be undefined or a different type.
| <FormErrorMessage>{form.errors[name] as string}</FormErrorMessage> | |
| <FormErrorMessage> | |
| {typeof form.errors[name] === "string" ? form.errors[name] : null} | |
| </FormErrorMessage> |
| colorScheme="red" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| void form.setFieldValue(name, ""); |
There was a problem hiding this comment.
Setting field value to empty string instead of null is inconsistent with the type definition. The field type expects 'UploadFile | null', so consider using null instead of an empty string for consistency.
| void form.setFieldValue(name, ""); | |
| void form.setFieldValue(name, null); |
oops mb