London | 26-ITP-May | Mandip Sanger | Sprint 1 | Form Contols#1440
London | 26-ITP-May | Mandip Sanger | Sprint 1 | Form Contols#1440mandipsanger wants to merge 16 commits into
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.
mandipsanger
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
mandipsanger
added
🏕 Priority Mandatory
Poonam-raj
added
Review in progress
| </footer> | ||
| </body> | ||
| </html> | ||
| <html lang="en"User-agent: * |
There was a problem hiding this comment.
What does <html lang="en"User-agent: * Allow: /> mean?
There was a problem hiding this comment.
oh my bad. I was auto complete.
| <header> | ||
| <h1></h1> | ||
| </header> |
There was a problem hiding this comment.
What is this header doing? There's no text here. Empty elements should be removed
| </select> | ||
| </div> | ||
|
|
||
| <input type="submit" value="Submit"> |
There was a problem hiding this comment.
I would have expected this to be a submit button
This tag is also not closed.
| <div> | ||
| <p>Please select a size</p> | ||
|
|
||
| <label for="size">Choose a size:</label> | ||
| <select name="size" id="size"> | ||
| <option value="xs">Extra Small</option> | ||
| <option value="small">Small</option> | ||
| <option value="medium">Medium</option> | ||
| <option value="large">Large</option> | ||
| </select> | ||
| </div> |
There was a problem hiding this comment.
How come you decided not to wrap this section in a fieldset tag but you wrapped lines 30 - 41 in a fieldset tag?
There was a problem hiding this comment.
Thank you I think I have fixed the issues.
There was a problem hiding this comment.
Could you please review my PR so I can submit the enrollment form.
| <option value="xs">Extra Small</option> | ||
| <option value="small">Small</option> | ||
| <option value="medium">Medium</option> | ||
| <option value="large">Large</option> |
There was a problem hiding this comment.
It could be useful here to have a default select option so that the form doesn't default to picking a size accidentally. E.g. if i don't pick a size, the form will submit XS but I might not want XS. It would be better to have a default value that doesn't let the user submit and forces them to select a size.
Poonam-raj
left a comment
Poonam-raj
left a comment
There was a problem hiding this comment.
Some issues:
- When I leave all the inputs empty and click submit the page reloads like it has submitted. I get no messages saying anything is wrong or that I've missed inputs, but I should get some feedback like that.
- When I submit less than two characters for the Name field I get a generic message that doesn't tell me what is actually wrong (screenshot)
-
When i do input two characters in the Name field, the form lets me submit but I shouldn't be able to because Email and the other fields aren't filled out yet.
-
Throughout your form you lack 'required' tags which would mean the form cannot submit unless the user inputs into the fields. Please look into this.
mandipsanger
added
Needs Review
2 participants
Learners, PR Template
Self checklist
Changelist
I have completed Form Controls task.