-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add task solution #5513
base: master
Are you sure you want to change the base?
add task solution #5513
Conversation
martunyarmoshevych
commented
Mar 20, 2025
- DEMO LINK
- TEST REPORT LINK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Hi! Thanks for your submission. I've reviewed your code and found a few things that need to be addressed:
- File Structure: The checklist specifies that each BEM block should reside in its own separate file. Currently, all styles are located in
src/style.css
. Please refactor your CSS to adhere to this requirement. - CSS Image Paths: In
src/style.css
, thebackground-image
paths are absolute but should be relative for portability. - Code Style Rules: The HTML code in
src/index.html
does not follow the code style rules, specifically regarding indentation and attribute formatting. - HTML Formatting: There are missing empty lines between multiline sibling blocks of HTML in
src/index.html
.
Please address these points and resubmit your pull request. Keep up the great work! 🚀
✨ 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
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the checklist, you should add empty lines between multiline sibling blocks of HTML to improve readability. Add an empty line here.
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the checklist, you should add empty lines between multiline sibling blocks of HTML to improve readability. Add an empty line here.
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the checklist, you should add empty lines between multiline sibling blocks of HTML to improve readability. Add an empty line here.
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the checklist, you should add empty lines between multiline sibling blocks of HTML to improve readability. Add an empty line here.
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the checklist, you should add empty lines between multiline sibling blocks of HTML to improve readability. Add an empty line here.
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
<div class="stars__star"></div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to checklist, each BEM block has its own separate file. Consider moving each block to a separate file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 identified issues and recommendations for improvement:
-
File Structure:
- The checklist requires that "Each BEM block has its own separate file". Currently, the
stars
block styles are located insrc/stars.css
, but this file is imported intosrc/style.css
. - Action: Move the contents of
src/stars.css
directly intosrc/style.css
and remove the import statement and thesrc/stars.css
file to adhere to the BEM principle of separate files for each block. - Action: Ensure that all import paths are relative.
- The checklist requires that "Each BEM block has its own separate file". Currently, the
-
Code Style Rules:
- The HTML code in
src/index.html
does not follow the code style rules, specifically regarding indentation and attribute formatting. - Action: Review the
HTML Formatting
section in thechecklist
file and fix the formatting issues insrc/index.html
.
- The HTML code in
-
HTML Formatting:
- There are missing empty lines between multiline sibling blocks of HTML in
src/index.html
. - Action: Review the
HTML Formatting
section in thechecklist
file and add the missing empty lines insrc/index.html
to improve readability.
- There are missing empty lines between multiline sibling blocks of HTML in
These points are crucial for maintaining a clean, organized, and scalable codebase. Addressing them will significantly improve the quality of your project. Keep up the good work, and don't hesitate to ask if you have any questions!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
@@ -1 +1 @@ | |||
/* add styles here */ | |||
@import '/src/stars.css'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The checklist requires that each BEM block should reside in its own separate file.
Also, the path should be relative.