Skip to content

added solution#6892

Open
justnicko wants to merge 2 commits into
mate-academy:masterfrom
justnicko:develop
Open

added solution#6892
justnicko wants to merge 2 commits into
mate-academy:masterfrom
justnicko:develop

Conversation

@justnicko

Copy link
Copy Markdown

No description provided.

@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 there are still a couple of direct violations of the task requirements that will likely break the automated tests.

  1. The stars container .stars is currently using gap in your flex layout (in src/styles/blocks/stars.css and referenced in src/styles/style.css). The task explicitly says not to use gap on flex containers because it does not work in tests. Instead, remove gap from .stars and add horizontal spacing via margins on .stars__star, for example:
    • src/styles/blocks/stars.css: use something like .stars__star:not(:last-child) { margin-right: 4px; }, making sure the exact value matches the distance specified in Figma.
  2. The requirement says to reset the browser’s default margin, and in your CSS you only reset body { margin: 0; }. Typically the default margin on html should be reset too, and the checklist forbids using the universal * selector. Add an explicit rule like html { margin: 0; } in src/styles/style.css next to your body rule.

Everything else about your structure (6 blocks, 5 stars each, modifiers, background images, and separate BEM file) looks solid, so once you fix the flex spacing and the html margin reset, your solution should pass the requirements. Keep going—your implementation is very close, and these changes will strengthen both correctness and adherence to the checklist.


✨ 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/styles/blocks/stars.css Outdated
.stars {
display: flex;
flex-direction: row;
gap: 4px;

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 gap on the flex container violates the requirement: "DON'T use gap property for flex container because it does not work in tests". Replace gap with margins on stars__star elements to control the distance between stars while keeping blocks themselves without vertical margins.

Comment thread src/styles/style.css
/* add styles here */
@import './blocks/stars.css';

body {

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 the task requirement: "DON'T use gap property for flex container because it does not work in tests". Replace gap with spacing via margins on .stars__star elements instead.

Comment thread src/styles/style.css
/* add styles here */
@import './blocks/stars.css';

body {

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 description requires "Reset browser's default margin". Here you reset only body. Add a rule for htmlto reset its margin as well, without using the forbidden universal*` selector.

@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 fully meets the functional and checklist requirements for the Stars block task. You correctly reset browser default margins on both html and body, and implemented exactly six .stars blocks in src/index.html, each containing five .stars__star elements with the stars--0 through stars--5 modifiers used once each. The active yellow stars are driven purely by container modifiers and :nth-child selectors in src/styles/blocks/stars.css, with background-image used instead of <img> or <svg>, matching the task constraints and [CHECKLIST ITEM #1]. The flex layout is implemented on .stars without using gap, and spacing is handled via horizontal margin-right on .stars__star, which aligns with the corrected layout requirements. Each BEM block has its own CSS file under styles/blocks/ and is imported into styles/style.css, satisfying the separation of concerns requirement and [CHECKLIST ITEM #2]. Overall, there are no critical blockers or violations of the explicit checklist items, and your implementation should work correctly with the automated tests. Nicely done bringing everything in line with the BEM rules and the task details.


✨ 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