-
Notifications
You must be signed in to change notification settings - Fork 2
Registration page #6
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
base: main
Are you sure you want to change the base?
Registration page #6
Conversation
|
ah its volo |
|
I will add working google docs sync once this is approved and I get access to the gmail account that you guys want to use |
|
@kotelyakvatwit there's assets in the discord channel in a folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a registration form for what appears to be a hackathon event. The implementation includes a Svelte frontend with form validation, a backend API endpoint for handling submissions, and supporting validation logic. The PR also adds three decorative images displayed alongside form sections and updates package dependencies.
Key Changes:
- New registration form with personal info, school, major, food allergies, t-shirt size, and project idea fields
- Client and server-side validation for form submissions
- API endpoints for submitting and viewing registrations
- Three images added via Git LFS for visual enhancement
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/register/+page.svelte | New registration form component with multi-section layout, real-time validation, and word count tracking |
| src/routes/api/register/+server.ts | API endpoints for handling registration POST requests and GET requests to view submissions |
| src/lib/registration.ts | Validation logic and type definitions for registration form data |
| static/images/hack-identity.png | Decorative image for identity section (Git LFS) |
| static/images/hack-school.png | Decorative image for school section (Git LFS) |
| static/images/hack-food.png | Decorative image for food preferences section (Git LFS) |
| package-lock.json | Dependency updates removing "peer: true" flags from several packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| projectWordCount = countWords(form.projectIdea); | ||
| projectTooLong = projectWordCount > MAX_WORDS; | ||
|
|
||
| console.log(`Project idea word count: ${projectWordCount}`); |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The console.log statement should be removed before production. Debug logging statements should not be left in production code as they can clutter the console and potentially expose implementation details.
| const submissions: any[] = []; | ||
|
|
||
| export const POST: RequestHandler = async ({ request }) => { | ||
| let body: any; |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'any' type removes type safety. Consider creating a proper type for the submission object or using 'unknown' with proper type guards. This would help catch potential type-related bugs at compile time.
| const submissions: any[] = []; | |
| export const POST: RequestHandler = async ({ request }) => { | |
| let body: any; | |
| type RegistrationData = ReturnType<typeof validateRegistration> extends { ok: true; data: infer D } | |
| ? D | |
| : never; | |
| type Submission = RegistrationData & { | |
| id: string; | |
| createdAt: string; | |
| }; | |
| const submissions: Submission[] = []; | |
| export const POST: RequestHandler = async ({ request }) => { | |
| let body: unknown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only valid thing, but we r in web. Types here are basically non existent u little piece of scrap
| if (typeof payload?.email === "string" && !/^\S+@\S+\.\S+$/.test(payload.email)) { | ||
| errors.email = "Invalid email"; | ||
| } |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The email validation regex is overly simplistic and will accept many invalid email addresses (e.g., "a@b.c" or emails with invalid characters). Consider using a more robust email validation pattern or a dedicated email validation library to ensure proper email format validation.
| $effect(() => { | ||
| if (form.school === "__OTHER__") { | ||
| otherSelected = true; | ||
| }else if (schools.includes(form.school)) { |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after the closing brace before 'else'. The code should be formatted as '} else if' instead of '}else if' for better readability and consistency with standard JavaScript/TypeScript formatting conventions.
| }else if (schools.includes(form.school)) { | |
| } else if (schools.includes(form.school)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bro, fr.... Stop hallucinating random functions before commenting on my formatting
| alert( | ||
| "There are some errors with your submission:\n\n" + | ||
| Object.values(result.errors).join("\n") | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const res = await fetch("/api/register", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(form) | ||
| }); | ||
|
|
||
| const data = await res.json().catch(() => null); | ||
|
|
||
| if (!res.ok) { | ||
| console.error("Server validation failed:", data); | ||
| alert("Submit failed. Check console for validation errors."); | ||
| return; | ||
| } | ||
|
|
||
| alert(`Submitted! id = ${data.id}`); |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using alert() for displaying validation errors and submission feedback provides a poor user experience. Consider implementing inline error messages or a more modern notification system (e.g., toast notifications) that doesn't block the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the thing is.... IT SHOULD BE IMPOSSIBLE TO GET TO THAT ALERT... so yes, no UX affected
| image: { src: "/images/hack-identity.png", alt: "blabla1" } | ||
| }, | ||
| { | ||
| image: { src: "/images/hack-school.png", alt: "blabla2" } |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alt text "blabla2" is not descriptive. Alt text should meaningfully describe the image content for accessibility purposes. Consider using descriptive text like "School registration section illustration" or similar based on what the image actually depicts.
| image: { src: "/images/hack-school.png", alt: "blabla2" } | |
| image: { src: "/images/hack-school.png", alt: "School registration section illustration" } |
| image: { src: "/images/hack-school.png", alt: "blabla2" } | ||
| }, | ||
| { | ||
| image: { src: "/images/hack-food.png", alt: "blabla3" } |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alt text "blabla3" is not descriptive. Alt text should meaningfully describe the image content for accessibility purposes. Consider using descriptive text like "Food preferences section illustration" or similar based on what the image actually depicts.
| image: { src: "/images/hack-food.png", alt: "blabla3" } | |
| image: { src: "/images/hack-food.png", alt: "Food preferences section illustration" } |
| schools = []; | ||
| } finally { | ||
| schoolsLoading = false; | ||
| console.log("Finished loading schools."); |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The console.log statement should be removed before production. Debug logging statements should not be left in production code.
| console.error("Server validation failed:", data); | ||
| alert("Submit failed. Check console for validation errors."); |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The console.error statement should be removed or replaced with proper error handling before production. Consider using a proper logging service or error reporting mechanism instead of console logging in production code.
| console.error("Server validation failed:", data); | |
| alert("Submit failed. Check console for validation errors."); | |
| alert( | |
| `Submit failed. ${data?.message ?? "Server validation failed. Please review your input and try again."}` | |
| ); |
| import { json } from "@sveltejs/kit"; | ||
| import type { RequestHandler } from "./$types"; | ||
| import { validateRegistration } from "$lib/registration"; | ||
|
|
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storing submissions in an in-memory array will lose all data when the server restarts. This is not suitable for production use. Consider implementing persistent storage using a database or file system, or at minimum, add a comment documenting that this is temporary/for development only.
| // NOTE: In-memory store for development/debugging only. Not suitable for production. | |
| // In production, replace this with persistent storage (e.g., database or filesystem). |
I guess working version, send me ideas on design as well as pictures …
Jasper, don't be lazy