Solution by KrzywMan · Pull Request #6125 · mate-academy/layout_catalog

KrzywMan · 2026-03-23T12:46:14Z

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

mateacademy-ai-mentor

Review Summary

Great job on the visual implementation and the HTML structure! The page is responsive and all the hover effects work as expected.

However, some changes are needed regarding the SCSS architecture. The main issue is that src/styles/index.scss contains all the styles, duplicating the content from the partial files in styles/blocks/ and _variables.scss. The index.scss file should only be used to @import the other partials to create a modular and maintainable structure.

Additionally, many repeated values like paddings, gaps, card widths, and media query breakpoints are hardcoded. Please define these as variables in _variables.scss as required by the checklist [CHECKLIST ITEM #2].

One last detail: for the card's z-index on hover to work correctly, the .card element also needs a position: relative; property. Please make these adjustments to align the project with the task requirements.

✨ 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-03-23T12:49:04Z

+$font-main: 'Roboto', 'Arial', sans-serif;
+$color-text-nav: #060b35;
+$color-accent: #00acdc;
+$color-title-hover: #34568b;
+$color-white: #fff;
+$transition-duration: 300ms;


This file is a good start for centralizing your variables, but there are a couple of major issues with the overall SCSS implementation:

Unused Partials: This file and all the files in the styles/blocks/ directory are currently unused. Your main stylesheet, index.scss, duplicates all styles and variables instead of importing these partials. You should use @import in index.scss to build your styles from these modular files.

Missing Variables: This violates checklist item Vitalii Pshychenko #2: 'Repeated sizes and special colors are put to variables'. This file should contain all reusable values. Please add variables for sizes like card width, gaps, and paddings. For example:

$card-width: 200px; $catalog-gap: 46px 48px;

Then use these variables throughout your other SCSS files.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

+  padding: 50px 40px;
+  gap: 46px 48px;
+  grid-template-columns: repeat(1, 200px);


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The hardcoded values for padding, gap, and card width should be defined as variables in your _variables.scss file for better maintainability. The card width 200px is used in multiple files and is a clear candidate for a variable.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

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


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. These media query breakpoints should also be defined as variables (e.g., $breakpoint-tablet, $breakpoint-desktop). This makes your responsive design more consistent and easier to manage across the project.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

@@ -0,0 +1,53 @@
+.card {


For the z-index property (set on line 47) to have an effect, this element needs a position property with a value other than static (e.g., relative). This is crucial for making sure the hovered card appears on top of its neighbors, which is required so that "neighboring cards should not be affected".

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

+    list-style: none;
+    padding: 0;
+    margin: 0;
+    gap: 20px;


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The gap value 20px should be stored in a variable in your _variables.scss file to improve maintainability, for example: $nav-gap: 20px;.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

+$font-main: 'Roboto', sans-serif;
+$color-nav: #060b35;
+$color-hover: #00acdc;
+$color-title-hover: #34568b;
+$color-white: #fff;
+$transition-speed: 300ms;


These variables should not be redefined here. The index.scss file should import the _variables.scss partial at the very top using @import '../variables';. The current approach creates duplicated and conflicting variable definitions, which undermines the maintainability of the code.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

+  padding: 50px 40px;
+  gap: 46px 48px;
+  grid-template-columns: repeat(1, 200px);


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. All these hardcoded values (padding, gap, width) should be defined as variables in _variables.scss and then used here. For example: $catalog-padding: 50px 40px;.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

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


This violates checklist item #2: 'Repeated sizes and special colors are put to variables'. The media query breakpoints (488px, 768px, 1024px) should also be defined as variables in _variables.scss for consistency and easier maintenance.

mateacademy-ai-mentor · 2026-03-23T12:49:05Z

+.header {
+  padding: 20px 40px;
+}
+
+.nav {
+  &__list {
+    display: flex;
+    margin: 0;
+    padding: 0;
+    list-style: none;
+    gap: 20px;
+  }
+
+  &__link {
+    font-weight: 500;
+    color: $color-nav;
+    text-decoration: none;
+    transition: color $transition-speed;
+    cursor: pointer;
+
+    &:hover,
+    &.is-active {
+      color: $color-hover;
+    }
+  }
+}
+
+.catalog {
+  display: grid;
+  justify-content: center;
+  padding: 50px 40px;
+  gap: 46px 48px;
+  grid-template-columns: repeat(1, 200px);
+
+  @media (min-width: 488px) {
+    grid-template-columns: repeat(2, 200px);
+  }
+
+  @media (min-width: 768px) {
+    grid-template-columns: repeat(3, 200px);
+  }
+
+  @media (min-width: 1024px) {
+    grid-template-columns: repeat(4, 200px);
+  }
+}
+
+.card {
+  position: relative;
+  width: 200px;
+  border-radius: 8px;
+  background-color: #f9f9f9;
+  transition: transform $transition-speed;
+  cursor: pointer;
+
+  &__image {
+    display: block;
+    width: 100%;
+  }
+
+  &__content {
+    padding: 10px;
+  }
+
+  &__title {
+    margin: 10px 0;
+    font-size: 16px;
+    transition: color $transition-speed;
+  }
+
+  &__footer {
+    display: flex;
+    align-items: center;
+    justify-content: space-between;
+    margin-top: 15px;
+  }
+
+  &__price {
+    font-weight: 700;
+    color: $color-nav;
+  }
+
+  &__button {
+    padding: 8px 16px;
+    color: $color-white;
+    text-decoration: none;
+    background-color: $color-hover;
+    border: 1px solid $color-hover;
+    border-radius: 4px;
+    transition: all $transition-speed;
+
+    &:hover {
+      color: $color-hover;
+      background-color: $color-white;
+    }
+  }
+
+  &:hover {
+    z-index: 10;
+    transform: scale(1.2);
+
+    .card__title {
+      color: $color-title-hover;
+    }
+  }
+}
+
+.stars {
+  font-size: 14px;
+  color: #ccc;
+
+  &__item {
+    &--filled {
+      color: #ffc107;
+    }
+  }


All of these style blocks (.header, .nav, .catalog, .card, .stars) should be removed from this file. They should exist in their own respective partial files within the styles/blocks/ directory. This file should then import them using @import statements. The current implementation makes all the other .scss files redundant.

mateacademy-ai-mentor

Review Summary

Excellent work on refactoring the SCSS structure! The code is much cleaner and more modular now. However, a few changes are needed to meet all the requirements.

There's a small typo in src/styles/blocks/_card.scss where you've used $color-nav-text instead of the correct $color-text-nav. This typo is preventing the card price from being styled correctly.

Additionally, to fully comply with the requirement to use variables [CHECKLIST ITEM #2], please move the remaining hardcoded values to _variables.scss. This includes:

The background-color: #f9f9f9; in _card.scss.
The 20px and 40px padding values in _header.scss.
The 20px gap in _nav.scss.
The 4px gap and 14px font-size in _stars.scss.

Once these fixes are in place, the solution will be perfect. Keep up the great 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 · 2026-03-23T13:12:16Z

@@ -0,0 +1,16 @@
+$color-text-nav: #060b35;


Great job defining this variable for the navigation text color. Just a small heads-up: in src/styles/blocks/_card.scss, when setting the color for .card__price, this variable is misspelled as $color-nav-text. Please correct the typo to match the name defined here.

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

+$card-width: 200px;
+$bp-mobile-plus: 488px;
+$bp-tablet: 768px;
+$bp-desktop: 1024px;


This file is well-organized. To fully comply with checklist item #2 ('Repeated sizes and special colors are put to variables'), please add variables for a few remaining hardcoded values. For instance, the background-color: #f9f9f9; in _card.scss should be defined as a variable here (e.g., $color-card-bg: #f9f9f9;).

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

+$card-width: 200px;
+$bp-mobile-plus: 488px;
+$bp-tablet: 768px;
+$bp-desktop: 1024px;


Checklist item #2 also applies to repeated sizes. The value 20px is used as padding in _header.scss and as a gap in _nav.scss. To avoid this repetition, please define a variable for it here (e.g., $spacing-md: 20px;).

Additionally, _header.scss uses a hardcoded 40px padding, for which you already have the $catalog-padding-h variable.

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

+
+  &__price {
+    font-weight: 700;
+    color: $color-nav-text;


It seems there's a typo in the variable name here. The variable $color-nav-text is not defined in your _variables.scss file. I believe you intended to use $color-text-nav.

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

+              Apple
+            </a>
+          </li>
+


It seems there's a small typo in this variable name. According to _variables.scss, it should be $color-text-nav.

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

+  gap: 4px;
+  font-size: 14px;


This is a good implementation. To fully comply with checklist item #2 ('Repeated sizes and special colors are put to variables'), please avoid using hardcoded 'magic numbers' for sizes.

The gap: 4px; uses a value that is also present in _card.scss (border-radius: 4px;). Please create a variable for this repeated size in _variables.scss (e.g., $spacing-xs: 4px;) and use it in both places.

It would also be good practice to create a variable for the font-size: 14px;.

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

@@ -0,0 +1,3 @@
+.header {
+  padding: 20px 40px;


To comply with checklist item #2, hardcoded values should be avoided.

You already have a variable $catalog-padding-h: 40px;. Please use it here for the horizontal padding.

The 20px value is also used in _nav.scss for the gap. Please create a new variable for this repeated size (e.g., $spacing-md: 20px;) in _variables.scss and use it in both places.

mateacademy-ai-mentor · 2026-03-23T13:12:16Z

+    margin: 0;
+    padding: 0;
+    list-style: none;
+    gap: 20px;


To comply with checklist item #2 ('Repeated sizes and special colors are put to variables'), this hardcoded 20px value should be a variable. This size is also used as padding in the _header.scss file, making it a repeated value. Please define it in _variables.scss (e.g., $spacing-md: 20px;) and use the variable here.

mateacademy-ai-mentor

Review Summary

Excellent progress on this task! You've successfully addressed almost all the feedback, and your code is clean and well-structured.

I am approving your solution as an educational override so you can move forward. However, please note that a few "magic numbers" for sizes and font weights remain in your SCSS files, specifically in src/styles/blocks/_card.scss and src/styles/blocks/_nav.scss. To fully meet the requirements [CHECKLIST ITEM #2], values like padding: 8px 16px and font-weight: 700 should be moved to variables in _variables.scss.

This practice greatly improves code maintainability. Keep up the fantastic 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-03-23T13:21:56Z

+.card {
+  position: relative;
+  width: $card-width;
+  border-radius: $spacing-xs;
+  background-color: $color-card-bg;
+  transition: transform $transition-speed;
+  cursor: pointer;
+
+  &__image {
+    display: block;
+    width: 100%;
+  }
+
+  &__content {
+    padding: 10px;
+  }
+
+  &__title {
+    margin: 10px 0;
+    font-size: 16px;
+    transition: color $transition-speed;
+  }
+
+  &__footer {
+    display: flex;
+    align-items: center;
+    justify-content: space-between;
+    margin-top: 15px;
+  }
+
+  &__price {
+    font-weight: 700;
+    color: $color-text-nav;
+  }
+
+  &__button {
+    padding: 8px 16px;
+    color: $color-white;
+    text-decoration: none;
+    background-color: $color-accent;
+    border: 1px solid $color-accent;
+    border-radius: $spacing-xs;
+    transition:
+      background-color $transition-speed,
+      color $transition-speed;
+
+    &:hover {
+      color: $color-accent;
+      background-color: $color-white;
+    }
+  }
+
+  &:hover {
+    z-index: 10;
+    transform: scale(1.2);
+
+    .card__title {
+      color: $color-title-hover;
+    }
+  }
+}


Great job addressing the previous feedback! To fully satisfy checklist item #2 ('Repeated sizes and special colors are put to variables'), let's also move the remaining hardcoded values in this file to variables.

For example, values like 10px, 16px, 15px, font-weight: 700, and padding: 8px 16px should be defined in _variables.scss to avoid 'magic numbers' and improve maintainability.

mateacademy-ai-mentor · 2026-03-23T13:21:56Z

+  }
+
+  &__link {
+    font-weight: 500;


This looks great! To be fully compliant with checklist item #2, it's a good practice to move typographic values like font-weight into variables. Consider creating a variable like $font-weight-medium: 500; in your _variables.scss file and using it here.

Solution

a3ca1db

mateacademy-ai-mentor suggested changes Mar 23, 2026

View reviewed changes

Fix

0d4141d

KrzywMan requested a review from mateacademy-ai-mentor March 23, 2026 13:09

mateacademy-ai-mentor suggested changes Mar 23, 2026

View reviewed changes

Fix 2

551b2d3

KrzywMan requested a review from mateacademy-ai-mentor March 23, 2026 13:19

mateacademy-ai-mentor approved these changes Mar 23, 2026

View reviewed changes

Conversation

KrzywMan commented Mar 23, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants