Skip to content

add task solution#6230

Open
Ruslan95fs wants to merge 1 commit into
mate-academy:masterfrom
Ruslan95fs:develop
Open

add task solution#6230
Ruslan95fs wants to merge 1 commit into
mate-academy:masterfrom
Ruslan95fs:develop

Conversation

@Ruslan95fs
Copy link
Copy Markdown

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

Review Summary

The implementation meets all core functional requirements and is functionally complete. The BEM methodology issues identified are style and methodology improvements rather than functional blockers, so the code can be approved as is.


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

Footnotes

  1. Rate AI review example

Comment on lines +59 to +61
}

&__rating {
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 global a:hover selector is outside the BEM block structure and can affect all anchor elements in the document. Consider moving this hover behavior inside the .nav__link block as &:hover to maintain proper BEM methodology and avoid unintended side effects on other links.

Comment thread src/index.html
Comment on lines +1 to 2
<!-- #region head -->
<!doctype html>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Redundant box-sizing: border-box declaration - this property is already set globally in index.scss for all elements


&.is-active::after {
content: '';
position: absolute;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hardcoded color #34568b should be moved to a variable for consistency with other colors.

color: #00acdc;
}
}

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 global selector a:hover violates BEM methodology. BEM requires all styles to be scoped within block classes. Consider using .nav__link:hover instead, or add :not(.is-active) to exclude the active state from hover color change.

Comment thread src/styles/variables.scss
Comment on lines +1 to +2
$width-card: 200px;
$radius-card: 5px;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Redundant box-sizing: border-box - this property is already set globally in index.scss for all elements via the * selector. This declaration in card.scss is unnecessary.

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.

2 participants