Card commit #1 #5920

SerhiyDmytruk · 2025-08-28T07:37:49Z

DEMO LINK
TEST REPORT LINK
there 2 BEM blocks card and stars each in their own file
SCSS Nesting is used for elements, modifiers and pseudo-classes
SCSS Variables are used for main values and placed in a separate file
all stars--N modifiers work as expected (Highlight first N stars)
Code follows all the Code Style Rules ❗️

mateacademy-ai-mentor

Review Summary

Decision: REQUEST_CHANGES — there are critical blockers that violate explicit requirements.

Why: Several core checklist items are not met, which will break automated checks and required functionality.

Critical issues to fix:

Incorrect BEM block name for the card
- Where: src/index.html line with root card element (uses class="product__wrapper"), and src/styles/card.scss selectors (use .product)
- Requirement: Checklist #1 — implement a card block (separate from stars)
- How to fix: Rename the root element class to card and refactor all related selectors to .card, e.g., card__image, card__title, etc. Avoid product as a block name.
Flexbox not used for the card layout
- Where: src/styles/card.scss
- Requirement: Card layout must use Flexbox
- How to fix: Add display: flex (and appropriate flex-direction, gaps/alignment) to the main card container and/or its internal layout wrappers per the mockup.
Card width must include border
- Where: src/styles/card.scss
- Requirement: Card width exactly 200px including border (Req 1.4, checklist 3.7)
- How to fix: Add box-sizing: border-box; to .card to ensure borders are included in the 200px width.
Variables file import missing in card.scss
- Where: src/styles/card.scss
- Requirement: Checklist #3 — variables in a separate file and imported where needed
- How to fix: Add @import 'variables'; (or your chosen import method) at the top so $color-white, $color-blue, etc., resolve correctly.
Incomplete stars modifiers
- Where: src/styles/stars.scss
- Requirement: Checklist #4 — stars--N for N=1..5 must highlight first N stars
- How to fix: Implement modifiers for stars--1, --2, --3, --4, --5 so only the first N items get the highlighted background.

Minor suggestions:

Consider a global *, *::before, *::after { box-sizing: border-box; } reset for consistency.
Double-check the product image path in HTML to match your build output expectations.

You’re very close—your project structure, variables, and the stars block foundation look good. Address the items above and you should be set to pass. Keep it up!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-08-28T07:42:04Z

src/index.html

+        <div
+          class="product__wrapper"


The root card element uses class="product__wrapper" instead of the required BEM block name card. This violates checklist item #1: "Implement two separate BEM blocks: card and stars." The card block must be named card, not product__wrapper.

mateacademy-ai-mentor · 2025-08-28T07:42:04Z

src/styles/card.scss

@@ -0,0 +1,92 @@
+.product {


This violates checklist item #1: "Implement two separate BEM blocks: card and stars." The card block must be named card, not product. All elements should be nested under .card instead of .product.

mateacademy-ai-mentor · 2025-08-28T07:42:04Z

src/styles/card.scss

@@ -0,0 +1,92 @@
+.product {


You are using SCSS variables like $color-white and $color-blue, but there is no import of the variables file. This violates checklist item #3: "Define SCSS variables for main values ... and place them in a separate variables file that is imported where needed."

mateacademy-ai-mentor · 2025-08-28T07:42:04Z

src/styles/card.scss

+    border: 1px solid rgba(243, 243, 243, 1);
+    border-radius: 5px;
+    padding: 32px 0 16px;
+    width: 200px;


The card width is set to 200px, but you must ensure that the border is included in the total width. Add box-sizing: border-box; to the card block to satisfy requirement 1.4 and checklist item 3.7.

mateacademy-ai-mentor · 2025-08-28T07:42:05Z

src/styles/stars.scss

+  margin: 0;
+}
+
+.stars--4 {


This violates checklist item #4: "Implement stars--N modifiers so that for each valid N, the first N star elements are visually highlighted while others are not." Only stars--4 is implemented. You must add modifiers for all valid N (1 to 5).

mateacademy-ai-mentor

Review Summary

Decision: REQUEST_CHANGES

Your work is strong overall (clean BEM, good structure, correct assets, global resets and fonts). However, there are critical blockers that violate explicit checklist items and affect required functionality.

Critical issues to fix

stars.scss – Missing required modifiers (Checklist #4)
- Location: src/styles/stars.scss
- Issue: Only .stars--4 is implemented. The component must support .stars--1, .stars--2, .stars--3, .stars--4, and .stars--5, with the first N stars highlighted for each modifier.
- Why it’s wrong: The task requires verifiable support for all rating levels used (1–5). With only --4, other ratings cannot render correctly.
- How to fix: Add modifiers for all N or generate them via a loop. For example, use a loop to target .stars--#{n} .stars__item:nth-child(-n+#{n}) and apply the active background image. Ensure .stars__item has the default (inactive) star, and the active state overrides it for the first N items.
card.scss – Width must include border in final 200px (Checklist 3.7)
- Location: src/styles/card.scss, .card__wrapper width and border (exact line numbers may vary, but where width: 200px and border are defined)
- Issue: There’s no explicit box-sizing: border-box; on .card__wrapper (or .card). While you have a global reset in index.scss, the checklist explicitly requires ensuring the card’s computed width is 200px including border at the component level.
- Why it’s wrong: Without explicit box-sizing on the card container, the requirement can be inadvertently broken if global styles change. Review feedback flagged this as a violation.
- How to fix: Add box-sizing: border-box; to .card__wrapper (or to .card if you move the sizing there). Example: .card__wrapper { box-sizing: border-box; width: 200px; border: 1px solid ... }.

Minor suggestions

Consider placing primary layout/sizing on the root .card container for clarity, keeping .card__wrapper for inner layout if needed.

You’re very close—address these items and it should fully meet the requirements. Keep up the good work!

✨ Please rate this review in the Platform's notification center¹. 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 button² 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 tag³.
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.

mateacademy-ai-mentor · 2025-08-28T07:55:59Z

src/styles/card.scss

+    border: 1px solid rgba(243, 243, 243, 1);
+    border-radius: 5px;
+    padding: 32px 0 16px;
+    width: 200px;


This violates checklist item 3.7: "Ensure the card total width computes to 200px including border (i.e., account for border width in sizing; box-sizing choice must yield correct final width)." Add box-sizing: border-box; to .card__wrapper (or globally) to ensure the border is included in the 200px width.

mateacademy-ai-mentor · 2025-08-28T07:55:59Z

src/styles/stars.scss

+.stars--4 {
+  .stars__star {
+    @for $i from 1 through 4 {
+      &:nth-child(#{$i}) {
+        background-image: url('../images/star-active.svg');
+      }
+    }
+  }
+}


This violates checklist item #4: "Implement stars--N modifiers (where N corresponds to rating levels used in the layout) so that the first N stars are highlighted correctly for each N. This must be verifiable for all supported N values." Only .stars--4 is implemented. You must provide .stars--1, .stars--2, .stars--3, .stars--4, and .stars--5 modifiers so that the first N stars are highlighted for each N.

mateacademy-ai-mentor

Review Summary

Educational Approval

Fantastic work—this implementation cleanly meets the task’s requirements and demonstrates solid SCSS + BEM practices. Highlights:

BEM/SCSS structure is clear and modular (card and stars blocks in separate files).
Stars modifiers .stars--1…--5 are generated via an SCSS loop, correctly highlighting the first N stars (checklist #4).
Card width is precisely handled with explicit box-sizing: border-box; on .card__wrapper, ensuring the 200px requirement is met.
Accessibility/semantics are sound: main container, button for BUY, and meaningful structure.
Parcel + Google Fonts integration is correct; assets are referenced from src/images as required.

Minor polish ideas (optional, for learning):

Consider adding :focus-visible styles to the BUY button for better keyboard accessibility.
If this grew, extract additional SCSS variables (e.g., spacing, font-sizes) for easier theming.

Great job—everything important is in place and tidy. Keep up the strong attention to detail!

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

Rate AI review example ↩

Card commit mate-academy#1

3ceb6f4

mateacademy-ai-mentor suggested changes Aug 28, 2025

View reviewed changes

Fixed remarks

d982401

SerhiyDmytruk requested a review from mateacademy-ai-mentor August 28, 2025 07:52

mateacademy-ai-mentor suggested changes Aug 28, 2025

View reviewed changes

Fixed remarks

87fd296

SerhiyDmytruk requested a review from mateacademy-ai-mentor August 28, 2025 08:04

mateacademy-ai-mentor approved these changes Aug 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Card commit #1 #5920

Card commit #1 #5920

Uh oh!

SerhiyDmytruk commented Aug 28, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Uh oh!

Uh oh!

Card commit #1 #5920

Are you sure you want to change the base?

Card commit #1 #5920

Uh oh!

Conversation

SerhiyDmytruk commented Aug 28, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Uh oh!