Create popup #201
Create popup #201
Conversation
|
Hey! Congratulations with your PR! 😎😎😎 Please, be sure you haven't followed common mistakes. Also, be aware, that if you would silently ignore this recommendation, a mentor may think that you are still working on fixes. And your PR will not be reviewed. 😒 |
| <img src="img/popup-button.png" alt="popup-button"> | ||
| </label> | ||
| <ul class="popup"> | ||
| <li><a href="#"><img src="img/calendar.png" alt="calendar"><p>Calendar</p></a></li> |
There was a problem hiding this comment.
For icons it's better set an empty alt attribute, read more here
| <li class="more"> | ||
| <input id="checkbox_more" class="checkbox_more" type="checkbox"> | ||
| <label for="checkbox_more"> | ||
| <span>More</span> |
There was a problem hiding this comment.
When More button is clicked, it is floats left for some reason, please, check.
Anyway, you don't need to show it after clicking
There was a problem hiding this comment.
- Add
:focusstate - You don't need to show it after clicking
| position: relative; | ||
| } | ||
|
|
||
| .menu > li{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
It is related to all tags styling, use class names instead
There was a problem hiding this comment.
You don't fixed it. Please, replace ALL the tags by class names in css file.
| } | ||
|
|
||
| .popup img, .popup_more img{ | ||
| max-width: 45px!important; |
There was a problem hiding this comment.
Why do you need important here? Try to avoid it, it is anti-pattern read more
There was a problem hiding this comment.
You don't fixed, please, check all css file and avoid !important
|
Changed all points according to your comments |
| <img src="img/popup-button.png" alt=""> | ||
| </label> | ||
| <ul class="popup"> | ||
| <li class="popup_li"><a href="#"><img src="img/calendar.png" alt="calendar"><p>Calendar</p></a></li> |
There was a problem hiding this comment.
For icons it's better set an empty alt attribute, read more here
| box-shadow: 0 2px 8px rgba(0, 0, 0, .25); | ||
| } | ||
|
|
||
| .popup a, .popup_more label{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| line-height: 16px; | ||
| } | ||
|
|
||
| .popup img, .popup_more img{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| margin: 10px auto 0; | ||
| } | ||
|
|
||
| .popup p, .popup_more p{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| margin-bottom: 10px; | ||
| } | ||
|
|
||
| .popup_li > a{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| display: flex; | ||
| } | ||
|
|
||
| .checkbox:checked ~ label{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| height: 35px; | ||
| } | ||
|
|
||
| .menu_li > label{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| border: 2px solid #a6c8ff; | ||
| } | ||
|
|
||
| .menu_li > a{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
| border: 2px solid #a6c8ff; | ||
| } | ||
|
|
||
| .menu_li > a:hover:focus { |
There was a problem hiding this comment.
Please, avoid styling by tags link
| height: 35px; | ||
| } | ||
|
|
||
| .menu_li > a:hover{ |
There was a problem hiding this comment.
Please, avoid styling by tags link
|
Corrected all your comments |
task name
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.