London | 26-ITP-Sep | Alan Mak | Sprint 1 | Form Controls - #1515
AlanGit-debug2604 wants to merge 6 commits into
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
AlanGit-debug2604
added
the
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
The validation is all correct and the file is clean, validator and Prettier both pass. Two things to sort before it can be marked Complete:
- The colour radios have no caption. On the page a customer just sees Red, Green, Blue with nothing saying what they're choosing, and a screen reader has even less to go on. See line 23.
- The footer still says HOMEWORK SOLUTION, line 45.
Your questions:
- One is enough. The browser treats radios that share a
nameas one group, and if any radio in the group hasrequiredthe whole group needs a selection. The usual practice is to put it on the first radio, so the next person reading the code finds it where they expect. Putting it on all three does no harm, it's just repeated. MDN covers this under "required" here: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/radio - Both meet the task, so this is a design question rather than a code one. The GOV.UK Design System has short, research-backed guidance on when to use each: https://design-system.service.gov.uk/components/radios/ and https://design-system.service.gov.uk/components/select/
- "Info" is neither an error nor a warning, so the checklist is satisfied. The
/before>on elements like<input>does nothing in HTML, the browser ignores it. Prettier adds it by default, which is why it's there. Leave it or remove it, both are valid.
Add the Needs Review label again once you've pushed.
| <label for="Email">Your email address</label> | ||
| <input type="email" id="Email" required /> | ||
| <!-- Colour Any one from three options available--> | ||
| <input type="radio" name="colour" id="Red" value="Red" required /> |
There was a problem hiding this comment.
Nothing on the page tells the customer what these three radios are for, and a <label> on its own can't caption a group. Which element does the MDN page from the prep on structuring a form use to group radio buttons and give them a caption?
There was a problem hiding this comment.
The fieldset and legend elements =)
There was a problem hiding this comment.
That's the one. A screen reader now reads the legend before each of the three options.
| this will also help you fill in your PR message later--> | ||
| <!-- Name: required, at least 2 non-space characters--> | ||
| <label for="Your-name">What is your name</label> | ||
| <input type="text" id="Your-name" pattern=".*\S.*\S.*" required /> |
There was a problem hiding this comment.
The radios have a name attribute, this input and the email and size don't. What does name do when a form is submitted? Submit the form and look at the address bar. Not a blocker.
abdishakoor-dev
added
Reviewed
AlanGit-debug2604
added
the
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
Fieldset and legend in, validator and Prettier both pass. One small thing left before I mark this Complete: the placeholder in the footer said "change to your name", and the comment saying so is still there on line 47. It now shows your GitHub username. Put your name in, remove that comment, and add the Needs Review label again.
| <footer> | ||
| <!-- change to your name--> | ||
| <p>By HOMEWORK SOLUTION</p> | ||
| <p>By AlanGit-debug2604</p> |
There was a problem hiding this comment.
The starter comment above asks for your name, and your PR title gives it. Small detail, but it's what a customer would see at the bottom of the page.
There was a problem hiding this comment.
Thank you. Updated as requested.
abdishakoor-dev
removed
the
Needs Review
AlanGit-debug2604
added
the
Needs Review
abdishakoor-dev
left a comment
abdishakoor-dev
left a comment
There was a problem hiding this comment.
All done, marking this Complete. Well done Alan.
abdishakoor-dev
added
Complete
2 participants
Learners, PR Template
Self checklist
Task code
CYF-1004
Changelist
Create a T-shirt order form. Included required name (regex-validated), email, colour (radio group, 3 options), and size (dropdown, 6 options). Verified negative/positive validation paths. Lighthouse accessibility score 100, W3C validator shows no errors or warnings.
Questions
Why those outcomes happen and what they are after?