-
-
Notifications
You must be signed in to change notification settings - Fork 419
Cape Town | 26-ITP-JAN | Shannon Davids | Sprint 2 | Form Controls #1126
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?
Cape Town | 26-ITP-JAN | Shannon Davids | Sprint 2 | Form Controls #1126
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jenny-alexander
left a comment
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.
Hi @Shannon-Davids - good job with the form control exercise 👏. Your form works as intended. I left a few comments for you to review.
💡 In VSCode, you can format your document to make it easier to read. Right-click within the code. This will bring up a menu. You should see 'Format Document'. Click on that to align the code.
Form-Controls/index.html
Outdated
| <footer> | ||
| <!-- change to your name--> | ||
| <h2>By HOMEWORK SOLUTION</h2> | ||
| <h2>By Shannon Davids</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.
<h2> announces a major section in the footer. Since your name is the only thing in the footer, a different tag should be used. Can you think of a more appropriate 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.
I think a
tag i have made the changes
Form-Controls/index.html
Outdated
| <br> | ||
| <div> | ||
| <label for="size">T-shirt Size</label> | ||
| <select name"size" id="size" required> |
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.
There is a small typo on line 39, can you find it?
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.
found it a fixed it
Form-Controls/index.html
Outdated
|
|
||
| <div> | ||
| <label for="name">Customer Name:</label> | ||
| <input type="text" name="name" id="name" placeholder="Username" 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.
The label on this input is 'Customer Name' but the placeholder is 'Username'. This might be a bit confusing for a user. Can you use the same wording for both label and placeholder?
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.
updated it to be the same
|
Good work @Shannon-Davids |

Learners, PR Template
Self checklist
Changelist
I have edited form as required with semantic Html
Questions
No Questions