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.
|
Great work on this, The form implementation looks solid and addresses all requirements: Correct input fields and semantic structure (Name, Email, Colour, Size). Proper regex validation (.\S.\S.*) ensuring at least two non-space characters for the name. Complete size dropdown list with all 6 required options (XS through XXL). Strong accessibility setup with linked labels (for) and aria-describedby helper text. This PR is approved and ready to merge. |
|
I am having trouble doing a merge on this PR. I cannot see the merge button as I passed all criteria? any guidance would be welcome. |
To learn more about Markdown, see
|
cjyuan
left a comment
There was a problem hiding this comment.
Code is well formatted and free of syntax error. Well done.
Could you address the inline comments?
Suggestion: Use AI to explore "How to address inline comments in a PR?" and then practice what you learn in this PR.
There was a problem hiding this comment.
You were supposed to implement the form in index.html.
| <style> | ||
| body { | ||
| font-family: Arial, sans-serif; | ||
| background: #f7f7f7; | ||
| padding: 40px; | ||
| line-height: 1.6; | ||
| } |
There was a problem hiding this comment.
Could you move the internal CSS to an external fie? It is best practice to separate CSS from HTML.
| id="customer-name" | ||
| name="customer-name" | ||
| required | ||
| pattern=".*\S.*\S.*" | ||
| aria-describedby="nameHelp" | ||
| > | ||
| <small id="nameHelp">Name must contain at least two non‑space characters.</small> |
There was a problem hiding this comment.
customer-name- This naming convention is called kebab-case.nameHelp- This naming convention is called camelCase.
It's best practice to use a consistent naming convention throughout.
Self checklist
Needs review
CYF-1004