Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds edit functionality and a warn state for duplicate registrations to the registration form. The changes allow users to check if a registration exists, load it for editing, and update it. Additionally, an "isPresenting" checkbox field is introduced to allow attendees to register without presenting a talk.
Changes:
- Added
isPresentingcheckbox field to differentiate between presenters and attendees - Implemented duplicate detection with warning state and edit prompt
- Added registration edit functionality (load existing registration by name and update)
- Applied comprehensive dark mode styling across all form elements and states
Comments suppressed due to low confidence (1)
app/register/page.tsx:59
- The "isDuplicate" state is not reset when successfully loading an existing registration. After a duplicate error triggers the warning state and then a user successfully loads their registration, the warning styling remains active unnecessarily. Add "setIsDuplicate(false)" after successfully loading the registration data at line 56 to clear the warning state.
if (result.success && result.data) {
// Load the data into the form
setFormData({
name: result.data.name || '',
isPresenting: result.data.isPresenting === 'Yes',
talkTitle: result.data.talkTitle || '',
description: result.data.description || '',
expectations: result.data.expectations || '',
});
setIsEditing(true);
setShowLoadForm(false);
setLoadName('');
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="mb-6"> | ||
| <label htmlFor="name" className="block text-sm font-semibold text-black mb-2"> | ||
| <label htmlFor="name" className="block text-sm font-semibold text-black dark:text-white darkest:text-white mb-2"> | ||
| Name (Presenter) <span className="text-red-500">*</span> |
There was a problem hiding this comment.
The label "Name (Presenter)" assumes all registrants are presenters, but the form now includes an "I'm presenting a talk" checkbox indicating that not all attendees need to be presenters. This creates inconsistency in the user interface. Consider changing the label to just "Name" or "Full Name" to accurately reflect that attendees can register without presenting.
| Name (Presenter) <span className="text-red-500">*</span> | |
| Full Name <span className="text-red-500">*</span> |
| className="w-5 h-5 cursor-pointer" | ||
| /> | ||
| <span className="text-sm font-semibold text-black dark:text-white darkest:text-white"> | ||
| I'm presenting a talk <span className="text-red-500">*</span> |
There was a problem hiding this comment.
The required asterisk on the "I'm presenting a talk" checkbox is misleading. The form allows submission without presenting (when unchecked), so this shouldn't be marked as required. Only the name field and conditionally the talk fields (when presenting) should be marked as required.
| I'm presenting a talk <span className="text-red-500">*</span> | |
| I'm presenting a talk |
| type="text" | ||
| id="talkTitle" | ||
| name="talkTitle" | ||
| required |
There was a problem hiding this comment.
When the "talkTitle" and "description" fields have the "required" attribute and are conditionally rendered based on "isPresenting", this creates a form validation issue. If a user loads an existing registration where they were presenting, then unchecks the checkbox (clearing the fields), and tries to submit, the hidden required fields may still trigger validation errors in some browsers. Consider adding explicit validation logic in the handleSubmit function to ensure consistency across browsers.
| <textarea | ||
| id="description" | ||
| name="description" | ||
| required |
There was a problem hiding this comment.
The same validation issue applies to the "description" textarea field. When conditionally rendered with a "required" attribute, it may cause form validation issues in some browsers if the field is hidden. This needs to be handled consistently with the "talkTitle" field.
| description: isChecked ? formData.description : '' | ||
| }); | ||
| }} | ||
| className="w-5 h-5 cursor-pointer" |
There was a problem hiding this comment.
The checkbox input lacks styling for keyboard focus states needed for accessibility. While dark mode colors are applied throughout the form, the checkbox should also include focus ring styles to provide visual feedback for keyboard users navigating the form. Consider adding focus styling classes like "focus:ring-2 focus:ring-black dark:focus:ring-white" to match other form inputs.
| className="w-5 h-5 cursor-pointer" | |
| className="w-5 h-5 cursor-pointer focus:ring-2 focus:ring-black dark:focus:ring-white darkest:focus:ring-white focus:border-black dark:focus:border-white darkest:focus:border-white outline-none transition" |
This pull request implements the edit functionality (check if existing registration exists, load, and update) for the registration form.
Closes the following issues: