ZA | MAY-2025 | NOKWANDA PHEWA | FORM CONTROLS #646
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Code is error-free and well indented, and form implementation is solid! Great job!
Regarding your implementation
-
Can you address my suggestions/questions which I left as inline comment with the code?
-
The Lighthouse Accessibility score of your page is not yet 100. Can you improve the score to 100?
Regarding your PR description
-
Can you format the checked boxes using the proper Markdown syntax so that they look something like this?
(With proper Markdown syntax, we can use mouse to check/uncheck the items) -
Can you also provide a brief description (under the "Changelist" section) summarizing the purpose of the PR and the changes you’ve made?
The code for the "Changelist" section looks like this in the PR template:
### Changelist
Briefly explained your PR.
Form-Controls/index.html
Outdated
| <input type="radio" id="xs" name="size" value="XS" required> | ||
| <label for="xs">XS</label><br> | ||
|
|
||
| <input type="radio" id="s" name="size" value="S"> | ||
| <label for="s">S</label><br> | ||
|
|
||
| <input type="radio" id="m" name="size" value="M"> | ||
| <label for="m">M</label><br> | ||
|
|
||
| <input type="radio" id="l" name="size" value="L"> | ||
| <label for="l">L</label><br> | ||
|
|
||
| <input type="radio" id="xl" name="size" value="XL"> | ||
| <label for="xl">XL</label><br> | ||
|
|
||
| <input type="radio" id="xxl" name="size" value="XXL"> | ||
| <label for="xxl">XXL</label><br><br> |
There was a problem hiding this comment.
Between using radio buttons and using a drop-down list, which of them is more suitable for reading one of six sizes?
cjyuan
added
Reviewed
|
Thank you for your review @cjyuan , i have made improvements and also updated the PR description with proper markdown checkboxes. Could you kindly review again and let me know if i need to make further adjustments.Thanks |
There was a problem hiding this comment.
The changes you made are quite good.
-
The Lighthouse Accessibility score is not yet 100. Can you improve the score to 100?
-
We can reply directly to an inline comment to keep all related discussions organized in one place. Can you address this inline comment in my previous comment?
-
Don't forget to replace the label from "Reviewed" to "Needs review" if your changes is ready to be reviewed.
| minlength="2" pattern=".*[^ ].*" | ||
| title="Please enter at least 2 non-space characters" |
There was a problem hiding this comment.
The description given in the title attribute does not quite match what the pattern check.
Current pattern can guarantee only "at least one non-space character" in the name.
Changelist
-Implemented a complete t-shirt order form with validation that meets all project requirements.
Self-Checklist