add task solution product card #5132
Conversation
vsolodkyi-mate
commented
Jan 1, 2025
vsolodkyi-mate
commented
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Your code is on the right track, but there are a few critical issues that need to be addressed to fully meet the task requirements. 🚀
- BEM Blocks: Separate the
cardandstarsBEM blocks into their own SCSS files as required. - SCSS Variables: Move the SCSS variables to a separate file for better organization.
- SCSS Nesting: Implement SCSS nesting for elements, modifiers, and pseudo-classes to align with the task requirements.
- Stars Modifiers: Use BEM modifiers like
stars--Nto ensure all star modifiers work as expected.
These changes will help improve the structure and maintainability of your code. Keep up the great work, and I'm confident you'll nail it with these adjustments! 💪
✨ 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.
There was a problem hiding this comment.
Great job on addressing the previous issues and meeting the task requirements! 🎉 Your code is well-organized, and you've effectively used SCSS features. However, there are a few points to consider for future improvements:
- BEM Naming Conventions: Ensure that classes like
allstarsfollow BEM conventions. Consider renaming it to something likecard__allstarsfor consistency. - SCSS Nesting: The class
allstars__textshould be nested under the correct BEM block. Make sure the SCSS reflects this structure. - SCSS Structure: Verify that
index.scssis organized with separate files for different BEM blocks.
These adjustments will enhance the maintainability and readability of your code. 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! ✨
| APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
| </h1> | ||
| <p class="card__text">Product code: 195434</p> | ||
| <div class="allstars"> |
There was a problem hiding this comment.
The class allstars does not follow the BEM naming convention. Consider renaming it to something like card__allstars to maintain consistency with the BEM methodology.
| <div class="stars__star stars__star--active"></div> | ||
| <div class="stars__star stars__star--active"></div> | ||
| </div> | ||
| <p class="allstars__text">Reviews: 5</p> |
There was a problem hiding this comment.
The class allstars__text should be nested under the allstars block if following BEM conventions. Ensure that the SCSS reflects this structure.
| @@ -11,8 +11,57 @@ | |||
| rel="stylesheet" | |||
| href="./styles/index.scss" | |||
There was a problem hiding this comment.
Ensure that the SCSS file index.scss is correctly structured and organized as per the task requirements, with separate files for different BEM blocks.
| @@ -0,0 +1,40 @@ | |||
| .allstars { | |||
There was a problem hiding this comment.
The class allstars does not follow the BEM naming convention. Consider renaming it to something like stars__container to maintain consistency with the BEM methodology.
| margin: 16px 0 0; | ||
| display: flex; | ||
|
|
||
| .stars { |
There was a problem hiding this comment.
The class stars should be a block, not an element under allstars. Ensure that the BEM structure is correctly implemented.
| } | ||
| } | ||
|
|
||
| &__text { |
There was a problem hiding this comment.
The class allstars__text should be nested under the correct BEM block. Ensure that the SCSS reflects this structure.
