add task solution - #801
Conversation
commented
Jun 18, 2023
|
should be visible now |
left a comment
There was a problem hiding this comment.
Check this checklist https://github.com/mate-academy/layout_html-form/blob/master/checklist.md, you need to fix:
- Formatting, make sure the indentation is correct, also add empty lines between sibling elements like in this example:
<div class="field_wrapper">
<label for="surname">Surname: </label>
<input
type="text"
name="surname"
id="surname"
autocomplete="off"
>
</div>- Also avoid stylin by using element selector, add class to the element and style it using class like:
html file:
<div class="field_wrapper">
<label for="surname">Surname: </label>
<input
type="text"
name="surname"
id="surname"
autocomplete="off"
>
</div>css file:
.field_wrapper {
margin-bottom: 10px
}- Add proper labels to all inputs so when the user clicks label it will focus on the proper input (for-id relation):
<label for="name">Name: </label>
<input
type="text"
name="name"
id="name"
autocomplete="off"
>| type="date" | ||
| name="date" | ||
| id="date" | ||
| value=" " |
There was a problem hiding this comment.
Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here
| type="text" | ||
| name="name" | ||
| id="name" | ||
| value=" " |
There was a problem hiding this comment.
Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here
| type="text" | ||
| name="surname" | ||
| id="surname" | ||
| value=" " |
There was a problem hiding this comment.
Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here
| type="email" | ||
| name="email" | ||
| id="email" | ||
| value=" " |
There was a problem hiding this comment.
Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here
| <input | ||
| type="color" | ||
| name="color" | ||
| value=" " |
There was a problem hiding this comment.
Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here
| name="time" | ||
| id="time" | ||
| step="2" | ||
| value=" " |
There was a problem hiding this comment.
Empty space as a default value in input is not a good practice. Please remove this attribute value=" " it's redundant here
| } | ||
|
|
||
| input { | ||
| .field_wrapper { |
There was a problem hiding this comment.
following the BEM naming convention this name should have double underscore mark field__wrapper
|
|
||
| label { | ||
| margin-top: 10px; | ||
| .box_style { |
There was a problem hiding this comment.
following the BEM naming convention this name should have double underscore mark box__style
| name="email" | ||
| id="email" | ||
| placeholder="email@example.com" | ||
| multiple |
There was a problem hiding this comment.
You can use multiple attribute with input type email, but I think that in this case more suitable would be to add multiple attribute to select tag, as this is registration form so we are allowing one email value :)
| <label for="user-password">Password: </label> | ||
|
|
||
| <input | ||
| name="user-password" |
There was a problem hiding this comment.
Value of name attribute should be in camelCase like userPassword
| name="user-password" | ||
| id="user-password" | ||
| type="password" | ||
| required autocomplete="off" |
There was a problem hiding this comment.
Fix formatting:
| required autocomplete="off" | |
| required | |
| autocomplete="off" |
| <input | ||
| type="color" | ||
| name="color" | ||
| > |
There was a problem hiding this comment.
Label for this field is not clickable(when clicking label it should focuse on the input).
Add id which coresponds with for attribute in label
| <input | |
| type="color" | |
| name="color" | |
| > | |
| <input | |
| type="color" | |
| name="color" | |
| id="color" | |
| > |
| <textarea | ||
| name="cars" | ||
| id="cars" | ||
| rows="4" | ||
| cols="2" | ||
| >BMW Audi Lada | ||
| </textarea> |
There was a problem hiding this comment.
Here more suitable would be to use select tag instead of textarea
Here is an example:
<select name="cars" id="cars" multiple>
<option value="volvo">Volvo</option>
<option value="saab">Saab</option>
<option value="opel">Opel</option>
<option value="audi">Audi</option>
</select>This will also fix your test results :)
| <div class="field__wrapper"> | ||
| <label for="comment">Comments: </label> | ||
|
|
||
| <textarea |
| name="comment" | ||
| rows="2" | ||
| cols="20" | ||
| > | ||
| </textarea> |
There was a problem hiding this comment.
<textarea> is a tricky tag to format:
It requires you to put a closing tag on the same line as the > of the opening tag, not to have default content, and be able to see the placeholder.
At the same time, if it has several attributes, you need to put each of them on its own line.
So, use the following formatting where > is moved 1 position left from the normal alignment:
<textarea
class="..."
placeholder="..."
></textarea>| <label for="reco">Would you recommend us?</label> | ||
| <select id="reco" name="reco"> |
| <label for="reco">Would you recommend us?</label> | ||
| <select id="reco" name="reco"> |
There was a problem hiding this comment.
Use more descriptive name and id value like recommendation
| </fieldset> | ||
| <button type="submit">Submit</button> |
There was a problem hiding this comment.
Add empty line between sibling elements
| </fieldset> | |
| <button type="submit">Submit</button> | |
| </fieldset> | |
| <button type="submit">Submit</button> |
| <h1>HTML Form</h1> | ||
| <form | ||
| action="https://mate-academy-form-lesson.herokuapp.com/create-application" | ||
| method="get" |
There was a problem hiding this comment.
Method here should be POST rather than GET -> https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods
| type="radio" | ||
| id="yes" | ||
| name="cat" | ||
| value="Y" |
There was a problem hiding this comment.
This value is very vague, it would be better here to reflect what is checked at the input -> Yes and below No
Uh oh!
There was an error while loading. Please reload this page.