Skip to content

Develop#6876

Open
nastachypak-pixel wants to merge 3 commits into
mate-academy:masterfrom
nastachypak-pixel:develop
Open

Develop#6876
nastachypak-pixel wants to merge 3 commits into
mate-academy:masterfrom
nastachypak-pixel:develop

Conversation

@nastachypak-pixel

Copy link
Copy Markdown
  • DEMO LINK

  • TEST REPORT LINK

  • [x ] Yellow stars are added with container modifier + pseudo-selector (NO extra classes)

  • Each BEM block has its own separate file

  • [x ] All Typical Mistakes from BEM lesson theory are checked.

  • [x ] Code follows all the Code Style Rules ❗️

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because there are still a couple of critical checklist and requirements issues that can block automated tests. First, [CHECKLIST ITEM #2] requires that each BEM block has its own CSS file, but all stars block styles are currently in src/style.css; please move the .stars, .stars__star, and modifier rules into a dedicated file (e.g. src/stars.css) and then import or link it from your main stylesheet or HTML. Second, the requirement states that the block with stars--N must have exactly N first stars active, including stars--0, but you do not define any explicit CSS for .stars--0; add a modifier rule like .stars--0 .stars__star { /* keep all inactive */ } to make it clear that this state has zero active stars and cannot be overridden by other rules. When you adjust the CSS structure and add explicit handling for stars--0, your implementation should align with both the functional spec and the BEM checklist. Overall, the core logic and layout look good, so once these changes are made you’ll be in great shape.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/style.css Outdated
padding: 0;
}

.stars {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates checklist item #2: Each BEM block has its own separate file. All stars block styles are in style.css; you should move them to a dedicated file for this BEM block and keep global styles separated.

Comment thread src/style.css Outdated
margin-right: 0;
}

.stars--1 .stars__star:nth-child(1) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task requires "The block with stars--N modifier should have exactly N first stars active" including the stars--0 case. There is no explicit CSS for .stars--0, so consider adding a rule ensuring that all stars remain inactive for this modifier to match tests more strictly.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it meets all core functional and checklist requirements for the Stars block. You correctly implemented 6 stars blocks with 5 stars__star elements each, used stars--0 through stars--5 modifiers, and ensured the first N stars become active purely via container modifiers and :nth-child selectors, without extra classes. The layout uses display: flex without gap, stars are implemented as background images instead of <img> or <svg>, and the browser default margin is reset on body, all matching the task constraints. You also respected the BEM structure by putting the stars block styles into a dedicated stars.css file and avoided tag selectors and the universal selector, which aligns with [CHECKLIST ITEM #2].

The only remaining point is a minor style improvement: consider adding a fallback font on body, for example font-family: system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;, to satisfy typical code style recommendations and improve robustness. This is not a blocker for this task, but it is a good habit for future projects. Nicely done tightening up the implementation and aligning both functionality and style with the requirements.


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

Footnotes

  1. Rate AI review example

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