- 
                Notifications
    You must be signed in to change notification settings 
- Fork 611
Develop #795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Develop #795
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting and indentations, check this checklist https://github.com/mate-academy/layout_html-form/blob/master/checklist.md
When the code is more readable after fixes we will continue with review
        
          
                src/index.html
              
                Outdated
          
        
      | <div> | ||
| Surname: <input type="text" name="Surname:"> | ||
| </div> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add proper labels
Fix formatting
Fix indentations
| <div> | |
| Surname: <input type="text" name="Surname:"> | |
| </div> | |
| <div class="form__section_field"> | |
| <label for="surname">Surname: </label> | |
| <input | |
| type="text" | |
| name="surname" | |
| id="surname" | |
| autocomplete="off" | |
| > | |
| </div> | 
Apply this pattern to all other fields
Remember to add spaces between sibling elements for better readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okey, I make fixes please to check again :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few more changes needed
        
          
                src/index.html
              
                Outdated
          
        
      |  | ||
| <input | ||
| type="date" | ||
| name="date-birth" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name attibute should be camelCase
| name="date-birth" | |
| name="dateBirth" | 
        
          
                src/index.html
              
                Outdated
          
        
      |  | ||
|  | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one line space between siblings is needed
        
          
                src/index.html
              
                Outdated
          
        
      |  | ||
|  | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
        
          
                src/index.html
              
                Outdated
          
        
      | <textarea | ||
| name="comment" | ||
| id="comment" | ||
| cols="20" | ||
| rows="2" | ||
| autocomplete="off"> | ||
| </textarea> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
        
          
                src/index.html
              
                Outdated
          
        
      | <select | ||
| name="recommend" | ||
| id="recommend" | ||
| autocomplete="off"> | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting
| <select | |
| name="recommend" | |
| id="recommend" | |
| autocomplete="off"> | |
| <select | |
| name="recommend" | |
| id="recommend" | |
| autocomplete="off" | |
| > | |
        
          
                src/index.html
              
                Outdated
          
        
      | <legend>An interesting fact about you!</legend> | ||
| <div> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add empty line between siblings
        
          
                src/index.html
              
                Outdated
          
        
      | <legend>Registration</legend> | ||
| <div> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add empty line - double check if there are empty lines between siblings elements, do not add empty lines between parent and child
        
          
                src/index.html
              
                Outdated
          
        
      |  | ||
| <input | ||
| type="radio" | ||
| name="choice" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets make this name more descriptive so it is easier to use (in scenartio when form is being send)
| name="choice" | |
| name="lovesCats" | 
        
          
                src/index.html
              
                Outdated
          
        
      | <label for="yes">Yes</label> | ||
|  | ||
| <input type="radio" | ||
| name="choice" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| name="choice" | |
| name="lovesCats" | 
        
          
                src/index.html
              
                Outdated
          
        
      | <form action="https://mate-academy-form-lesson.herokuapp.com/create-application" class="form"> | ||
| <fieldset> | ||
| <legend>Personal information</legend> | ||
| <div> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be consistant and add class to all form fields wrappers. You can then specify the margins using pseudo selectors in css
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tips done I hope now its good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few more for perfection :)
        
          
                src/index.html
              
                Outdated
          
        
      |  | ||
| <input | ||
| type="color" | ||
| name="colour" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be
| name="colour" | |
| name="color" | 
        
          
                src/index.html
              
                Outdated
          
        
      | autocomplete="off" | ||
| multiple | ||
| > | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove space
        
          
                src/index.html
              
                Outdated
          
        
      | > | ||
| </div> | ||
|  | ||
| <div> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion best way is to add form-field class to all divs and in css you can remove margin-bottom in :last-child selector. This is more consistant. It is a separation of duites, that styles will determin how the markup looks.
        
          
                src/index.html
              
                Outdated
          
        
      | <input type="radio" | ||
| name="lovesCats" | ||
| id="no" | ||
| autocomplete="off" | ||
| value="No" | ||
| > | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting
| <input type="radio" | |
| name="lovesCats" | |
| id="no" | |
| autocomplete="off" | |
| value="No" | |
| > | |
| <input | |
| type="radio" | |
| name="lovesCats" | |
| id="no" | |
| autocomplete="off" | |
| value="No" | |
| > | 
        
          
                src/index.html
              
                Outdated
          
        
      | id="recommend" | ||
| autocomplete="off" | ||
| > | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job :)

https://Kacper-Leman.github.io/layout_html-form/ - Demo Link
https://Kacper-Leman.github.io/layout_html-form/report/html_report/ - Test Report Link