Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 1 | Form Control#1270
Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 1 | Form Control#1270Monsur0001 wants to merge 10 commits into
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Monsur0001
added
Module-Onboarding
Luro91
added
the
Review in progress
| <label for="name">Name</label> | ||
| <input | ||
| type="text" | ||
| name="name" |
There was a problem hiding this comment.
How can you ensure that the name needs to have at least 2 characters?
There was a problem hiding this comment.
Thank you so much for your response and feedback, I have ensured the form requires at least 2 characters. I included the 'minlength' attribute and value of '2' to solve the issue.
| <label for="email">Email</label> | ||
| <input | ||
| type="email" | ||
| email="email" |
There was a problem hiding this comment.
The w3c validator shows me an error here. "Attribute email not allowed on element input at this point." How can you fix this?
There was a problem hiding this comment.
Thank you so much for your feedback, I have substituted the wrong 'email' attribute to the right one 'name'. Thank you so much.
| </div> | ||
| </fieldset> |
There was a problem hiding this comment.
The indentation of the children does not use the standards. How can you ensure consistent formatting in your code?
There was a problem hiding this comment.
I have removed the div placed before the fieldset, to ensure a consistency in formatting
There was a problem hiding this comment.
You did not need to remove the div. I was referring to Prettier which you can use to automatically format your code consistently.
There was a problem hiding this comment.
Oh! Thank you so much for this. I’ll definitely take that into cognisance
thank you so much for taking the time to review and give adequate feedback
| <select name="color" id="color" required> | ||
| <option value="White">White</option> | ||
| <option value="Green">Green</option> | ||
| <option value="Black">Black</option> | ||
| </select> | ||
| </div> | ||
| <div> | ||
| <fieldset> | ||
| <Legend>Size</Legend> | ||
|
|
||
| <div class="size-option"> |
There was a problem hiding this comment.
A note on the different types of form inputs. I would use a radio button if there are only a few options and a dropdown menu if there are a lot of options. (Space is often limited on pages, and this can save some space). So I would switch the types for colors and sizes. (You don't need to change it now. Just something you can consider the next time)
There was a problem hiding this comment.
Thank you so much for your feedback and enlightenment on this. I will put that into cognizance next time. I really do appreciate that.
Luro91
added
Reviewed
Monsur0001
added
the
Needs Review
Luro91
left a comment
Luro91
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments and thanks for using single commits for each of the fixes. This makes it easier to follow.
Luro91
added
Complete
|
Thank you so much for the kind words, and for taking the time to review my PR |
2 participants
Form Control Completed