- 
                Notifications
    You must be signed in to change notification settings 
- Fork 611
Develop #796
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 #796
Conversation
         natispatis
  
      
      
      commented
      
            natispatis
  
      
      
      commented
        May 30, 2023 
      
    
  
- DEMO LINK
- TEST REPORT LINK
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think it's not a label for name, please correct labels for tags in whole file to points correct inputs :)