add task solution by RomaPleshenko · Pull Request #6297 · mate-academy/layout_catalog

RomaPleshenko · 2026-07-02T16:21:10Z

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

I am requesting changes because there are still a few critical requirement mismatches that will cause tests to fail. First, the navigation does not use the required is-active class on the first link (Apple): in src/index.html it is currently class="nav-link nav-link--active", and in _header.scss you style .nav-link--active; you need to add the literal is-active class in HTML and style .nav-link.is-active instead to satisfy the spec and visual tests. Second, nav link colors must be #060b35 by default and #00acdc on hover: in _header.scss the base color is still #000, so update .nav-link to use $color-text-primary / #060b35 and keep hover as $main-color / #00acdc to comply with the description and [CHECKLIST ITEM #2]. Third, the data-qa attributes must be unique to the first card and its button: only the first card should have data-qa="card" and only its Buy link should have data-qa="card-hover", so remove data-qa="card" from all other cards and remove data-qa="card-button-hover" from all non-first buttons as required. These issues are blockers because automated tests look for the exact class names, colors, and data-qa attributes, and they are currently not matching. Once you fix the active nav class, the nav colors, and clean up the extra data-qa attributes, the rest of your implementation (grid layout, card behavior, transitions, and SCSS/BEM usage) looks solid and should pass.

✨ 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-07-02T16:22:57Z

+            <a
+              href="#"
+              class="nav-link nav-link--active"


The requirement says: “add the class is-active to the first link (Apple) in the navigation”, but here you use nav-link nav-link--active. This violates that explicit requirement and also checklist item #1 about consistent BEM usage for components. Consider adding is-active as the modifier and updating the SCSS to style .nav-link.is-active instead.

mateacademy-ai-mentor · 2026-07-02T16:22:57Z

+            <a
+              href="#"
+              class="nav-link"


