add task solution #5098
add task solution #5098
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 However, there are a few critical issues that need to be addressed to meet the task requirements. Specifically, SCSS variables should be placed in a separate file, and BEM blocks need to be separated into individual files. Additionally, the background image URL should be relative to index.scss, and proper SCSS nesting should be used for elements, modifiers, and pseudo-classes. Once these are resolved, your code will be in great shape! Keep up the great effort! 💪
✨ 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 most of the previous issues! 🎉 Your project is well-structured, and you've made good use of SCSS and BEM conventions. Here are a few key points to consider for further improvement:
-
Image Paths: Ensure that the image paths in your
stars.scssfile are relative and accessible. For instance, the background image URL should start with../imagesinstead of/src/images. -
SCSS Variables: Verify that all SCSS variables such as
--main-accent,--font-family,--blue-accent,--secondary,--white, and--gray-elementsare defined in a separatevariables.scssfile and imported correctly. -
SCSS Nesting: Consider using SCSS nesting to group related styles together for better organization and maintainability.
-
BEM Naming: Ensure consistent application of the BEM naming convention across all elements and modifiers.
-
Compilation: Make sure that your SCSS files are correctly compiled to CSS by the Parcel bundler, as browsers do not interpret SCSS directly.
You've made great progress, and this approval is to help you move forward with your learning. Please review the comments to improve your code, and address any critical issues in future projects to avoid similar problems. Keep up the good work and continue to learn from your experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -11,8 +11,57 @@ | |||
| rel="stylesheet" | |||
| href="./styles/index.scss" | |||
There was a problem hiding this comment.
Ensure that the SCSS file is correctly compiled to CSS by the Parcel bundler, as browsers do not interpret SCSS directly.
| <h2 class="card__name"> | ||
| APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A) | ||
| <p class="card__code">Product code: 195434</p> | ||
| </h2> |
There was a problem hiding this comment.
The BEM naming convention is used here. Ensure that all elements and modifiers follow this convention consistently throughout your project.
| .card { | ||
| border: 1px solid var(--gray-elements); | ||
| border-radius: 5px; | ||
| width: 198px; | ||
| height: 406px; | ||
| background-color: var(--white); | ||
| justify-content: center; | ||
| } |
There was a problem hiding this comment.
Consider using SCSS nesting to group related styles together. For example, you can nest the styles for .card__img, .card__name, etc., within the .card block for better organization.
| @@ -0,0 +1,76 @@ | |||
| .card { | |||
| border: 1px solid var(--gray-elements); | |||
There was a problem hiding this comment.
Ensure that the SCSS variable --gray-elements is defined in a separate variables file and imported correctly.
| border-radius: 5px; | ||
| width: 198px; | ||
| height: 406px; | ||
| background-color: var(--white); |
There was a problem hiding this comment.
Ensure that the SCSS variable --white is defined in a separate variables file and imported correctly.
| .stars { | ||
| gap: 4px; | ||
| margin-top: 16px; | ||
| display: flex; | ||
| margin-left: 16px; | ||
| margin-bottom: 24px; | ||
|
|
||
| &__star { | ||
| background-image: url(/src/images/star.svg); | ||
| background-position: 50%; | ||
| background-repeat: no-repeat; | ||
| width: 16px; | ||
| height: 16px; | ||
| } | ||
|
|
||
| &__reviews { | ||
| font-family: var(--font-family); | ||
| margin-top: 1.7px; | ||
| margin-left: 18px; | ||
| margin-right: 16px; | ||
| font-weight: 400; | ||
| font-size: 10px; | ||
| line-height: 140%; | ||
| color: var(--main-accent); | ||
| text-align: right; | ||
| } | ||
| } |
There was a problem hiding this comment.
The use of SCSS nesting for the .stars block is appropriate. Ensure that the BEM naming convention is consistently applied across all elements and modifiers.
| margin-bottom: 24px; | ||
|
|
||
| &__star { | ||
| background-image: url(/src/images/star.svg); |
There was a problem hiding this comment.
Verify that the image path /src/images/star.svg is correct and the image is accessible.
| } | ||
|
|
||
| &__reviews { | ||
| font-family: var(--font-family); |
There was a problem hiding this comment.
Ensure that the SCSS variable --font-family is defined in a separate variables file and imported correctly.
| font-weight: 400; | ||
| font-size: 10px; | ||
| line-height: 140%; | ||
| color: var(--main-accent); |
There was a problem hiding this comment.
Ensure that the SCSS variable --main-accent is defined in a separate variables file and imported correctly.
| } | ||
|
|
||
| .stars__star:nth-child(-n + 4) { | ||
| background-image: url(/src/images/star-active.svg); |
There was a problem hiding this comment.
Verify that the image path /src/images/star-active.svg is correct and the image is accessible.
❗️ Copy this
Checklistto thePull Requestdescription after links, and put- [x]before each point after you checked it.cardandstarseach in their own fileelements,modifiersandpseudo-classesstars--Nmodifiers work as expected (Highlight firstNstars)