ITP-JAN-25 | Marco Martin Camon | Onboarding module - Week 1 #113
Conversation
…ropdown with options to Colour and Size.
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
… review indicated
… (and including) a # symbol, as indicated in the review
|
Hi @NeoBalder - which exercise would you like a review on here? I can see that there is work for both the form controls and the wireframe exercise here. Each exercise should ideally be done in its own branch, because pull requests refer to branches, and so if other unconnected work gets put into a branch for a particular feature it confuses that work. |
| </header> | ||
| <main> | ||
| <form> | ||
| <p> <span style="color:red">*</span><span><label for="FullName">Name: </label><input type="text" id="FullName" name="FullName" required pattern="[A-Za-z]{3,}"></span></p> |
There was a problem hiding this comment.
Please, avoid inline styling and use separate file (style.css) or at least add all your styles into <style> section in your HTML.
There was a problem hiding this comment.
For attributes, please, use lowercase values and hyphens.
Bad:
for="FullName"
Good:
for="full-name"
| </header> | ||
| <main> | ||
| <form> | ||
| <p> <span style="color:red">*</span><span><label for="FullName">Name: </label><input type="text" id="FullName" name="FullName" required pattern="[A-Za-z]{3,}"></span></p> | ||
| <span style="color:red">*</span><label for="EmailAddress">Your Email Address: </label><input type="text" id="EmailAddress" name="EmailAddress" required pattern="[a-z0-9._%+\-]+@[a-z0-9.\-]+\.[a-z]{2,}$"></span> |
There was a problem hiding this comment.
Please, format your code for better readability - split it to new lines, don't group many elements into one line.
tyzia
added
Reviewed
There was a problem hiding this comment.
Oh I found this half done review in a tab! It's probably a bit late but have it anyway.
Thanks so much for your hard work, @NeoBalder
| </header> | ||
| <main> | ||
| <form> | ||
| <p> <span style="color:red">*</span><span><label for="FullName">Name: </label><input type="text" id="FullName" name="FullName" required pattern="[A-Za-z]{3,}"></span></p> | ||
| <span style="color:red">*</span><label for="EmailAddress">Your Email Address: </label><input type="text" id="EmailAddress" name="EmailAddress" required pattern="[a-z0-9._%+\-]+@[a-z0-9.\-]+\.[a-z]{2,}$"></span> |
There was a problem hiding this comment.
Is there a type of input that validates email addresses automatically?
| </p> | ||
|
|
||
| <p>All fields marked with (<span style="color:red">*</span>) are mandatory</p> | ||
| <button>Sumbit</button> |
There was a problem hiding this comment.
| <option value="green">Green</option> | ||
| <option value="black">Black</option> | ||
| </select> | ||
| </span></p> |
There was a problem hiding this comment.
I'm not sure these spans and ps are necessary. What do you think?
| </header> | ||
| <main> | ||
| <form> | ||
| <p> <span style="color:red">*</span><span><label for="FullName">Name: </label><input type="text" id="FullName" name="FullName" required pattern="[A-Za-z]{3,}"></span></p> |
There was a problem hiding this comment.
What would happen if my name was O'Donnell?
https://www.kalzumeus.com/2010/06/17/falsehoods-programmers-believe-about-names/
|
Stopped watching this PR for changes. If you will need my review for this PR in the future, please, ping me and add 'Needs review' label. |
Self checklist
Changelist
Briefly explain your PR.
-Added input boxes and their names
-Added form controls with HTML validations for name and email. Added dropdown with options to Colour and Size.
-Updated the title and the footer
-Added a styled "*" to the mandatory fields and a line explaining the meaning
-Added ID and Name to each form field
-Added labels
-Tested the website for accessibility in Lighthouse
download2.pdf
Questions
Ask any questions you have for your reviewer.