-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 #5384
base: master
Are you sure you want to change the base?
add task solution #5384
Conversation
bogdank0
commented
Feb 10, 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.
Thank you for your hard work on this project! 😊 While your solution meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
CSS Selector for Last Child: The current CSS rule targets the last
.stars
block instead of the last.stars__star
within each block. This needs to be corrected to ensure proper styling. -
File Structure: Each BEM block should have its own separate CSS file. Currently, all styles are in a single
style.css
file, which does not meet the task requirements. -
Missing Required Files: The task specifies using images from an
images
folder, but this folder and its contents are missing. Please ensure that the necessary images (star.svg
andstar-active.svg
) are included. -
HTML Formatting: Ensure that your HTML formatting follows the checklist requirements, such as using 2 spaces for indentation.
Once these issues are addressed, your solution will be much stronger and ready for approval. Keep up the great work, and don't hesitate to reach out if you have any questions! 🌟
✨ 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.
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.
Great job on implementing the star rating component using the BEM methodology! 🎉 While the Pull Request is approved to help you move forward with your learning, there are a few critical issues that need attention:
-
CSS Selector for Last Child: Ensure the CSS selector
.stars .stars__star:last-child
is applied correctly in your HTML structure to remove the right margin from the last star in each block. -
File Structure: Consider removing the
style.css
file if it only serves to importstars.css
and is not needed for additional styles. This will help maintain a clean file structure. -
Missing Images: The
images
folder is referenced in your CSS for background images, but the actual images (star.svg
andstar-active.svg
) are missing. Make sure to include these images as required by the task. -
File Paths: Double-check the file paths in your import statements and link tags to ensure they are correct relative to your project's directory structure.
Addressing these issues will enhance the functionality and maintainability of your code. Keep up the good work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
background-repeat: no-repeat; | ||
} | ||
|
||
.stars .stars__star:last-child { |
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 selector .stars .stars__star:last-child
targets the last .stars__star
within each .stars
block, which is correct according to the task requirements. However, ensure that this selector is applied correctly in the context of your HTML structure.
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.
Explain why it is not correct? If it correctly indents the last star
src/index.html
Outdated
@@ -9,10 +9,51 @@ | |||
<title>Stars</title> | |||
<link | |||
rel="stylesheet" | |||
href="./style.css" | |||
href="style.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 href attribute in the link tag is pointing to 'style.css'. Ensure that this path is correct relative to the HTML file's location. If the CSS files are organized in a 'css' directory, update the path to 'css/stars.css'.
src/style.css
Outdated
@@ -1 +1,5 @@ | |||
/* add styles here */ | |||
@import '../src/css/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 import statement for 'stars.css' uses a relative path. Ensure that this path is correct based on your project's directory structure. If the CSS files are in a different directory, adjust the path accordingly.