develop by fedechkova · Pull Request #6021 · mate-academy/layout_catalog

fedechkova · 2026-02-04T21:33:36Z

DEMO LINK
TEST REPORT LINK
[ x ] All components follow BEM and use SCSS
[ x ] Repeated sizes and special colors are put to variables
[ x ] Grid is used for the columns
[ x ] Cards are shown in 1, 2, 3, or 4 columns based on screen resolution
[ x ] All changes on :hover are smooth
[ x ] Code follows all the Code Style Rules ❗️

mateacademy-ai-mentor

Review Summary

Great work on the overall structure and functionality! The BEM implementation and responsive grid are well done. However, changes are needed to ensure full compliance with the project's coding standards, specifically regarding the use of SCSS variables, which violates checklist item #2: 'Repeated sizes and special colors are put to variables'.

To improve maintainability, please address the following:

In src/styles/blocks/nav.scss, replace the hardcoded values 12px and 4px with the existing variables $font-size-sm and $space-xxs.
In src/styles/blocks/header.scss, the box-shadow color #0000000d should be moved to a new variable in src/styles/utils/variables.scss.
In src/styles/blocks/card.scss, the $card-width should be a global variable as it's shared with other components. Variables used only once locally can be removed.

Once these variables are tidied up, the solution will be perfect. Keep up the great effort!

✨ 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-04T21:35:40Z

src/styles/blocks/card.scss

+$card-width: 200px;
+$card-height: 408px;
+$card-padding-y: 32px;
+$card-padding-x: 16px;
+$image-width: 160px;
+$image-height: 134px;
+$radius: 5px;
+$border-width: 1px;


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

The card width of 200px is also used in catalog.scss. To avoid magic numbers and maintain a single source of truth, variables for values shared across components (like $card-width) should be moved to the global src/styles/utils/variables.scss file.

For variables used only once within this file (e.g., $card-height), you can remove them and use the value directly to simplify the code.

mateacademy-ai-mentor · 2026-02-04T21:35:40Z

src/styles/blocks/nav.scss

