-
-
Notifications
You must be signed in to change notification settings - Fork 410
Cape Town | 26-ITP-Jan | Pretty Taruvinga | Sprint 2 | Form Controls #1097
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?
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
6 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cjyuan
left a comment
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.
Code looks good.
Regarding your question:
How did you decide what makes a “valid” customer name, and what rules did you use to validate it?
I would probably reference the answers gathered from AI and Google search.
If there is a rule, then chances are we can use JavaScript to enforce the rule.
| <input | ||
| type="radio" | ||
| id="color-blue" | ||
| name="color" | ||
| value="blue" | ||
| /> | ||
| <label for="color-blue">Blue</label> |
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.
Note:
We can also wrap the radio button inside the label element as:
<label><input type="radio" name="color" value="blue"> Blue</label>This way, we don't have to introduce id and for attributes.
| required | ||
| /><br><br> | ||
| <fieldset> | ||
| <legend>Choose a T-shirt Colour:</legend> |
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 wordings used on lines 32 and 58 could be made more consistent.
May I suggest showing your code to an AI tool and ask it to check for consistency and possible improvements?
It can often help us catch errors, improve consistency, and reinforce best practices.

Learners, PR Template
Self checklist
Changelist
Questions
How did you decide what makes a “valid” customer name, and what rules did you use to validate it?