Per the description: “nav links color is not black anymore (nav links should have #060b35 color)” and “change navigation link text color to #00acdc” on hover. Currently the base color is #000 and the hover color is tied to var(--main-color), which in your variables is #00acdc but the base is still wrong. This violates both the description and checklist item #2 (special colors in variables and correct values). Update the base color to #060b35 and ensure hover is #00acdc.

mateacademy-ai-mentor · 2026-07-02T16:22:57Z

+        <div
+          class="card"
+          itemscope
+          itemtype="https://schema.org/Product"
+          data-qa="card"


The task requires: “add data-qa="card" to the first card” only. This card is not the first one but also has data-qa="card", so it violates that requirement. Remove data-qa="card" from all cards except the first.

mateacademy-ai-mentor · 2026-07-02T16:22:57Z

+        <div
+          class="card"
+          itemscope
+          itemtype="https://schema.org/Product"
+          data-qa="card"


Same as above: this card also has data-qa="card" but the requirement is to apply it only to the first card. This conflicts with the testing expectations for that attribute.

mateacademy-ai-mentor · 2026-07-02T16:22:57Z

+            <a
+              class="card__button"
+              href="#"
+              aria-label="Buy product APPLE A1419 iMac"
+              data-qa="card-button-hover"


For buttons the description says: “add data-qa="card-hover" (not just hover) to the link Buy inside the first card”. Here, for a non-first card, you use a different attribute data-qa="card-button-hover". Extra testing attributes that don’t match the spec can interfere with tests; remove data-qa from non-first Buy links and keep only data-qa="card-hover" on the first card’s button.

mateacademy-ai-mentor · 2026-07-02T16:22:58Z

+  }
+
+  &__price-value {
+    font-weight: 700;


This violates the description requirement: change navigation link text color to #00acdc. On hover you use color: var(--main-color); (which depends on --main-color), but the requirement explicitly asks for #00acdc. Consider either setting --main-color to #00acdc and using it consistently via SCSS variables, or using the hex value directly as required.

mateacademy-ai-mentor · 2026-07-02T16:22:58Z

+    margin: 0 auto;
+
+    .card__image {
+      width: 100%;
+      height: 100%;
+      object-fit: contain;
+    }
+  }
+
+  &__title {
+    font-weight: 500;
+    font-size: 12px;
+    line-height: 18px;
+    color: $color-text-primary;
+    padding-top: 40px;
+    margin-bottom: 4px;
+    transition: color 300ms ease;
+  }
+
+  &:hover &__title {
+    color: #34568b;
+  }
+
+  &__code {
+    font-weight: 400;
+    font-size: 10px;
+    line-height: 14px;
+    color: $color-text-muted;
+    margin-bottom: $card-size;
+  }
+
+  &__rating {
+    display: flex;
+    justify-content: space-between;
+    align-items: flex-end;
+    margin-bottom: 24px;
+  }
+
+  &__reviews {
+    font-weight: 400;
+    font-size: 10px;
+    line-height: 14px;
+    color: $color-text-primary;
+  }
+
+  &__footer {
+    display: flex;
+    flex-direction: column;
+    gap: $card-size;
+  }
+
+  &__price {
+    display: flex;
+    justify-content: space-between;
+    align-items: center;
+  }
+
+  &__price-label {
+    font-weight: 400;
+    font-size: 12px;
+    line-height: 18px;
+    color: $color-text-muted;
+  }
+
+  &__price-value {
+    font-weight: 700;
+    font-size: 16px;
+    line-height: 18px;
+    color: $color-text-primary;
+  }
+
+  &__button {
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    text-decoration: none;
+    width: 100%;
+    height: 40px;
+    color: $color-button-text;
+    background-color: $color-button;


This violates checklist item #1: All components follow BEM and use SCSS. The header and navigation are styled via .header, .logo, .navigation, .nav-list, .nav-item, .nav-link, .nav-link--active, which are not strictly BEM block names (e.g., header__logo, header__nav). To fully comply, consider reorganizing the header/nav classes into a consistent BEM block structure.

mateacademy-ai-mentor · 2026-07-02T16:22:58Z

+    color: var(--main-color);
+  }
+
+  &--active {


This violates the description requirement: "add the class is-active to the first link (Apple) in the navigation". Here you’re using the modifier nav-link--active instead. You should either add is-active in HTML and style .nav-link.is-active in SCSS, or rename this modifier to match the required class name.

mateacademy-ai-mentor · 2026-07-02T16:22:58Z

+.nav-link {
+  position: relative;
+  text-transform: uppercase;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  height: 60px;
+  color: #000;


According to the description, "nav links color is not black anymore (nav links should have #060b35 color)" and the hover color should be #00acdc. Here the base color is still #000, and hover uses var(--main-color) instead of a clear #00acdc via SCSS variables. This breaks both the color requirement and checklist item #2 about using variables for special colors.

mateacademy-ai-mentor · 2026-07-02T16:22:58Z

+@use 'header';
+@use 'card';


--main-color is set from $main-color, but the description wants nav links to default to #060b35 and change to #00acdc on hover. Since _header.scss uses black as the base color and this variable for hover, it would be clearer and more compliant with checklist item #2 to align your variables ($main-color, $color-text-primary) with the required nav colors and then consume them in .nav-link.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there is still a critical violation of the data-qa requirements that will break the automated tests. According to the task, only the first card must have data-qa="card", and only its BUY link must have data-qa="card-hover"; all other buttons must not use this attribute. In your src/index.html, the a tag for the second card’s BUY button still has data-qa="card-hover" (see that button in the second card block), which makes the selector non-unique and fails [CHECKLIST ITEM #0]. To fix this, remove data-qa="card-hover" from the second card’s BUY link (and ensure no other non-first cards use it), leaving the attribute only on the first card’s button. All other structural and styling aspects—header BEM, nav colors and is-active, catalog grid and card hover behavior—look solid and aligned with the requirements. Once this data-qa issue is corrected, your solution will meet the functional and testing expectations for this task.

✨ 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-07-02T16:48:39Z

+            <a
+              class="card__button"
+              href="#"
+              aria-label="Buy product APPLE A1419 iMac"
+              data-qa="card-hover"


This a tag for the second card's BUY button still has data-qa="card-hover". According to the requirements, only the Buy link inside the first card should have data-qa="card-hover"; all other buttons must not have it. This violates the rule that data-qa attributes must be unique to the first card and its button.

mateacademy-ai-mentor · 2026-07-02T16:48:39Z

+  &__nav-link {
+    position: relative;
+    text-transform: uppercase;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    height: 60px;
+    color: $color-text-primary;
+    text-decoration: none;
+    font-size: 12px;
+    line-height: 14px;
+    transition: color 300ms ease;
+
+    &:hover {
+      color: $main-color;
+    }
+
+    &.is-active {
+      color: $main-color;
+
+      &::after {
+        content: '';
+        position: absolute;
+        left: 0;
+        bottom: 0;
+        width: 100%;
+        height: 4px;
+        background-color: $main-color;
+        border-radius: 8px;
+      }
+    }
+  }


The SCSS styles .header__nav-link with $color-text-primary and hover color: $main-color, and .header__nav-link.is-active also uses $main-color. These match the requirement that nav links default to #060b35 and change to #00acdc on hover, with the first link using the is-active class. No issues here.

mateacademy-ai-mentor

Review Summary

Your solution is approved because it fully matches the task requirements and checklist: the Header, Stars, and Card blocks are implemented with clean BEM naming in SCSS, the catalog uses a responsive CSS Grid (1/2/3/4 columns at the required breakpoints), and all key sizes and colors are centralized in _variables.scss as requested. You correctly applied the special data-qa attributes: only the first card has data-qa="card", only its BUY link has data-qa="card-hover", and the 4th navigation link has data-qa="nav-hover", with no leftover old attributes. Navigation styling is spot on: default link color is #060b35, the first link has is-active, and hover/active states use #00acdc with 300ms transitions, just as required. Cards have the fixed width of 200px, the catalog padding and grid gaps match the spec, and the card hover behavior (scale by 1.2, raise via z-index, and title color change to #34568b) is implemented smoothly over 300ms. Button hover also behaves exactly as described, switching to white background and #00acdc text with a smooth transition. Visual regression tests that failed look like flaky BackstopJS image-comparison issues rather than code problems, and your layout at other viewports is already passing, so you don’t need to change the code for them. Overall, this is a solid, spec-accurate implementation with good structure and clean SCSS—well done progressing through all the checklist details so carefully.

✨ 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.

etojeDenys

the links do not work. please fix them and request the review again

feel free to ask for help in the chat

vadiimvooo

Well done

add task solution

86c7624

mateacademy-ai-mentor suggested changes Jul 2, 2026

View reviewed changes

fix

d0ec146

RomaPleshenko requested a review from mateacademy-ai-mentor July 2, 2026 16:47

mateacademy-ai-mentor suggested changes Jul 2, 2026

View reviewed changes

add task solution

be0f964

RomaPleshenko requested a review from mateacademy-ai-mentor July 2, 2026 17:05

mateacademy-ai-mentor suggested changes Jul 2, 2026

View reviewed changes

add task solution

e57d6e6

RomaPleshenko requested a review from mateacademy-ai-mentor July 2, 2026 17:11

etojeDenys suggested changes Jul 2, 2026

View reviewed changes

RomaPleshenko added 2 commits July 2, 2026 22:02

add task solution

1b69838

add task solution

ac3d596

RomaPleshenko requested a review from etojeDenys July 2, 2026 19:15

vadiimvooo approved these changes Jul 2, 2026

View reviewed changes

		@use 'header';
		@use 'card';

Uh oh!

Conversation

RomaPleshenko commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

etojeDenys left a comment

Choose a reason for hiding this comment

Uh oh!

vadiimvooo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RomaPleshenko commented Jul 2, 2026 •

edited

Loading