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

Main #5014

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

Main #5014

wants to merge 4 commits into from

Conversation

Vitalii120296
Copy link

  • 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 Mistakes from BEM lesson theory are checked.
  • Code follows all the Code Style Rules ❗️

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! 😊 While you've made great progress, there are a few critical issues that need to be addressed before we can approve the pull request:

  1. BEM Naming Convention: The class star__stars does not adhere to the BEM naming convention. It should be stars__star to correctly reflect the block and element relationship.

  2. CSS Selector Specificity: The space before :nth-child in selectors like .stars--1 :nth-child(1) is causing incorrect specificity. This space should be removed to ensure the pseudo-class applies directly to the intended children.

  3. File Organization: Each BEM block should have its own separate file as per the requirements. This helps in maintaining a clean and organized codebase.

  4. Use of Pseudo-Selectors: The task requires the use of container modifiers and pseudo-selectors to manage the styling of yellow stars without additional classes. This aspect needs more attention in your implementation.

Please address these issues to meet the core requirements of the task. Once resolved, your solution will be in great shape! Keep up the good work, and don't hesitate to reach out if you need any help. 🌟

✨ 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/style.css Outdated
Comment on lines 18 to 22
.stars--1 :nth-child(1),
.stars--2 :nth-child(-n + 2),
.stars--3 :nth-child(-n + 3),
.stars--4 :nth-child(-n + 4),
.stars--5 :nth-child(-n + 5) {

Choose a reason for hiding this comment

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

The selectors for .stars--1 :nth-child(1), .stars--2 :nth-child(-n + 2), etc., have a space before :nth-child, which increases specificity and may cause unexpected behavior. Consider removing the space to ensure the pseudo-class applies directly to the children of the .stars--X elements.

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.

Great job on your submission! 🎉 Your code meets the majority of the task requirements and constraints. Here are some key points from the review:

  • HTML Structure: Your HTML is well-formed, using semantic tags appropriately, and follows the indentation rules.
  • CSS Practices: You've used Flexbox effectively for the star rating system and adhered to the task's requirements, such as using background-image for stars and avoiding the gap property.
  • BEM Methodology: Each BEM block has its own CSS file, which is correctly implemented.

However, please ensure that vertical margins between blocks are not added, as this was not explicitly checked. While there are no critical issues, addressing this will help you refine your skills further.

Remember, the decision to approve is to help you move forward with your learning. Please review the comments to improve your code, and keep up the great work! 🚀

✨ 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