Develop#796
Conversation
natispatis
commented
May 30, 2023
- DEMO LINK
- TEST REPORT LINK
darokrk
left a comment
There was a problem hiding this comment.
Layout looks good, but code needs some improvements :)
| <fieldset class="form__fieldset form-fieldset"> | ||
| <legend class="form-fieldset__title">An interesting fact about you!</legend> | ||
| <div class="field"> | ||
| <label for="surname">Do you love cats? </label> |
There was a problem hiding this comment.
it's not a surname, think this should be rather cats, names of the inputs below also should be changed :)
| </div> | ||
| </fieldset> | ||
| </form> | ||
| <button type="submit">Submit</button> |
There was a problem hiding this comment.
Move button inside form to make it functional
| > | ||
| </div> | ||
| <div class="field"> | ||
| <label for="name">Password: </label> |
There was a problem hiding this comment.
Add an 'id' attribute with the value 'password' to match the 'for' attribute of the label.
| <fieldset class="form__fieldset"> | ||
| <legend class="form-fieldset__title">Registration</legend> | ||
| <div class="field"> | ||
| <label for="surname">E-mail: </label> |
There was a problem hiding this comment.
Add an 'id' attribute with the value 'email' to match the 'for' attribute of the label.
| <label for="name">Name: </label> | ||
| <input type="text" | ||
| name="firstname" | ||
| minlength="25" |
There was a problem hiding this comment.
The 'minlength' attribute value should be reasonable, reduce it to a smaller value (e.g. 2). we dont need that long name value here
| <fieldset class="form__fieldset form-fieldset"> | ||
| <legend class="form-fieldset__title">Personal information</legend> | ||
| <div class="field"> | ||
| <label for="surname">Surname: </label> |
There was a problem hiding this comment.
"Every field should have label, which focuses input on label click", your labels after click not focusing inputs.
Please add an id with the same value as you have in label for attribute and then it should work :)
Do that for the rest form elements also.
| <legend class="form-fieldset__title">Personal information</legend> | ||
| <div class="field"> | ||
| <label for="surname">Surname: </label> | ||
| <input type="text" |
There was a problem hiding this comment.
Add an 'id' attribute with the value 'surname' to match the 'for' attribute of the label.
| Replace `<your_account>` with your Github username and copy the links to Pull Request description: | ||
| - [DEMO LINK](https://<your_account>.github.io/layout_html-form/) | ||
| - [TEST REPORT LINK](https://<your_account>.github.io/layout_html-form/report/html_report/) | ||
| - [DEMO LINK](https://<natispatis>.github.io/layout_html-form/) |
There was a problem hiding this comment.
Think here you should remove brackets <> otherwise links will not work correctly, same for testing
| <input type="color" name="color"> | ||
| </div> | ||
| <div class="field"> | ||
| <label for="name">What time do you go to bed?</label> |
There was a problem hiding this comment.
Think it's not a label for name, please correct labels for tags in whole file to points correct inputs :)