BIRMINGHAM | 26-ITP-JAN | MERVE REIS | Sprint 2 | PRODUCT FORM#1079
BIRMINGHAM | 26-ITP-JAN | MERVE REIS | Sprint 2 | PRODUCT FORM#1079mervereis wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
mervereis
added
the
Needs Review
takanocap
left a comment
takanocap
left a comment
There was a problem hiding this comment.
Please check that you have saved the assignment and pushed to github. I am not sure I can see any solution in the file
takanocap
added
Reviewed
|
@takanocap I've added the product form to the html file. you can check again now |
mervereis
added
the
Needs Review
Form-Controls/index.html
Outdated
| <meta charset="UTF-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Product Form</title> | ||
| <link rel="stylesheet" href="style.css" /> |
There was a problem hiding this comment.
well done for the good Job. You have also included accessibility thats very good. Please keep it up. However, your styles.css may be missing or using the wrong reference. Please check this
Form-Controls/index.html
Outdated
|
|
||
| <div> | ||
| <label for="email">E-mail</label> | ||
| <input type="email" name="E-mail" id="email" placeholder="name@example.com" required aria-required="true" /> |
There was a problem hiding this comment.
is it possible to use the minlength for additional validation for email?
takanocap
removed
the
Needs Review
Form-Controls/index.html
Outdated
|
|
||
| <div> | ||
| <label for="colour">Colour</label> | ||
| <select name="colour" id="colour" required aria-required="true" aria-label="Select a colour"> |
There was a problem hiding this comment.
welldone for the good job.
when we have a visible label could there be a reason why using aria-label will be redundant ? what is the difference between the aria-label and aria-labelledby, when should we use one or the other
would there be a reason why using aria-required and required may be redundant ?
There was a problem hiding this comment.
@takanocap Thanks for the feedback. Since there is already a visible label and the required attribute provides the necessary accessibility information, I’ve reverted the redundant aria-label and aria-required usage.
takanocap
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Created a product pick form