Skip to content

add task solution#6289

Open
diiyavol wants to merge 4 commits into
mate-academy:masterfrom
diiyavol:develop
Open

add task solution#6289
diiyavol wants to merge 4 commits into
mate-academy:masterfrom
diiyavol:develop

Conversation

@diiyavol

@diiyavol diiyavol commented Jun 28, 2026

Copy link
Copy Markdown
  • 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 mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because some core task requirements around hover behavior are not yet met, which makes this a blocking issue. The description and checklist explicitly require that all changes on :hover are smooth and use 300ms transitions [CHECKLIST ITEM #5], but your .menu__link currently uses transition: color 0.2s ease; (200ms), and the card button hover (.card__button) has no transition at all, so its color, background, and border change abruptly. To fix this, update the nav link transition in both src/styles/header.scss and src/styles/index.scss to 300ms and ensure it covers the hovered color property (e.g. transition: color 300ms ease;). Then, in the card styles, add a transition on .card__button for color, background-color, and border-color with a 300ms duration so the hover effect becomes smooth as required. As a minor improvement (not blocking), consider moving repeated/special colors like #34568b and #f3f3f3 into variables.scss to better align with “Repeated sizes and special colors are put to variables” [CHECKLIST ITEM #2]. Overall, your structure, grid layout, data attributes, and most styles look solid—once you align these transitions to 300ms, your solution should fully satisfy the task and checklist.


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/index.html
class="menu__link"
>
gadgets
</a>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #5: "All changes on :hover are smooth" and the description requirement "Make all the changes smooth on hover (during 300ms)". The menu links use transition: color 0.2s ease; which is 200ms, not 300ms. Update this transition to 300ms, and ensure it covers the color change on hover.

Comment thread src/index.html
Comment on lines +70 to +72
</li>
<li class="menu__item">
<a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #5: "All changes on :hover are smooth". The card title color changes on hover but .card__title does not have a transition property, so the color change is instantaneous rather than smooth over 300ms. Add a transition: color 300ms; (or similar) to &__title to make this hover change smooth.

Comment thread src/index.html
Comment on lines +91 to +109
>
photo
</a>
</li>
<li class="menu__item">
<a
href="#"
class="menu__link"
>
video
</a>
</li>
</ul>
</nav>
</header>
<main>
<div
class="card"
data-qa="card"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #5: "All changes on :hover are smooth". The button hover changes (text color, background color, border) do not have an explicit transition defined, so they will change instantly. Add a transition (e.g. transition: color 300ms, background-color 300ms, border-color 300ms;) to &__button to make all hover changes smooth over 300ms.

Comment thread src/styles/header.scss Outdated
align-items: center;
height: 100%;
position: relative;
transition: color 0.2s ease;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #5: All changes on :hover are smooth and the description requirement Make all the changes smooth on hover (during 300ms). Here the nav link transition is 0.2s instead of 300ms; adjust to 300ms and ensure it covers all hovered properties (e.g. color).

Comment thread src/styles/header.scss
height: 60px;
margin-right: 50px;
margin-left: auto;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To better satisfy checklist item Repeated sizes and special colors are put to variables, consider moving the repeated border color #f3f3f3 to a SCSS variable instead of hardcoding it here.

Comment thread src/styles/header.scss
Comment on lines +75 to +80
height: 100%;
position: relative;
transition: color 0.2s ease;
}

