Skip to content

add task solution#6867

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

add task solution#6867
runaryndych wants to merge 3 commits into
mate-academy:masterfrom
runaryndych:develop

Conversation

@runaryndych

Copy link
Copy Markdown
  • 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 ❗️

@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

Changes are needed before this can be considered complete, because several core requirements and checklist items are violated.

Critical issues that must be fixed:

  • Active (yellow) stars are controlled via an extra class stars__star--active in both index.html (e.g. lines 28, 36, 37…) and _stars.scss, which breaks the rule that yellow stars must be driven only by the container modifier (stars--N) plus pseudo-selectors and that no extra classes are added to the elements [CHECKLIST ITEM #1]. You should remove stars__star--active from the HTML and CSS and instead use selectors like .stars--3 .stars__star:nth-child(-n+3) to make the first N stars yellow based on the parent modifier.
  • In index.html the <link> tag points to ./styles/style.scss, but browsers cannot load SCSS directly; this prevents styles from being applied. Update this to reference the compiled CSS output (for example style.css in the correct folder) so the page renders correctly.

Minor but recommended improvements:

  • The body margin reset is only applied via .page__body; since you are allowed to style body, also adding body { margin: 0; } will more robustly satisfy the “reset default margin” requirement.
  • The page uses only div for structure; wrapping the ratings in a semantic container such as <main> or <section> will better align with the “use semantic tags where possible” guideline.

Once you rework the active-star logic to use container modifiers with pseudo-selectors and fix the stylesheet link, the solution will be in much better shape and should meet the task expectations.


✨ 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/index.html Outdated
<link
rel="stylesheet"
href="./style.css"
href="./styles/style.scss"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

href points to style.scss, but browsers cannot load SCSS directly; update this to reference the compiled CSS file (e.g. style.css) so styles are applied correctly.

Comment thread src/index.html Outdated
</div>

<div class="stars stars--1">
<div class="stars__star stars__star--active"></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Active stars are marked with an extra class stars__star--active, which violates the checklist item: "Yellow stars are added with container modifier + pseudo-selector (NO extra classes)"; remove this class and rely on the stars--1 modifier with appropriate CSS selectors instead.

Comment thread src/index.html Outdated
</div>

<div class="stars stars--2">
<div class="stars__star stars__star--active"></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same issue as line 28: using stars__star--active on this element conflicts with the requirement to use only the container modifier plus pseudo-selectors to set active stars.

Comment thread src/index.html Outdated

<div class="stars stars--2">
<div class="stars__star stars__star--active"></div>
<div class="stars__star stars__star--active"></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same issue as other active stars: this stars__star--active class should be removed and its visual state derived from the parent stars--N modifier instead.

Comment thread src/index.html Outdated
</div>

<div class="stars stars--3">
<div class="stars__star stars__star--active"></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here again, the third star is made active via stars__star--active, but per requirements, the stars--3 modifier should control which stars are yellow using selectors, without extra element-level classes.

Comment thread src/index.html Outdated
</div>

<div class="stars stars--4">
<div class="stars__star stars__star--active"></div>

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 active star also relies on stars__star--active; you should instead use the stars--4 modifier and nth-child/pseudo-selector logic to control its appearance.

Comment thread src/index.html Outdated
</div>

<div class="stars stars--5">
<div class="stars__star stars__star--active"></div>

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 fifth stars block uses stars__star--active on every star; replace this approach with CSS driven purely by the stars--5 modifier.

Comment thread src/index.html Outdated
<link
rel="stylesheet"
href="./style.css"
href="./styles/style.scss"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

.stars__star--active is a separate modifier class on the element, which contradicts checklist item: "Yellow stars are added with container modifier + pseudo-selector (NO extra classes)"; remove this class and implement active-state styling via .stars--N selectors targeting .stars__star.

Comment thread src/styles/blocks/_stars.scss Outdated
background-repeat: no-repeat;
}

.stars__star--active {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Defining .stars__star--active contradicts checklist item #1: "Yellow stars are added with container modifier + pseudo-selector (NO extra classes)". Instead, adjust .stars modifier styles (e.g. .stars--3 .stars__star:nth-child(-n+3)) to change the background for active stars and remove this extra modifier class.

Comment thread src/styles/style.scss Outdated
Comment on lines +1 to +2
/* add styles here */
@use 'blocks/page';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using .page__body to reset margin works only if this class is always on <body>. Since the requirement is to reset browser default margin and you’re allowed to style body, consider also setting body { margin: 0; } to fully align with the requirement and avoid relying on a class.

@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 is still a critical functional issue that blocks correct rendering in the browser. In src/index.html, the <link> tag currently points to ./styles/style.scss, but browsers cannot load SCSS directly; they only understand compiled CSS, so your page will appear unstyled. This violates the requirement that the HTML must link to the compiled stylesheet so that all stars styling and the body { margin: 0; } reset are applied at runtime.

Critical issue to fix (blocker):

  1. Update the stylesheet link in src/index.html (around the <head> section) from href="./styles/style.scss" to the path of your compiled CSS file, for example:
    <link rel="stylesheet" href="./styles/style.css">
    Make sure your build actually outputs style.css (or adjust the name/path accordingly).

Everything else looks well done: you correctly implemented 6 stars blocks with the right modifiers, used container modifiers plus :nth-child selectors for active stars, separated BEM blocks into their own SCSS files, used flex without gap, and reset the body margin. As a minor, non-blocking improvement, you could wrap your ratings in a semantic container like <main> or <section> to better follow semantic HTML guidelines, but this is not required for acceptance. Once you fix the stylesheet link to target the compiled CSS, your solution should meet the task requirements.


✨ 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/index.html Outdated
<link
rel="stylesheet"
href="./style.css"
href="./styles/style.scss"

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 stylesheet link still points to the SCSS source (href="./styles/style.scss"), but browsers can only load compiled CSS; this violates the requirement that the <link> must reference the compiled CSS output so styles actually apply.


background-image: url(../images/star.svg);
background-position: center;
background-repeat: no-repeat;

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 <link> tag still points to style.scss, but browsers can’t load SCSS directly. You need to update href to the compiled CSS file (e.g. ./styles/style.css) so styles are actually applied. This is required by the task and repeats the previous correction about not linking to SCSS.

@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 approving your solution as an educational override to help you continue progressing while you learn. You did a very solid job meeting the key requirements: six .stars blocks with five .stars__star elements each, correct use of modifiers stars--0 through stars--5, flex layout without gap, and using background-image instead of <img> or <svg> tags. Your BEM structure is clean, with separate CSS files for page and stars, and the body { margin: 0; } reset is correctly placed and linked.

One improvement to keep in mind is the way you activate yellow stars: you correctly used .stars--N .stars__star:nth-child(-n + N) to control active stars, but the checklist phrase "pseudo-selector" may be hinting toward using a pseudo-element pattern (e.g., ::before) for the visual part [CHECKLIST ITEM #1]; if you revisit this, try to move the decorative part to a pseudo-element while keeping the same logic. Also, double-check your star size and spacing against the Figma design to ensure pixel-perfect alignment. Overall, your structure and attention to constraints are strong, and these refinements will help you get even closer to production-quality code. Keep going—this is a great foundation and you’re clearly understanding the requirements well.


✨ 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