card #5261
card #5261
Conversation
There was a problem hiding this comment.
Your code is on the right track, but there are a few important changes needed to fully meet the task requirements. Please move the SCSS variables to a separate file, create individual SCSS files for the card and stars BEM blocks, and ensure your code adheres to the specified code style rules. These adjustments will help improve the maintainability and organization of your project. Keep up the great work, and I'm confident you'll nail it with these tweaks! 😊
✨ 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.
Review Summary
Great job on your progress so far! 🎉 Your HTML structure is well-organized, and your use of BEM naming conventions in SCSS is effective. However, there are a few areas to improve:
-
SCSS Variables File: Please move the SCSS variables from the
index.scssfile to a dedicated variables file. This will enhance maintainability. -
Separate SCSS Files for BEM Blocks: Create individual SCSS files for the
cardandstarsBEM blocks instead of defining them inindex.scss. This will help in organizing your styles better. -
Code Style Rules: Ensure adherence to all specified code style rules, including proper indentation and naming conventions.
These improvements are crucial for maintaining clean and organized code. Addressing these issues will also 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! ✨
src/styles/index.scss
Outdated
| :root { | ||
| --main-black: #060b35; | ||
| --main-grey: #616070; | ||
| } |
There was a problem hiding this comment.
Consider moving these SCSS variables to a separate file to improve maintainability and organization, as suggested in the previous review.
src/styles/index.scss
Outdated
| .card { | ||
| width: 200px; | ||
| margin: 0 auto; | ||
| background-color: #fff; | ||
| padding: 32px 16px 16px; | ||
| border-radius: 5px; | ||
| border: 1px solid #f3f3f3; | ||
|
|
||
| &__image { | ||
| display: block; | ||
| width: 160px; | ||
| height: 134px; | ||
| margin-bottom: 40px; | ||
| margin-right: auto; | ||
| margin-left: auto; | ||
| } | ||
|
|
||
| &__title { | ||
| font-weight: 500; | ||
| font-size: 12px; | ||
| line-height: 18px; | ||
| margin-bottom: 4px; | ||
| } | ||
|
|
||
| &__code { | ||
| font-weight: 400; | ||
| font-size: 10px; | ||
| line-height: 14px; | ||
| color: var(--main-grey); | ||
| margin-bottom: 16px; | ||
| } | ||
|
|
||
| &__rate { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| margin-bottom: 24px; | ||
|
|
||
| .review { | ||
| padding-top: 2px; | ||
| font-weight: 400; | ||
| font-size: 10px; | ||
| line-height: 14px; | ||
| text-align: right; | ||
| display: flex; | ||
| align-items: center; | ||
| align-self: center; | ||
| } | ||
| } | ||
|
|
||
| &__price { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| font-weight: 400; | ||
| font-size: 12px; | ||
| line-height: 18px; | ||
| color: var(--main-grey); | ||
| margin-bottom: 16px; | ||
|
|
||
| & span { | ||
| font-weight: 700; | ||
| font-size: 16px; | ||
| line-height: 18px; | ||
| color: var(--main-black); | ||
| } | ||
| } | ||
|
|
||
| &__btn { | ||
| display: block; | ||
| height: 40px; | ||
| font-weight: 700; | ||
| font-size: 14px; | ||
| line-height: 16px; | ||
| padding: 12px 70px 12px 69px; | ||
| background-color: #00acdc; | ||
| color: #fff; | ||
| border-radius: 5px; | ||
| text-transform: uppercase; | ||
| box-sizing: border-box; | ||
| transition: none; | ||
|
|
||
| &:hover { | ||
| border: 1px solid #00acdc; | ||
| background-color: #fff; | ||
| color: #00acdc; | ||
| height: 40px; | ||
| padding: 11px 69px 11px 68px; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
It's recommended to create a separate SCSS file for the card BEM block to adhere to the task requirements and improve code organization.
src/styles/index.scss
Outdated
| .stars { | ||
| display: flex; | ||
| align-items: center; | ||
| &__star { | ||
| background-image: url(../images/star.svg); | ||
| background-repeat: no-repeat; | ||
| background-position: center; | ||
| width: 16px; | ||
| height: 16px; | ||
| margin-right: 4px; | ||
| } | ||
|
|
||
| @for $i from 1 through 5 { | ||
| &--#{$i} :nth-child(-n + #{$i}) { | ||
| background-image: url(../images/star-active.svg); | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider creating a separate SCSS file for the stars BEM block to enhance maintainability and adhere to the task requirements.
DEMO LINK
TEST REPORT LINK
there 2 BEM blocks
cardandstarseach in their own fileSCSS Nesting is used for
elements,modifiersandpseudo-classesSCSS Variables are used for main values and placed in a separate file
all
stars--Nmodifiers work as expected (Highlight firstNstars)Code follows all the Code Style Rules ❗️