+  }
+
+  &__link {
+    font-size: 12px;


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

A variable $font-size-sm already exists for the value 12px in the variables.scss file. Please use the variable to maintain consistency.

mateacademy-ai-mentor · 2026-02-04T21:35:40Z

src/styles/blocks/nav.scss

+        left: 0;
+        right: 0;
+        bottom: 0;
+        height: 4px;


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

A variable $space-xxs already exists for the value 4px in the variables.scss file. Please use this variable instead of a hardcoded value.

mateacademy-ai-mentor · 2026-02-04T21:35:40Z

src/styles/blocks/header.scss

+  justify-content: space-between;
+  padding: 0 50px;
+  background-color: $color-white;
+  box-shadow: 0 2px 4px 0 #0000000d;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The color value #0000000d for the box shadow should be moved to a variable in src/styles/utils/variables.scss for better maintainability.

mateacademy-ai-mentor

Review Summary

Excellent progress on the project, and thank you for addressing the previous feedback! The solution is very close to being perfect. I am requesting changes because a core requirement to replace all hardcoded values with variables has not been fully met yet. This is a violation of checklist item #2 ('Repeated sizes and special colors are put to variables').

Specifically, I found hardcoded "magic numbers" for padding, gaps, and media query breakpoints in the following files:

src/styles/blocks/catalog.scss
src/styles/blocks/header.scss
src/styles/blocks/nav.scss

Please extract these values into centrally managed variables in src/styles/utils/variables.scss. Once that's done, the 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-04T22:00:55Z

src/styles/blocks/card.scss

+$card-padding-y: 32px;
+$card-padding-x: 16px;
+$image-width: 160px;
+$image-height: 134px;
+$radius: 5px;
+$border-width: 1px;


Good job moving the shared variables! However, the local variables $card-padding-y, $card-padding-x, and $image-width are still defined here but are only used once each.

As per the previous feedback and checklist item #2 ('Repeated sizes and special colors are put to variables'), it's best to remove local variables that aren't reused. This improves readability by reducing unnecessary abstractions. You can place these values directly where they are used, for example: padding: 32px 16px;.

mateacademy-ai-mentor · 2026-02-04T22:00:55Z

src/styles/blocks/catalog.scss

@@ -0,0 +1,22 @@
+.catalog {
+  padding: 50px 40px;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The padding value 50px is also used in header.scss. Please extract it into a new variable. Additionally, you can replace 40px with the existing $space-xl variable.

mateacademy-ai-mentor · 2026-02-04T22:00:55Z

src/styles/blocks/catalog.scss

+    display: grid;
+    justify-content: center;
+    grid-template-columns: $card-width;
+    gap: 46px 48px;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. These hardcoded gap values should be moved to variables for better readability and easier maintenance. For example, you could name them $catalog-row-gap and $catalog-column-gap.

mateacademy-ai-mentor · 2026-02-04T22:00:55Z

src/styles/blocks/catalog.scss

+    @media (min-width: 488px) {
+      grid-template-columns: repeat(2, $card-width);
+    }
+
+    @media (min-width: 768px) {
+      grid-template-columns: repeat(3, $card-width);
+    }
+
+    @media (min-width: 1024px) {
+      grid-template-columns: repeat(4, $card-width);


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The media query breakpoints (488px, 768px, 1024px) are hardcoded. Please define these responsive breakpoints as variables (e.g., $screen-sm, $screen-md, $screen-lg) in src/styles/utils/variables.scss to manage them centrally.

mateacademy-ai-mentor · 2026-02-04T22:00:55Z

src/styles/blocks/nav.scss

+    margin-right: 20px;
+  }
+
+  &__link {
+    font-size: $font-size-sm;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    height: 60px;
+    text-transform: uppercase;
+    text-decoration: none;
+    color: $color-text-primary;
+    transition: color $transition;
+
+    &:hover {
+      color: $color-accent;
+    }
+
+    &.is-active {
+      position: relative;
+      color: $color-accent;
+
+      &::after {
+        content: '';
+        position: absolute;
+        left: 0;
+        right: 0;
+        bottom: 0;
+        height: $space-xxs;
+        border-radius: 8px;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The values 20px, 60px, and 8px are hardcoded. Please extract these into appropriately named variables in your variables.scss file to avoid using 'magic numbers'.

mateacademy-ai-mentor · 2026-02-04T22:00:55Z

src/styles/blocks/header.scss

+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  padding: 0 50px;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The padding value 50px is also used in catalog.scss. Please extract it into a new variable (e.g., $space-lg) in src/styles/utils/variables.scss and use it in both places.

mateacademy-ai-mentor

Review Summary

Excellent progress on this task! You've successfully addressed most of the feedback from the previous review, and the code quality has improved significantly. I am approving this solution to help you move forward, but please pay close attention to the remaining feedback for future tasks.

A few hardcoded "magic numbers" still exist in your styles, which violates the requirement to use variables for all repeated sizes [CHECKLIST ITEM #2]. For example:

In src/styles/blocks/card.scss, values for padding, background-size, and top should be variables.
In src/styles/blocks/header.scss, the pixel values within the box-shadow property should also be variables.

Always remember to centralize these values in variables.scss to keep your code clean and maintainable. Keep up the great work!

✨ 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-04T22:42:59Z

src/styles/blocks/card.scss

+$image-height: 134px;
+$radius: 5px;
+$border-width: 1px;


It's good practice to centralize all variables. Consider moving these block-specific variables ($image-height, $radius, $border-width) to the src/styles/utils/variables.scss file to maintain consistency across the project.

mateacademy-ai-mentor · 2026-02-04T22:42:59Z

src/styles/blocks/card.scss

+$border-width: 1px;
+
+.card {
+  height: 408px;


This hardcoded value 408px violates checklist item #2 ('Repeated sizes and special colors are put to variables'). Please define a variable for the card height in variables.scss and use it here.

mateacademy-ai-mentor · 2026-02-04T22:42:59Z

src/styles/blocks/card.scss

+  width: $card-width;
+  border: $border-width solid $color-border;
+  border-radius: $radius;
+  padding: 32px 16px;


The value 32px is a hardcoded "magic number". Please create a new spacing variable for it in variables.scss, similar to how you've used $space-sm for 16px.

mateacademy-ai-mentor · 2026-02-04T22:42:59Z

src/styles/blocks/card.scss

+
+  &__image {
+    width: 100%;
+    background-size: 160px $image-height;


The background-size contains a hardcoded value 160px. This should be replaced with a variable defined in variables.scss to adhere to the project's standards and checklist item #2.

mateacademy-ai-mentor · 2026-02-04T22:42:59Z

src/styles/blocks/card.scss

+    font-size: $font-size-xs;
+    line-height: $line-height-xs;
+    position: relative;
+    top: 1px;


This 1px value used for positioning is another "magic number". It's better to define it as a small spacing or border-width variable to avoid hardcoding pixel adjustments.

mateacademy-ai-mentor · 2026-02-04T22:42:59Z

src/styles/blocks/header.scss

+  justify-content: space-between;
+  padding: 0 $space-xxl;
+  background-color: $color-white;
+  box-shadow: 0 2px 4px 0 $color-shadow;


The box-shadow property contains hardcoded values (2px, 4px). Please replace them with variables. You can use $space-xxs for 4px and consider creating a new variable for 2px in variables.scss.

add task solution

c791ace

mateacademy-ai-mentor suggested changes Feb 4, 2026

View reviewed changes

refactor shared and local scss variables

8820528

fedechkova requested a review from mateacademy-ai-mentor February 4, 2026 21:58

mateacademy-ai-mentor suggested changes Feb 4, 2026

View reviewed changes

extract nav sizes to variables

1cf806e

fedechkova requested a review from mateacademy-ai-mentor February 4, 2026 22:41

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

View reviewed changes

Conversation

fedechkova 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments