add task solution #6068
add task solution #6068
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
CSS Class Naming: The class names do not follow the BEM naming convention, which is important for maintaining scalable and organized code. Consider renaming classes like
.nav_ul,.nav_li, and.nav_itemto better reflect their purpose and adhere to BEM principles. -
CSS Styling: Avoid using tag names for styling. Instead, apply styles using classes to ensure consistency and maintainability.
-
HTML Formatting: Ensure that your HTML lines adhere to the 80-character limit for better readability and compliance with the checklist.
-
Placeholder Links: The use of
href="#"as a placeholder should be replaced with actual URLs or handled appropriately if intended for future implementation.
These changes are necessary to ensure the functionality and maintainability of your code. Once these issues are resolved, we can proceed with approving your pull request. Keep up the great work, and don't hesitate to reach out if you have any questions! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/index.html
Outdated
| <header class="header"> | ||
| <a | ||
| class="logo" | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
| <ul class="nav_ul"> | ||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li laptops"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/index.html
Outdated
|
|
||
| <li class="nav_li"> | ||
| <a | ||
| href="#" |
There was a problem hiding this comment.
The href="#" attribute is used as a placeholder. Consider replacing it with an actual URL or handling it appropriately if it's intended for future implementation.
src/style.css
Outdated
|
|
||
| .nav_item { | ||
| display: block; | ||
| align-items: center; |
There was a problem hiding this comment.
The align-items property is not applicable to block elements like .nav_item. Consider removing it or ensuring that .nav_item is a flex container if alignment is needed.
There was a problem hiding this comment.
Review Summary
Great effort on addressing several issues from the previous review! 🎉 However, there are still some critical points that need attention to ensure your code is scalable and maintainable:
-
CSS Class Naming: The class names such as
.nav_ul,.nav_li, and.nav_itemdo not follow the BEM naming convention. Consider renaming them to something likenav__list,nav__item, andnav__linkto adhere to BEM principles. This will help in maintaining organized and scalable code. -
CSS Styling: Avoid using tag names for styling, such as
bodyand.nav_ul. Instead, apply styles using classes to ensure consistency and maintainability. This is crucial for avoiding conflicts and making your CSS more modular. -
HTML Formatting: Ensure that your HTML file adheres to the 80-character limit for lines. This will improve readability and make your code easier to manage.
Despite these issues, the decision to approve your pull request is made to help you move forward with your learning journey. Please take the time to review the comments and improve your code. Addressing these critical issues will help you avoid similar problems in the future. Keep up the good work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| /> | ||
| </a> | ||
| <nav class="nav"> | ||
| <ul class="nav_ul"> |
There was a problem hiding this comment.
The class name 'nav_ul' does not follow the BEM naming convention. Consider renaming it to something like 'nav__list' to adhere to BEM principles.
| </a> | ||
| <nav class="nav"> | ||
| <ul class="nav_ul"> | ||
| <li class="nav_li"> |
There was a problem hiding this comment.
The class name 'nav_li' does not follow the BEM naming convention. Consider renaming it to something like 'nav__item' to adhere to BEM principles.
| <li class="nav_li"> | ||
| <a | ||
| href="https://www.moyo.ua/ua/brands/apple/" | ||
| class="nav_item is-active" |
There was a problem hiding this comment.
The class name 'nav_item' does not follow the BEM naming convention. Consider renaming it to something like 'nav__link' to adhere to BEM principles.
| </a> | ||
| </li> | ||
|
|
||
| <li class="nav_li laptops"> |
There was a problem hiding this comment.
The class name 'nav_li laptops' does not follow the BEM naming convention. Consider renaming it to something like 'nav__item nav__item--laptops' to adhere to BEM principles.
| align-items: center; | ||
| } | ||
|
|
||
| .nav_ul { |
There was a problem hiding this comment.
The class name 'nav_ul' does not follow the BEM naming convention. Consider renaming it to something like 'nav__list' to adhere to BEM principles.
| align-items: center; | ||
| } | ||
|
|
||
| .nav_li:not(:last-child) { |
There was a problem hiding this comment.
The class name 'nav_li' does not follow the BEM naming convention. Consider renaming it to something like 'nav__item' to adhere to BEM principles.
| margin-right: 20px; | ||
| } | ||
|
|
||
| .nav_item { |
There was a problem hiding this comment.
The class name 'nav_item' does not follow the BEM naming convention. Consider renaming it to something like 'nav__link' to adhere to BEM principles.
| bottom: 0; | ||
| } | ||
|
|
||
| .nav_li a:hover { |
There was a problem hiding this comment.
Avoid using tag names for styling. Instead, apply styles using classes to ensure consistency and maintainability.
DEMO LINK
TEST REPORT LINK
Header height is set in 1 place (for the links)
Content is vertically centered (for any header height)
CSS is used to show all letters in Uppercase (don't type them in HTML)
Logo is an image wrapped with a link
CSS Variable is used for a blue color
Pseudo-element is used for a blue line below the active link
Code follows all the Code Style Rules ❗️