sheffield_ArminNouri_Form controls _Week 2 #210
Conversation
…ncluding their name, address, and details about the selected T-shirt such as size, color, and shipping date.
✅ Deploy Preview for cyf-ufd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ArminNouri98
changed the title
ArminNouri98
changed the title
ArminNouri98
changed the title
ArminNouri98
added
👀 Review Testing
ArminNouri98
changed the title
ArminNouri98
added
👀 Review Requirements
SallyMcGrath
left a comment
SallyMcGrath
left a comment
There was a problem hiding this comment.
Hi there. What is the purpose of the JS you have submitted?
SallyMcGrath
removed
📅 Sprint 2
ArminNouri98
added
Reviewed
|
I checked everything and realized I had made many mistakes, so I also deleted the JavaScript. I looking forward! |
jamesbaskerville
left a comment
jamesbaskerville
left a comment
There was a problem hiding this comment.
Good start @ArminNouri98! My main feedback is focused on:
- requirements -- there are a few that have been missed
- styling -- you have a lot of styles, and some of it conflicts/gets overwritten!
| <div> | ||
| <label for="date">Choose Delivery Date:</label> | ||
|
|
||
| <input type="date" name="date" id="date" min="31/10/2024" max="31/11/2024" required=""> |
There was a problem hiding this comment.
I notice that the date picker allows dates outside of your bounds here. Any ideas why?
Also, what does required="" do?
There was a problem hiding this comment.
thank you for notice this i relisede I wrote wrong format it should be yyyy/mm/dd.
required is force to write here but I can set what is format what i want inside of "" this.
| <div> | ||
| <label for="address">Address</label> | ||
| <input type="text" id="address" name="address" required> | ||
| </div> | ||
| <div class="postcode"> | ||
| <label for="postcode">Postcode</label> | ||
| <input type="text" id="postcode" name="postcode" required> | ||
| </div> |
There was a problem hiding this comment.
Do we need this information? If so, how should we validate it (e.g. Postcodes have a certain format)
There was a problem hiding this comment.
I think for send Item we need address but for validate I can add patern on inpute for the check
Post code:pattern="^[A-Z]{1,2}\d[A-Z\d]? \d[A-Z]{2}$" like this
|
|
||
| input, select, button { | ||
| color: #1a1a1a; /* Darker text for better contrast */ | ||
| background-color: #ffffff; /* White background for form controls */ |
There was a problem hiding this comment.
I notice that your form control backgrounds are not white -- what is your intention?
There was a problem hiding this comment.
I'm not haveing really good realtions with colors but for take 100% accebility i should do it.
| padding: 10px 0; | ||
| margin-top: 20px; | ||
| } | ||
| button#choose { |
There was a problem hiding this comment.
is there a way we can target the submit button of the form without using its ID?
There was a problem hiding this comment.
yeah I think there is just one button so we can but I couldn't remeber why I did this ?:)
| <div class="formbox"> | ||
| <div> | ||
| <label for="name">Name</label> | ||
| <input type="text" id="name" name="name" required> |
There was a problem hiding this comment.
What are the validation criteria for names in the requirements?
There was a problem hiding this comment.
just force to write name but if we want we can write a Pattern for the name
pattern="^[A-Za-z]+(([',. -][A-Za-z ])?[A-Za-z])$"
jamesbaskerville
added
👀 Review Testing
delete js code from html
delete index.js file
ArminNouri98
added
Complete
4 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
I did as much as I did in one commet! i should make more commit?
Ask any questions you have for your reviewer.