first commit #4917
first commit #4917
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 There are a few critical issues that need to be addressed before we can approve the pull request:
-
BEM Naming Convention: The task requires consistent use of BEM naming conventions across all classes. Some classes like
.list--item-5and.item-3do not adhere to this standard, which is crucial for maintainability and scalability. -
Data Attributes: The
data-qaattributes are not correctly implemented. Specifically,data-qa="card-hover"is placed incorrectly, and non-standard quotation marks are used, which can lead to functionality issues. -
Core Requirements: The placement of
data-qaattributes and adherence to BEM are core requirements of the task. Ensuring these are correctly implemented is essential for the functionality and future maintenance of the code.
Please address these issues, and feel free to reach out if you need any help. Once these are resolved, we can proceed with the approval. Keep up the great work! 🚀
✨ 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> | ||
|
|
||
| <main class="main"> | ||
| <section class="product-card" data-qa=«card»> |
There was a problem hiding this comment.
The data attribute data-qa=«card» uses non-standard quotation marks. It should be changed to standard double quotes like data-qa="card".
| </div> | ||
|
|
||
| <div class="content__buy"> | ||
| <a href="#" data-qa=«card-hover»> |
There was a problem hiding this comment.
The data attribute data-qa=«card-hover» uses non-standard quotation marks. It should be changed to standard double quotes like data-qa="card-hover".
|
|
||
| <li class="item item-3"> | ||
| <a class="list" href="#"> | ||
| samsung |
There was a problem hiding this comment.
The product name 'samsung' should be capitalized as 'Samsung' to maintain consistency with other product names.
| .content__price { | ||
| color: $secondary-text-color; | ||
| font-size: 12px; | ||
| line-height: 18px; | ||
| } |
There was a problem hiding this comment.
The .content__price class is defined twice. Consider merging these definitions to avoid redundancy and potential conflicts.
| &:hover { | ||
| background-color: #fff; | ||
| border: 1px solid $button-color; | ||
| color: #00acdc; |
There was a problem hiding this comment.
The color #00acdc is hardcoded. Consider using a variable for this color to maintain consistency and make future changes easier.
| $secondary-text-color: #616070; | ||
| $button-color: #00ACDC; | ||
| $main-page-color: #ffffff; | ||
| $navigation-color: #060b35; |
There was a problem hiding this comment.
The color value for $navigation-color is #060b35, which is the same as $text-color but with different capitalization. Consider using consistent capitalization for hex color codes to maintain uniformity.
| $main-page-color: #ffffff; | ||
| $navigation-color: #060b35; | ||
| $hover-text-color: #34568b; | ||
| $active-navigation: #00acdc; |
There was a problem hiding this comment.
The color value for $active-navigation is #00acdc, which is the same as $button-color but with different capitalization. Consider using consistent capitalization for hex color codes to maintain uniformity.
There was a problem hiding this comment.
Great effort on your submission! 🎉 While your code is on the right track, there are a few areas that need attention to fully meet the task requirements:
-
BEM Naming Convention: Ensure consistent use of BEM across all classes. For example, classes like
.list--number-5and.item--3should be refactored to adhere to BEM standards. -
Data Attributes: The
data-qaattributes need consistency in naming and placement. Specifically, thedata-qa="card-hover"attribute should use standard double quotes and be correctly positioned according to task requirements.
These improvements are crucial for the maintainability and functionality of your code. Addressing these issues will enhance your skills and ensure your code meets the necessary standards. 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> | ||
| </li> | ||
|
|
||
| <li class="item item--3"> |
There was a problem hiding this comment.
The class item--3 does not follow the BEM naming convention. According to BEM, modifiers should be appended to the block or element with a double hyphen, and the base class should be a valid BEM block or element. Consider renaming this class to follow the BEM convention.
| </a> | ||
| </li> | ||
|
|
||
| <li class="item item--5"> |
There was a problem hiding this comment.
The class item--5 does not adhere to the BEM naming convention. Ensure that the base class is a valid BEM block or element and that modifiers are appended correctly.
| </li> | ||
|
|
||
| <li class="item item--5"> | ||
| <a class="list list--number-5" data-qa="nav-hover" href="#"> |
There was a problem hiding this comment.
The data-qa attribute nav-hover is not placed correctly according to the task requirements. Ensure that data-qa attributes are used consistently and correctly across the HTML elements.
| </div> | ||
|
|
||
| <div class="content__buy"> | ||
| <a href="#" data-qa=«card-hover»> |
There was a problem hiding this comment.
The data-qa attribute uses non-standard quotation marks («card-hover»). Replace them with standard double quotes (") to ensure proper functionality.
| </div> | ||
|
|
||
| <div class="content__buy"> | ||
| <a href="#" data-qa="hover"> |
There was a problem hiding this comment.
The data-qa attribute hover is not consistent with other similar attributes. Ensure consistency in naming and usage of data-qa attributes across the document.
| </div> | ||
|
|
||
| <div class="content__buy"> | ||
| <a href="#" data-qa="hover"> |
There was a problem hiding this comment.
The data-qa attribute hover is inconsistent with the naming convention used elsewhere. Ensure all data-qa attributes follow a consistent naming pattern.
| </div> | ||
|
|
||
| <div class="content__buy"> | ||
| <a href="#" data-qa="hover"> |
There was a problem hiding this comment.
The data-qa attribute hover should be consistent with other similar attributes. Ensure uniformity in the naming and application of data-qa attributes.
| </div> | ||
|
|
||
| <div class="content__buy"> | ||
| <a href="#" data-qa="hover"> |
There was a problem hiding this comment.
The data-qa attribute hover is not consistent with other similar attributes. Ensure consistency in naming and usage of data-qa attributes across the document.
| } | ||
| } | ||
|
|
||
| .list--number-5 { |
There was a problem hiding this comment.
The class list--number-5 does not adhere to the BEM naming convention. In BEM, modifiers should be appended to the block or element with a double hyphen, and the base class should be a valid BEM block or element. Consider renaming this class to follow the BEM convention.
| background-image: url("../images/star.svg"); | ||
| } | ||
| } | ||
| .stars--N .stars__star:nth-child(-n+4) { |
There was a problem hiding this comment.
The class stars--N does not adhere to the BEM naming convention. In BEM, modifiers should be appended to the block or element with a double hyphen, and the base class should be a valid BEM block or element. Consider renaming this class to follow the BEM convention.
No description provided.