Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on implementing the BEM structure and the logic for the star ratings! However, a few changes are needed to meet all the project requirements.
The primary issue is that the automated build is failing due to visual mismatches. The tests indicate that the size and spacing of the stars are incorrect. Please review the Figma design again and update the width, height, and margin-right properties for the .stars__star element in src/style.css.
Additionally, there are two structural points to address:
- The task requires each BEM block to have its own CSS file. Please move the styles for the
starsblock into a separate file. - The
class="body"insrc/index.htmlis unnecessary and violates the project's 'no extra classes' rule. You should remove it and style thebodytag directly.
Addressing these points will resolve the test failures and complete the task. You're very close!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/index.html
Outdated
| </head> | ||
| <body> | ||
| <h1>Stars</h1> | ||
| <body class="body"> |
There was a problem hiding this comment.
The body class is not necessary here. According to the task requirements, you should style the body tag directly in your CSS file. The task also specifies not to add any classes other than those required for the BEM block.
src/style.css
Outdated
| .stars { | ||
| display: flex; | ||
| padding: 0; | ||
| } |
There was a problem hiding this comment.
This violates a checklist item: 'Each BEM block has its own separate file'. According to BEM principles, styles for a specific block like stars should be in their own dedicated file (e.g., stars.css), separate from global styles like the body reset.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job addressing the structural feedback from the previous review! You've correctly separated the BEM block styles into their own file and cleaned up the HTML.
However, this requires changes because the core visual requirement is not yet met, which is causing the automated tests to fail. The size (width, height) and spacing (margin-right) of the stars in src/stars.css do not match the Figma design.
Please open the Figma file again, use the measure tool to get the exact dimensions, and update your CSS accordingly. Once the visual appearance matches the design, the tests will pass.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational exception to help you move forward. You've done an excellent job with the HTML structure and correctly separating the CSS into its own files. The BEM implementation is also very well done!
However, the critical issue from the previous review regarding the star dimensions in src/stars.css has not been addressed. The star width and height should be 24px, and the spacing between them should be 8px to match the Figma design. Please update these values in your .stars__star style rules. This is the final change needed to perfect your solution. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| width: 16px; | ||
| height: 16px; |
There was a problem hiding this comment.
The width and height of the stars should match the dimensions in the Figma file. According to the design, each star should be 24px by 24px.
| } | ||
|
|
||
| .stars__star:not(:last-child) { | ||
| margin-right: 4px; |
There was a problem hiding this comment.
The spacing between the stars is also defined in the Figma design. Please measure the distance between them again; it should be 8px.
2 participants
DEMO LINK
TEST REPORT LINK
[x ] Yellow stars are added with container modifier + pseudo-selector (NO extra classes)
[x ] Each BEM block has its own separate file
[x ] All
Typical MistakesfromBEMlesson theory are checked.[x ] Code follows all the Code Style Rules ❗️