Hooli-style Popup #355
Hooli-style Popup #355
Conversation
|
Hey! Congratulations on your PR! 😎😎😎 Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code. Go through the requirements/most common mistakes listed/linked below and fix the code as appropriate. If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat. When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly. Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒 Please, make sure you haven't made common mistakes. Universal recommendations:
Also take a note of the requirements above and follow them in all your future projects. By the way, you may proceed to the next task before this one is reviewed and merged. Sincerely yours, |
|
I went through the code again and completed the self test. There are no fixes for me. I really appreciate your time. I would be glad to receive information about my mistakes in order to avoid them. |
|
|
||
| <header class="header"> | ||
| <div class="header_logo"> | ||
| <img src="./img/logo_u.png" alt="logo"> |
There was a problem hiding this comment.
For icons it's better set an empty alt attribute, read more here
| <nav class="menu"> | ||
| <a href="#">Mail</a> | ||
| <a href="#">Images</a> |
There was a problem hiding this comment.
Please, use ul>li>a fot lists of links
| </span> | ||
| </label> | ||
|
|
||
| <div class="popup_menu"> |
There was a problem hiding this comment.
Please, use ul>li>a fot lists of links here too
| animation-duration: 4s; | ||
| animation-name: text; | ||
| } | ||
| .sun, .green { |
There was a problem hiding this comment.
| .sun, .green { | |
| .sun, | |
| .green { |
Please, write each class name from new line
| padding-bottom: 20px; | ||
| font-size: 1.1em; | ||
| } | ||
| a{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
use class names instead
| font-size: 0.8em; | ||
| text-align: center; | ||
| } | ||
| .header_menu_input:checked ~ .popup_menu{ |
There was a problem hiding this comment.
Add padding: 10px; to make it more pretty
|
I made changes. Pushed files according to instructions. How to update a pull request? I don't quite understand yet... |
|
You updated it by commiting and pushing changes 🙂 |
Hooli-style Popup
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.