Develop #5521
Develop #5521
Conversation
There was a problem hiding this comment.
Review Summary
Hey! 👋 There are a few things we need to address before merging this in. In src/index.html, please ensure you're using 2 spaces for indentation and add an empty line between multiline sibling blocks. Also, in src/style.css, avoid using tag names like body for styling, as this goes against our style rules. Plus, remember each BEM block should ideally have its own separate file. Let's get these sorted! 🚀
✨ 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
There was a problem hiding this comment.
Review Summary
Okay, the review is complete. I'm approving this pull request to help you move forward, but please carefully review the comments below to improve your code!
Here's a summary of the key issues to address:
- CSS Structure: The checklist specifies that each BEM block should ideally have its own separate file. Consider refactoring your CSS to adhere to this structure, separating styles for different blocks into their respective files. This will improve maintainability and organization.
- Tag Name Styling: Styling the
.pageselector (which is implicitly thebodytag) violates the rule against using tag names for styling (excepthtmlandbody). As noted in the checklist, use.classesand, if needed, with:pseudo-classes,pseudo-elements, and[attributes]instead.
These issues are important to address to align with the project's guidelines. Keep up the good work, and don't hesitate to ask if you have any questions as you work on these improvements! You are doing great, keep up the good work! 👍
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
src/style.css
Outdated
| .page{ | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
Styling .page selector (which is the body tag) violates the rule against using tag names for styling. Please, use .classes and if needed with :pseudo-classes, pseudo-elements and [attributes] instead. See the checklist for details.
| .stars--4 .stars__star:nth-child(-n + 4), | ||
| .stars--5 .stars__star:nth-child(-n + 5) { | ||
| background-image: url(./images/star-active.svg); | ||
| } |
There was a problem hiding this comment.
Each BEM block should ideally have its own separate file. Consider refactoring your CSS to adhere to this structure, separating styles for different blocks into their respective files as per the checklist.
|
Please review again the Backstop report is correct now |
Typical MistakesfromBEMlesson theory are checked.