-
-
Notifications
You must be signed in to change notification settings - Fork 341
West Midlands | ITP-September-2025 | Mustaf Asani | Sprint 2 | Form-Controls #889
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. |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes) If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
|
Your PR description contained template fields which weren't filled in. Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
Form-Controls/index.html
Outdated
| try writing out the requirements first as comments | ||
| this will also help you fill in your PR message later--> | ||
| <fieldset> | ||
| <legend><h2>Personal Information</h2></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.
do you think the legend tag can just have the text string without the h2 tag or have another inline tag like strong to replace h2 to make it bold?
Form-Controls/index.html
Outdated
| </p> | ||
| </fieldset> | ||
| <fieldset> | ||
| <legend><h2>Product Information</h2></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.
strong tag is useful here instead of h2 inside legend tag.
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.
took off the h2 tag as it turns out it was not doing anything for accessibility or semantics.
Form-Controls/index.html
Outdated
| <option value="xxlarge">XX-Large</option> | ||
| </select> | ||
| </p> | ||
| </fieldset> |
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.
Is the form missing a submit button? could be useful in testing field validation, even without form action?
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.
added a submit button to test the validation and prompts are given if the validation parameters are not met.
Form-Controls/index.html
Outdated
| <label for="firstname">Firstname:</label> | ||
| <input type="text" id="firstname" pattern=".*\S.*" title="firstname must not be only spaces" name="firstname" required minlength="2" /> | ||
| <Label for="lastname">Lastname:</Label> | ||
| <input type="text" id="lastname" pattern=".*\S.*" title="lastname must not be only spaces" name="lastname" required minlength="2" /> |
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.
Do you think each input element can have its own p tag or div tag, for layout flexibility and accessibility?
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.
used a div tag to separate the layout for the 2 input sections name and email.
Form-Controls/index.html
Outdated
| <label for="firstname">Firstname:</label> | ||
| <input type="text" id="firstname" pattern=".*\S.*" title="firstname must not be only spaces" name="firstname" required minlength="2" /> | ||
| <Label for="lastname">Lastname:</Label> | ||
| <input type="text" id="lastname" pattern=".*\S.*" title="lastname must not be only spaces" name="lastname" required minlength="2" /> |
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.
Nice one with the pattern and title. 👍

Learners, PR Template
Self checklist
Changelist
Sprint 2 task, created a form to given specifications with only html and ran it in lighthouse then made adjustments as suggested by lighthouse to make accessibility 100%.