add task solution #5506
add task solution #5506
Conversation
There was a problem hiding this comment.
Hey! 👋 There are a couple of things we need to address before merging. First, could you please ensure all image paths in your CSS are relative (like ./images/star.svg) for consistency? I noticed some use absolute paths, and relative paths are generally better for portability. Second, let's avoid using tag names for styling (except for html and body). Use classes instead—it's a cleaner and more maintainable approach. Also double check your HTML formatting for correct indentation. Keep up the great work! ✨
✨ 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.
🤖 If you have any comments about this review, please leave a message in the Add a comment field below and mention me with @mateacademy-ai-mentor tag. I will read your message when I perform the next review of your pull request.
src/style.css
Outdated
| html, | ||
| body, | ||
| div { |
There was a problem hiding this comment.
As per the checklist and task requirements, avoid using tag names for styling except for html and body. Use classes instead.
src/style.css
Outdated
| } | ||
|
|
||
| .stars--1 .stars__star:nth-child(-n + 1) { | ||
| background-image: url('/src/images/star-active.svg'); |
There was a problem hiding this comment.
The path /src/images/star-active.svg looks like an absolute path. For consistency and to avoid potential issues, use relative paths like ./images/star-active.svg as used in other rules.
| } | ||
|
|
||
| .stars--2 .stars__star:nth-child(-n + 2) { | ||
| background-image: url('./images/star-active.svg'); |
There was a problem hiding this comment.
It's better to use star-active.svg for consistency.
| } | ||
|
|
||
| .stars--3 .stars__star:nth-child(-n + 3) { | ||
| background-image: url('./images/star-active.svg'); |
There was a problem hiding this comment.
It's better to use star-active.svg for consistency.
| } | ||
|
|
||
| .stars--4 .stars__star:nth-child(-n + 4) { | ||
| background-image: url('./images/star-active.svg'); |
There was a problem hiding this comment.
It's better to use star-active.svg for consistency.
| } | ||
|
|
||
| .stars--5 .stars__star:nth-child(-n + 5) { | ||
| background-image: url('./images/star-active.svg'); |
There was a problem hiding this comment.
It's better to use star-active.svg for consistency.
There was a problem hiding this comment.
Great job on completing the task! 🎉 The HTML structure and CSS styling for the stars block look good and generally follow the requirements.
However, there are a couple of things to keep in mind for future improvements, based on the review:
- In
src/style.css, avoid styling HTML tag names likehtmlandbodydirectly. Instead, use classes to style elements. This promotes better reusability and avoids unintended side effects. - In
src/style.css, each BEM block should ideally have its own separate file for better organization and maintainability. While not critical for this task, it's a good practice to follow for larger projects.
The decision to approve is to help you move forward with your learning and development. Please review the comments to improve your code and address the mentioned issues in future tasks. Keep up the good work! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
DEMO LINK
TEST REPORT LINK
[х] Yellow stars are added with container modifier + pseudo-selector (NO extra classes)
[х] Each BEM block has its own separate file
[х] All
Typical MistakesfromBEMlesson theory are checked.[х] Code follows all the Code Style Rules ❗️