- 
                Notifications
    You must be signed in to change notification settings 
- Fork 611
Develop #789
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 #789
Conversation
         Patryk-Buczkowski
  
      
      
      commented
      
            Patryk-Buczkowski
  
      
      
      commented
        May 11, 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.
Adjust according to my comments and the checklist
        
          
                src/index.html
              
                Outdated
          
        
      | method="post" | ||
| action="https://mate-academy-form-lesson.herokuapp.com/create-application" | ||
| > | ||
|  | 
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.
There shouldn't be empty line between parent and first child
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.
You should have empty lines only between siblings on the same level, not between parent and child, here's an example
<section>
  <div>
    <span>First Child of Div</span>
    <span>Second Child of Div</span>
  <div>
</section>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.
An exemption to that rule would be an ul
<ul>
  <li></li>
  <li></li>
  <li></li>
  <li></li>
</ul>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 the rest of your markup doesn't adhere to these rules so adjust accordingly, also take a look at the checklist and also adhere to it's rules
        
          
                src/index.html
              
                Outdated
          
        
      | </head> | ||
| <body> | ||
| <h1>HTML Form</h1> | ||
| <script type="text/javascript" src="./main.js"></script> | 
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.
This script should be at the bottom of the markup to work move it below your form
|  | ||
| <button type="submit">Submit</button> | ||
| </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.
Remove those 2 empty lines
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 minor changes needed :)
        
          
                src/index.html
              
                Outdated
          
        
      | <label for="surname">Surname:</label> | ||
|  | ||
| <input | ||
| id="surname" | ||
| type="text" | ||
| name="surname" | ||
| autocomplete="off" | ||
| class="input" | ||
| > | ||
|  | ||
| <br> | 
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 
 tags, you can achive this by grouping form-fiels into block element like this:
| <label for="surname">Surname:</label> | |
| <input | |
| id="surname" | |
| type="text" | |
| name="surname" | |
| autocomplete="off" | |
| class="input" | |
| > | |
| <br> | |
| <div class="form-field"> | |
| <label for="name">Name:</label> | |
| <input | |
| type="text" | |
| name="name" | |
| id="name" | |
| autocomplete="off" | |
| required | |
| > | |
| </div> | 
When you wrap label and input into div, it will take 100% width and make newline automatically for next elements. Wrapp all fields like this and remove 
 :)
        
          
                src/style.css
              
                Outdated
          
        
      | .input { | ||
| margin-bottom: 10px; | ||
| display: inline-block; | ||
| } | 
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.
And you can easly style it like this:
| .input { | |
| margin-bottom: 10px; | |
| display: inline-block; | |
| } | |
| .form-field { | |
| margin-bottom: 10px; | |
| } | 
        
          
                src/index.html
              
                Outdated
          
        
      | <label for="car" class="input"> | ||
| What are your favorite brand of cars? | 
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.
Close label tag here:
| <label for="car" class="input"> | |
| What are your favorite brand of cars? | |
| <label for="car" class="input"> | |
| What are your favorite brand of cars? | |
| </label> | 
        
          
                src/index.html
              
                Outdated
          
        
      | <select | ||
| id="car" | ||
| name="carBrand" | ||
| 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.
Fix formating:
| <select | |
| id="car" | |
| name="carBrand" | |
| multiple | |
| > | |
| <select | |
| id="car" | |
| name="carBrand" | |
| multiple | |
| > | 
        
          
                src/index.html
              
                Outdated
          
        
      | <label for="comments" class="input"> | ||
| Comments: | 
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.
| <label for="comments" class="input"> | |
| Comments: | |
| <label for="comments" class="input"> | |
| Comments: | |
| </label> | 
        
          
                src/index.html
              
                Outdated
          
        
      | <label for="recommend" >Would you recommend us?</label> | ||
| <select | ||
| id="recommend" | ||
| name="recommend" | ||
| > | 
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
| <label for="recommend" >Would you recommend us?</label> | |
| <select | |
| id="recommend" | |
| name="recommend" | |
| > | |
| <label for="recommend" >Would you recommend us?</label> | |
| <select | |
| id="recommend" | |
| name="recommend" | |
| > | 
| </fieldset> | ||
| <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.
| </fieldset> | |
| <button type="submit">Submit</button> | |
| </fieldset> | |
| <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.
Looks nice 🥇 ! I give approve but please correct the indentation in the code according to the example given :)
| <fieldset class="fieldset"> | ||
| <legend>Personal information</legend> | ||
| <div class="form-field"> | ||
|  | 
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.
Like Radek mentioned before there should not be an empty line.
"You should have empty lines only between siblings on the same level, not between parent and child, here's an example"
<section>
  <div>
    <span>First Child of Div</span>
    <span>Second Child of Div</span>
  <div>
</section>
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 done ;)