London | 25-ITP-May | GISLAINE DELLA BELLA | Sprint 2 | FORM STRUCTURE #835
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Della-Bella
added
the
Needs Review
jenny-alexander
added
Review in progress
|
👋 @Della-Bella - I started to review your PR but I see you have included changes for both the wireframe and form controls exercises. A branch should have the changes for one exercise only. ❓ Is this PR for form controls exercise? 
|
|
Hi @jenny-alexander, thank you |
Della-Bella
added
the
Needs Review
|
@Della-Bella I think you put a lot of good work into this exercise. There are, however, a few syntax errors that should be corrected. Also, for this form controls exercise, only the index.html file should be included in this PR. |
Form-Controls/index.html
Outdated
|
|
||
| <!-- 3 options colour/ pick only 1/ require --> | ||
|
|
||
| <label for="colour-YELLOW"> |
There was a problem hiding this comment.
Normally when defining html attributes, we use lowercase characters.
Can you review your "for" and "id" attribute names and ensure they are in lowercase?
Form-Controls/index.html
Outdated
| <ul> | ||
|
|
||
| <li> | ||
| <label for="size-XS"> |
There was a problem hiding this comment.
Can you check the value of the "for" attribute in your XS size select label and the corresponding "id" value? There is a typo here.
Form-Controls/index.html
Outdated
| </li> | ||
|
|
||
| <li> | ||
| <label for="-XXL"> |
There was a problem hiding this comment.
Small typo on line 99, can you find it?
| <input type="radio" style="transform: scale(2)" id="colour-BLUE" value="BLUE" name="colour" required /> | ||
| <span>Blue </span> | ||
| </label> | ||
|
|
There was a problem hiding this comment.
Can you review all of the radio button values? They have value ="color-*" instead of the size value (for example,value="BLUE").
Form-Controls/index.html
Outdated
| <label for="colour-YELLOW"> | ||
| <p><strong>Colour:</strong></p> | ||
| <input type="radio" style="transform: scale(2)" id="colour-YELLOW" value="YELLOW" name="colour" required /> | ||
| <span>Yellow </span> </span> |
There was a problem hiding this comment.
Small typo at line 46. Can you find it?
Form-Controls/index.html
Outdated
|
|
||
| <li> | ||
| <label for="size-XS"> | ||
| <input type="radio" style="transform: scale(1.5); margin-bottom: 20px"; id="colour-XS" name="size" |
There was a problem hiding this comment.
At line 65, the style syntax has a small error. The semi-colon should not be outside of the quotes. Can you fix it?
I see this typo for all inline styling of size radio buttons.
There was a problem hiding this comment.
@Della-Bella Can you check the other size radio buttons? The semi-colon is outside the double quotes and it should be inside the quotes.
jenny-alexander
added
Reviewed
|
hi @jenny-alexander , |
|
@Della-Bella Have a look at this message about this PR having extra files within it. |
|
hi @jenny-alexander , |
No problem @Della-Bella. I see that you did delete those files. That means that the files will be deleted from the main repository too (if you were to actually push your PR changes to the main branch). I'm sure the person helping you on Saturday will be able to show you how to do this. 😊 |
|
@Della-Bella - were you able to get the help you needed on Saturday? |
|
hi @jenny-alexander, good morning. I forgot I had a family event, travelled to the north last saturday ...I will try it again before going to the next class on Saturday on my own if not I will ask a volunteer there. ;) |
Della-Bella
added
Complete
2 participants
Learners, PR Template
Self checklist
Thank you for your time,