Conversation
natispatis
commented
May 30, 2023
natispatis
commented
- DEMO LINK
- TEST REPORT LINK
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
darokrk
left a comment
darokrk
left a comment
There was a problem hiding this comment.
Layout looks good, but code needs some improvements :)
src/index.html
Outdated
| <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 :)
src/index.html
Outdated
| </div> | ||
| </fieldset> | ||
| </form> | ||
| <button type="submit">Submit</button> |
There was a problem hiding this comment.
Move button inside form to make it functional
src/index.html
Outdated
| > | ||
| </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.
src/index.html
Outdated
| <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.
src/index.html
Outdated
| <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.
readme.md
Outdated
| 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
src/index.html
Outdated
| <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 :)
2 participants
