- 
                Notifications
    You must be signed in to change notification settings 
- Fork 611
add task solution #784
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?
add task solution #784
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.
Good job, some small shortcomings to improve and it should be ok 👍🏽
        
          
                src/style.css
              
                Outdated
          
        
      | margin-bottom: 20px; | ||
| } | ||
|  | ||
| /* Usuń margines dolny dla ostatniego .form-field wewnątrz fieldset */ | 
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.
Please change the comment to English, for better understanding and consistency or remove it
| type="password" | ||
| id="password" | ||
| name="password" | ||
| > | 
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 minlength and maxlength attributes for the password input to define password strength requirements.
        
          
                src/index.html
              
                Outdated
          
        
      | <input | ||
| type="color" | ||
| id="color" | ||
| name="fav-color" | 
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.
if we follow camelCase convention like you named loveCats this should be name="favColor"  also for bedtime & favcars etc.
        
          
                src/style.css
              
                Outdated
          
        
      | margin-bottom: 10px; | ||
| } | ||
|  | ||
| fieldset { | 
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.
don't style elements by tag, rather use some class name here like .form-fieldset
        
          
                src/index.html
              
                Outdated
          
        
      | name="comments" | ||
| id="comments" | ||
| cols="30" | ||
| rows="10" | 
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 you rows value here is to big, it should be rather 3
        
          
                src/index.html
              
                Outdated
          
        
      | id="cars" | ||
| multiple | ||
| > | ||
| <option value="audi">Audi</option> | 
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.
try options input here like:
BMW
Audi
Lada
|  | ||
| <div class="form-field"> | ||
| <label for="bedTime">What time do you go to bed?</label> | ||
| <input | 
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 step="2" attribute to have seconds values
        
          
                src/index.html
              
                Outdated
          
        
      | <legend>Personal information</legend> | ||
|  | ||
| <div class="form-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.
You missed colon for each label in your file, please add : e.g. Surname:
| </div> | ||
| </fieldset> | ||
|  | ||
| <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.
do we need this button to be grouped in this div?
| 
 Thank you, i'll fix it later. | 
| Ok, i think i fixed everything. Can you accept my changes so I can move on? | 
| Should i click button "Close with comment"? | 
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.
Task approved! Nicely done, good job 🥇
| 
 Yes you did it correctly :) | 
No description provided.