&__link:hover {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says Make all the changes smooth on hover (during 300ms). Here you use transition: transform 300ms; which is correct for the scale, but the card title color change relies on this selector; if you decide to animate other properties on hover later, ensure they also use 300ms transitions (possibly by using transition on individual properties).

Comment thread src/styles/index.scss

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checklist item #5 requires: "All changes on :hover are smooth" and the description specifies 300ms, but here you use transition: color 0.2s ease;. Update the duration to 300ms and ensure it covers hover color changes for the nav links.

Comment thread src/styles/index.scss Outdated
Comment on lines +75 to +76
&:hover &__title {
color: #34568b;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hover changes the card title color to #34568b as required, but the color is hard-coded. Checklist item #2 says "Repeated sizes and special colors are put to variables" – consider moving this color into variables.scss and using a variable here.

Comment thread src/styles/index.scss
Comment on lines +91 to +115
&__button {
background-color: $blue-accent;
border-radius: 5px;
width: 166px;
height: 40px;
display: flex;
justify-content: space-evenly;
margin-top: 16px;
margin-bottom: 16px;

align-items: center;

color: #fff;
text-decoration: none;
font-family: Roboto, sans-serif;
font-weight: 700;
font-size: 14px;
line-height: 16px;
text-transform: uppercase;
&:hover {
color: $blue-accent;
background-color: #fff;
border: 1px $blue-accent solid;
box-sizing: border-box;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checklist item #5: "All changes on :hover are smooth" – this button has hover state but no transition, so its color/background/border changes are abrupt. Add a transition (300ms as per description) on the base .card__button to animate these properties smoothly.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because there are still critical checklist violations that block full task completion. Specifically, [CHECKLIST ITEM #5] requires all :hover changes to be smooth: the .card__button transition is currently defined only inside the :hover block and not on the base button, and it doesn’t explicitly target color, background-color, and border-color; this must be moved to the base .card__button rule so hover in both directions animates smoothly. In addition, [CHECKLIST ITEM #2] requires repeated or special colors to be extracted into variables: the card title hover color #34568b and the border color #f3f3f3 are hardcoded in multiple places and need to be moved into variables.scss (for example, $card-title-hover and $border-light) and then referenced via those variables in your SCSS.

To fix this, update your button styles so the default .card__button includes transition: color 300ms, background-color 300ms, border-color 300ms;, and keep the :hover rule focused only on changing the property values. Then, define new variables for #34568b and #f3f3f3 in src/styles/variables.scss and replace all direct occurrences of those colors in your header and card styles with the new variables. Everything else in your layout, grid, BEM structure, and most hover effects looks solid and in line with the requirements, so once you address these specific points your solution should be ready to pass. Keep going—you’re very close to a fully compliant implementation.


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/index.html
<div class="card">
<div class="card__image"></div>
<div class="card__info">
<div class="card__title">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #2: "Repeated sizes and special colors are put to variables". The card title hover color #34568b is a special color and should be moved into variables.scss and referenced as a variable here.

Comment thread src/index.html
>
<img
src="images/logo.png"
alt="logo"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #2: "Repeated sizes and special colors are put to variables". The border color #f3f3f3 is a special color reused in the header and cards, so it should be extracted to variables.scss and used via a variable.

Comment thread src/index.html
Comment on lines +286 to +300
<div class="card">
<div class="card__image"></div>
<div class="card__info">
<div class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</div>
<div class="card__code">Product code: 195434</div>
</div>
<div class="card__rating rating">
<div class="rating__stars rating__stars--4">
<div class="rating__star"></div>
<div class="rating__star"></div>
<div class="rating__star"></div>
<div class="rating__star"></div>
<div class="rating__star"></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #5: "All changes on :hover are smooth". The transition is declared inside the &:hover block and not tied to specific properties; move transition: color 300ms, background-color 300ms, border-color 300ms; to the base .card__button rule so the hover changes animate smoothly.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution as an educational override so you can keep progressing while learning. Overall, you did an excellent job: the HTML structure is semantic, your BEM naming is consistent, the responsive grid with 1/2/3/4 columns is correctly implemented, and the hover effects and transitions look smooth and well thought out. Your use of SCSS variables for repeated colors and sizes is strong and shows good attention to maintainability.

There is one point to improve for future iterations: in src/styles/header.scss, the header’s bottom border still uses the hardcoded color #f3f3f3 instead of the existing SCSS variable (for example, the same color you used for card borders). This goes against the requirement to move repeated and special colors into variables [CHECKLIST ITEM #2]; updating that border to use the shared variable will make your styles more consistent and easier to change later. Apart from that, your implementation aligns very well with the catalog layout requirements and the failed build is only due to visual regression baseline differences, not code issues. Keep up this level of care in structure and styling—you're very close to a fully polished, production-ready solution.


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

@Denys-Kravchuk9988 Denys-Kravchuk9988 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants