Develop by ahsoker · Pull Request #6019 · mate-academy/layout_catalog

ahsoker · 2026-02-04T13:41:49Z

mateacademy-ai-mentor

Review Summary

Great work on the overall structure and responsive layout! The BEM implementation is solid. However, a few core requirements are not yet met, so I'm requesting some changes.

Here's what needs to be fixed:

Incorrect Card Title Hover: In src/styles/blocks/card.scss, the title color should change when the entire card is hovered, not just the title itself. The selector should be .card:hover .card__name.
Missing Transitions: The color changes on the card title and navigation links are instant. Please add the required transition property with a 300ms duration to make these effects smooth.
Hardcoded Values: Several values (e.g., the card title hover color #34568b, layout gaps, and media query breakpoints) are hardcoded. Please move these into variables in src/styles/blocks/variables.scss.
Old data-qa Attributes: The old data-qa="hover" attributes must be removed from src/index.html as per the task requirements.

Once these points are addressed, your solution will be ready for approval!

✨ 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 · 2026-02-04T13:44:08Z

src/styles/blocks/card.scss

+body {
+  margin: 0;
+  padding: 0;
+
+  font-family: $font-family;
+  font-optical-sizing: auto;
+  font-style: normal;
+  color: $main-accent;
+}


Styles for the <body> tag are considered global or base styles. It's better to place them in a more general file (like main.scss or a new _base.scss file) instead of a component-specific file like card.scss. This improves code organization.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/card.scss

+    &:hover {
+      color: #34568b;
+    }


This violates a core requirement: "change the card title text color to #34568b when the card is hovered". Your current implementation applies the hover effect only when the mouse is over the title itself (.card__name:hover). The style should be applied to .card__name when the parent .card element is hovered.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/card.scss

+  &__name {
+    font-weight: 500;
+    font-size: 12px;
+    line-height: 18px;
+
+    &:hover {
+      color: #34568b;
+    }
+  }


This violates checklist item #5: 'All changes on :hover are smooth'. The color change for the card title is currently instant because it lacks a transition property. Please add a transition for the color property to make the effect smooth, with a duration of 300ms as required.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/header.scss

+.nav__link {
+  position: relative;
+  display: block;
+  font-size: 12px;
+  color: $main-accent;
+  text-transform: uppercase;
+  text-decoration: none;
+  line-height: 60px;
+}


According to the requirements, all changes on hover should be smooth with a duration of 300ms. The color change for the navigation link is implemented, but the transition itself is missing. Please add a transition property to this selector.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/header.scss

+}
+
+.nav__link:hover {
+  color: #00acdc;


This color #00acdc is already defined as the $blue-accent variable in variables.scss. Using hardcoded values violates the requirement to use variables for special colors. Please replace this with the appropriate variable.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/main.scss

+.container {
+  --column-count: 1;
+
+  padding: 50px 40px;
+
+  display: grid;
+  grid-template-columns: repeat(var(--column-count), 200px);
+  gap: 46px 48px;
+  justify-content: center;
+}


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The values for padding, grid-template-columns, and gap are hardcoded. These should be defined as variables in src/styles/blocks/variables.scss to improve maintainability.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/main.scss

+@media (min-width: 488px) {
+  .container {
+    --column-count: 2;
+  }
+}
+
+@media (min-width: 768px) {
+  .container {
+    --column-count: 3;
+  }
+}
+
+@media (min-width: 1024px) {
+  .container {
+    --column-count: 4;
+  }
+}


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The breakpoint values (488px, 768px, 1024px) are hardcoded. It's best practice to declare screen sizes as variables in your variables.scss file for consistency across the project.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/index.html

+          <a
+            href="#"
+            class="card__button"
+            data-qa="hover"


According to the task description, old data-qa attributes should be removed. This data-qa="hover" attribute and similar ones on other cards should be removed to meet the requirements.

mateacademy-ai-mentor · 2026-02-04T13:44:08Z

src/styles/blocks/variables.scss

+$main-accent: #060b35;
+$secondary: #616070;
+$white: #fff;
+$blue-accent: #00acdc;
+$gray-elements: #f3f3f3;
+$yellow-stars: #ffde6a;
+$font-family: 'Roboto', sans-serif;
+$font-weight: 400;
+$font-size: 10px;
+$font-line-height: 14px;
+$star-width: 16px;
+$star-height: 16px;
+$star-notlastchild-margin-right: 4px;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. You're missing several variables that are hardcoded in other files. For example:

The card title hover color (#34568b)

Card dimensions (e.g., width: 200px)

Layout gaps and padding for the .container

Media query breakpoints (e.g., 488px, 768px)

Please define all such values here to make the code more maintainable.

mateacademy-ai-mentor

Review Summary

Excellent progress on this task! You've addressed most of the previous feedback, and the hover effects and overall structure are looking great. However, this requires a few changes before it can be approved, as a core requirement has not been fully met.

Specifically, some special colors and repeated sizes are still hardcoded, which violates the task requirements [CHECKLIST ITEM #2].

In src/styles/blocks/card.scss, the card title's hover color is hardcoded.
In src/styles/blocks/header.scss, the box-shadow uses a hardcoded color (#0000000d) and the font-size: 12px is a repeated value.

Please move these values into variables in src/styles/blocks/variables.scss. Once that's done, this will be ready for approval!

✨ 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 · 2026-02-04T15:27:47Z

src/styles/blocks/card.scss

+  }
+
+  &:hover .card__name {
+    color: #34568b;


This color is hardcoded. Per the task requirements, special colors should be moved into variables. Please create a new variable for this color in src/styles/blocks/variables.scss.

This violates checklist item #2: 'Repeated sizes and special colors are put to variables'.

mateacademy-ai-mentor · 2026-02-04T15:27:47Z

src/styles/blocks/header.scss

+  padding: 0 50px;
+  align-items: center;
+
+  box-shadow: 0 2px 4px 0 #0000000d;


This box-shadow uses a hardcoded color (#0000000d), and on line 40, font-size: 12px is a repeated value (it's also used in card.scss). This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. Please move these values to variables.scss.

mateacademy-ai-mentor · 2026-02-04T15:27:47Z

src/styles/blocks/header.scss

+.is-active {
+  color: $blue-accent;
+}
+
+.is-active::after {
+  content: '';
+  position: absolute;
+  left: 0;
+  bottom: 0;
+  width: 100%;
+  height: 4px;
+  border-radius: 8px;
+  background: $blue-accent;
+}


For better code organization and to leverage SCSS features, it's recommended to nest modifier styles within the main class. You could refactor the .is-active styles to be inside the .nav__link block like this:

.nav__link { /* ... */ &.is-active { color: $blue-accent; &::after { /* ... */ } } }

mateacademy-ai-mentor

Review Summary

Great job addressing the feedback from the last review! You've correctly moved many hardcoded values into variables. I am approving this submission as an educational override to help you move forward, but there's one key principle to fully master.

To fully adhere to the 'Don't Repeat Yourself' (DRY) principle [CHECKLIST ITEM #2], please make sure all repeated values are defined as variables. For instance, in src/styles/blocks/card.scss, values like border-radius: 5px and font-weight: 700 are used more than once and should be moved to variables.scss. Making this a habit will greatly improve your code's maintainability.

You've done excellent work on this project, keep it up!

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

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-04T15:41:42Z

src/styles/blocks/card.scss

+  display: flex;
+  flex-direction: column;
+
+  width: 200px;


This width is also used for the grid column definition in main.scss via the $grid-temp-col variable. To adhere to the 'Don't Repeat Yourself' (DRY) principle and meet the requirements of checklist item #2, please use that variable here as well.

mateacademy-ai-mentor · 2026-02-04T15:41:42Z

src/styles/blocks/card.scss

+  width: 200px;
+  height: 408px;
+  border: 1px solid $gray-elements;
+  border-radius: 5px;


This border-radius value is also used for the button on line 97. Since it's a repeated size, it should be defined as a variable in variables.scss as per checklist item #2.

mateacademy-ai-mentor · 2026-02-04T15:41:43Z

src/styles/blocks/card.scss

+    }
+
+    .price__value {
+      font-weight: 700;


This font-weight is repeated on line 99. It's best to move repeated values like this to a variable in variables.scss to comply with checklist item #2.

mateacademy-ai-mentor · 2026-02-04T15:41:43Z

src/styles/blocks/variables.scss

+$hover-shadow: #0000000d;
+$yellow-stars: #ffde6a;
+$font-family: 'Roboto', sans-serif;
+$font-weight: 400;


This is great for the default font weight. I see that font-weight: 700 is used multiple times in card.scss. To fully comply with checklist item #2, please add a new variable here for that repeated value, for example, $font-weight-bold: 700;.

ahsoker added 3 commits February 4, 2026 13:18

solution

9b9256f

solution 2

36fd203

add task solution

f11682b

mateacademy-ai-mentor suggested changes Feb 4, 2026

View reviewed changes

ahsoker added 2 commits February 4, 2026 16:20

add task solution 2

91c9562

add task solution 3

3afcaa3

ahsoker requested a review from mateacademy-ai-mentor February 4, 2026 15:25

mateacademy-ai-mentor suggested changes Feb 4, 2026

View reviewed changes

add task solution 4

0a07147

ahsoker requested a review from mateacademy-ai-mentor February 4, 2026 15:39

mateacademy-ai-mentor approved these changes Feb 4, 2026

View reviewed changes

Conversation

ahsoker commented Feb 4, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments