add solution (header) #5393
add solution (header) #5393
Conversation
MaxOnRem
commented
Sep 20, 2024
MaxOnRem
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Great job on the Pull Request! 🌟 While there are some minor issues like ensuring proper fallback fonts, avoiding the universal selector for performance, using descriptive class names, and maintaining consistent indentation, none of these are critical blockers. Keep these tips in mind to enhance your code quality for future tasks. Keep up the good work! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/index.html
Outdated
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link is-active" | ||
| > | ||
| apple | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| Samsung | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| smartphones | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| data-qa="hover" | ||
| > | ||
| laptops & computers | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| gadgets | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| tablets | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| photo | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| video | ||
| </a> | ||
| </li> |
There was a problem hiding this comment.
Indentation should be consistent throughout the file. Each level of nesting requires a 2-space offset. Make sure all child elements are indented relative to their parent elements.
src/index.html
Outdated
| <a | ||
| href="#" | ||
| class="logo-link" | ||
| > | ||
| <img | ||
| src="./images/logo.png" | ||
| alt="MOYO" | ||
| /> | ||
| </a> | ||
| <nav> | ||
| <ul class="menu-list"> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link is-active" | ||
| > | ||
| apple | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| Samsung | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| smartphones | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| data-qa="hover" | ||
| > | ||
| laptops & computers | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| gadgets | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| tablets | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| photo | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| video | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </nav> |
There was a problem hiding this comment.
Class names should represent the meaning of the content, not the styles or tag names. For example, 'logo-link' is good, but 'item-link' could be more descriptive, like 'nav-link'.
src/index.html
Outdated
| <a | ||
| href="#" | ||
| class="logo-link" | ||
| > | ||
| <img | ||
| src="./images/logo.png" | ||
| alt="MOYO" | ||
| /> | ||
| </a> | ||
| <nav> | ||
| <ul class="menu-list"> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link is-active" | ||
| > | ||
| apple | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| Samsung | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| smartphones | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| data-qa="hover" | ||
| > | ||
| laptops & computers | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| gadgets | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| tablets | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| photo | ||
| </a> | ||
| </li> | ||
| <li class="list-item"> | ||
| <a | ||
| href="#" | ||
| class="item-link" | ||
| > | ||
| video | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </nav> |
There was a problem hiding this comment.
Avoid using spaces in the href attribute of <a> tags. If these links are placeholders, consider using # without any additional spaces or characters.
