-
-
Notifications
You must be signed in to change notification settings - Fork 416
Manchester | 26-ITP-Jan | Mehroz Munir | Sprint 2 | Form Controls #1030
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?
Manchester | 26-ITP-Jan | Mehroz Munir | Sprint 2 | Form Controls #1030
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi @MehrozMunir, your form controls exercise is done very well. The form validation works nicely and you have used semantic HTML. I think the use of I left a few comments in the code. Please have a look. |
|
Hi @jenny-alexander |
|
Hi @MehrozMunir - I changed the PR back to 'reviewed' since you have not addressed my comments on code changes. The work isn't complete until the reviewer changes the label to 'Complete'! |
|
Hi @jenny-alexander , sorry for the confusion earlier. I misunderstood your comments and thought they were general instructions for future work, so I didn’t make the changes at the time. I also overlooked the rule that the reviewer should change the label from reviewed to complete, not me; apologies for that. I’ve now committed the changes you mentioned in the code comments. Could you please take another look when you have a moment? Thank you! |
Form-Controls/index.html
Outdated
| @@ -1,27 +1,96 @@ | |||
| <!DOCTYPE html> | |||
| <!doctype html> | |||
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.
Can you research and see if doctype should be uppercase or lowercase?
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.
Yeah I see that now. Though it does not make any difference in the output as the HTML5 is case insensitive to the tag Doctype but the standard is to use with the uppercase "D". Thank you. I have corrected my code with capital D now.
Form-Controls/index.html
Outdated
| <footer> | ||
| <!-- change to your name--> | ||
| <h2>By HOMEWORK SOLUTION</h2> | ||
| <h2>By Mehroz Munir</h2> |
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.
Perhaps something other than <h2> would be a better choice for the footer? By using <h2> you are announcing a major section header within your footer.
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.
Yeah that's right. I have changed it from heading to putting it inside the <small> tag.
|
@MehrozMunir I apologize, I forgot to hit the 'submit' button with my 2 feedback comments. Would you be able to have a quick look at what I wrote? |
|
Hi @jenny-alexander Thank you for highlighting the issues in your comments. I have tried to solve them and committed my changes. Take a look please. |
|
Nice job 👏 @MehrozMunir |

Learners, PR Template
Self checklist
Changelist
I have created a form using html to get inputs from the user including their name, email, color and size of the shirt they want. And a submit button.