Skip to content
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

Develop #4997

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Develop #4997

wants to merge 5 commits into from

Conversation

PedroWeb10
Copy link

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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! 😊 There is a critical issue with the HTML structure due to a duplicate <body> tag, which can cause rendering problems in browsers. Please remove the extra <body> tag to ensure proper functionality. Additionally, make sure each BEM block has its own CSS file as required. Once these changes are made, your solution will be ready for approval. 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.

src/index.html Outdated Show resolved Hide resolved
@PedroWeb10
Copy link
Author

DEMO LINK
TEST REPORT LINK

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your implementation has been reviewed and I'm happy to say it meets the task requirements and constraints. 🎉

Review Summary:

  • HTML Structure: Well-formed and consistent. Each stars div contains five stars__star spans, and there are no critical issues.
  • CSS Styling: Correctly applies styles using nth-child selectors for active stars. The use of background-image is appropriate, and the layout is managed with display: flex without using the gap property.
  • BEM Methodology: Adhered to without additional classes beyond those specified.
  • Style Guidelines: Followed correctly, avoiding the universal selector * and tag name styling except for html and body.

Your code is well-structured and meets all specified requirements. There are no missing or unrelated files, and the implementation is correct.

Keep up the great work! Remember to review any comments for potential improvements and learning opportunities. If there are any critical issues, ensure they are addressed in future projects to enhance your coding skills. 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants