Develop #5088
Develop #5088
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 There are a couple of critical issues that need to be addressed before we can approve the pull request:
-
BEM Block Separation: The task requires that the
cardandstarsBEM blocks be separated into their own SCSS files. Currently, both blocks are defined within the same SCSS file (index.scss). Please separate them into individual files as per the requirement. -
Font-Size Unit: The
font-sizeproperty in the.card__buyclass is missing a unit. Please specify it as '14px', '14rem', etc., to ensure proper rendering across different browsers.
Once these issues are resolved, your solution will be in great shape! Keep up the great work, and I'm looking forward to seeing the updated version. 🚀
✨ 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/styles/index.scss
Outdated
| &__buy { | ||
| box-sizing: border-box; | ||
| display: block; | ||
| margin-top: 16px; | ||
| width: 166px; | ||
| height: $buyButtonHeight; | ||
|
|
||
| line-height: $buyButtonHeight; | ||
| font-size: 14; |
There was a problem hiding this comment.
The font-size property on line 142 is missing a unit. It should be specified as '14px' or '14rem', etc., to ensure proper rendering across different browsers.
There was a problem hiding this comment.
Great job on addressing the previous issues! 🎉 The card and stars BEM blocks are now correctly separated into their own SCSS files, _card.scss and _stars.scss, respectively. The font-size property in the .card__buy class is also correctly specified. All task requirements and constraints have been met, and the code adheres to the specified guidelines.
However, please ensure that the HTML file references the separated SCSS files appropriately, as it currently links to a single index.scss file. This is crucial for maintaining the modular structure of your stylesheets.
Keep up the good work, and remember to review the comments to improve your code further. Addressing these issues will help you avoid similar problems in the future and enhance your learning experience. 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <link | ||
| href="https://fonts.googleapis.com/css2?family=Manrope:[email protected]&family=Roboto:wght@400;500;700&family=Sen:[email protected]&display=swap" | ||
| rel="stylesheet" | ||
| /> | ||
| <link | ||
| rel="stylesheet" | ||
| href="./styles/index.scss" |
There was a problem hiding this comment.
The task requires that the card and stars BEM blocks be separated into their own SCSS files. Currently, the HTML file links to a single index.scss file. Please separate the SCSS files for each BEM block and update the link references accordingly.
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 ❗️