Glasgow | Jan-26 | Abraham Habte | Sprint 2 | form control#1016
Glasgow | Jan-26 | Abraham Habte | Sprint 2 | form control#1016Abrsh100 wants to merge 3 commits into
Conversation
…l as placeholders
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Abrsh100
added
the
Needs Review
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on this task, I've left some comments with pointers
| <span>Customer Name: </span> | ||
| <strong><span aria-label="required">*</span></strong> | ||
| </label> | ||
| <input type="text" id="name" name="customer-name" required pattern="[A-Za-z\s\.]+" minlength="2" maxlength="40"/> |
There was a problem hiding this comment.
Some people have very long names,how would your application react to this?
There was a problem hiding this comment.
I just think that most name is not more than 40 character but if the name is more than than I can delete teh maxlength and then there will be no limit for the name character
| <span>Email: </span> | ||
| <strong><span aria-label="required">*</span></strong> | ||
| </label> | ||
| <input type="email" id="email" name="customer-mail" required minlength="5" maxlength="50" placeholder="example@gmail.com" /> |
There was a problem hiding this comment.
same comment as for the names, if they have a very long email, would they still be able to use your app?
There was a problem hiding this comment.
I just limit the length in to 50, but if i thought that there is email more than that i can change the maxlenght to more than 50
| <span>Color:</span> | ||
| <strong><span aria-label="required">*</span></strong> | ||
| </label> | ||
| <select id="color preference" name="color preference"> |
There was a problem hiding this comment.
You offer one of the options pre-selected here. What do you think would happen if someone pressed the "enter" key thinking it would take them through the form to make choices?
There was a problem hiding this comment.
based on this the color is selected by default, but I can add another option in which it will not select a color but disable it
LonMcGregor
added
Reviewed
Abrsh100
added
Needs Review
|
Good work adding a default for colour - maybe you could also do the same for size? It's always good to see if something you learn in one area can be applied elsewhere. As for removing maximums - have another read of the README file. It contains the spec for this task. It gives a minimum, but never specifies a maximum. What do you think that means? |
LonMcGregor
added
Reviewed
|
Thanks i read the README file it does not specify the maximum that means no need to have a maximam limitation so I omit it |
Abrsh100
added
Needs Review
|
Great! this task is complete now. |
LonMcGregor
added
Complete
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
3 participants
Self checklist
Changelist
creatining all the requerd form with requirment and placeholders
Questions
No Questions