Skip to content

Develop#6255

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

Develop#6255
Oleksandr-Piechieniev wants to merge 4 commits into
mate-academy:masterfrom
Oleksandr-Piechieniev:develop

Conversation

@Oleksandr-Piechieniev

Copy link
Copy Markdown

@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

Your implementation has three critical blockers that must be fixed before approval:

  1. Missing <link> tag in HTML - The task explicitly requires linking SCSS files directly via <link> tags using Parcel. Add <link rel="stylesheet" href="./styles/index.scss"> in the <head> section of src/index.html. Without this, no styles will load.

  2. Incorrect import path in src/styles/index.scss - You're importing './utils/variables' but the file _variables.scss is located directly in src/styles/. Change to @import './variables';.

  3. Empty SCSS partial files causing lint failures - The files _card.scss, _header.scss, and _stars.scss are empty, triggering the no-empty-source stylelint rule which blocks the build. Either populate them with component styles or remove them if they're not needed.

Note: The BEM naming, data-qa attributes, and HTML structure are correctly implemented. Focus on fixing these connection issues between files and the build will pass.


✨ 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
Comment on lines 1 to 6
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta
name="viewport"
content="width=device-width, initial-scale=1.0"
/>
<meta charset="utf-8" />
<title>Catalog</title>
<link
rel="preconnect"
href="https://fonts.gstatic.com"
/>
<link
href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;500;700&display=swap"
rel="stylesheet"
/>
<link
rel="stylesheet"
href="styles/index.scss"
/>
</head>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing <link> tag for SCSS stylesheet. Add <link rel="stylesheet" href="./styles/index.scss"> in the <head> section to link the SCSS file as required by the task.

Comment thread src/index.html
@@ -1,27 +1,433 @@
<!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.

The import path ./utils/variables is incorrect. The _variables.scss file is located directly in src/styles/, not in a utils subdirectory. Change to @import './variables';

Comment thread src/styles/index.scss Outdated
@import './card';
@import './stars';
@import './header';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing required <link> tag for SCSS stylesheet. The task requires linking SCSS files directly in HTML using Parcel: <link rel="stylesheet" href="./styles/index.scss"> should be added in the <head> section.

Comment thread src/styles/index.scss Outdated
@@ -1,3 +1,13 @@
@import './utils/variables';

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 variables file is at _variables.scss but index.scss imports from './utils/variables'. This path won't resolve correctly - either move the variables file to a utils/ subdirectory or update the import path to './variables'.

Comment thread src/styles/_variables.scss Outdated
$card-width: 200px;
$card-border-color: #f3f3f3;
$main-text-color: #060b35;
$secondary-text-color: #616070;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing <link> tag to SCSS stylesheet. According to the task requirements, SCSS files should be linked directly in HTML using Parcel library. Add <link rel="stylesheet" href="./styles/index.scss"> in the <head> section.

Comment thread src/styles/_variables.scss Outdated
Comment on lines +1 to +4
$main-font: 'Roboto', sans-serif;
$card-width: 200px;
$card-border-color: #f3f3f3;
$main-text-color: #060b35;

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 SCSS imports ./card, ./stars, ./header but the variables are imported from ./utils/variables. This path inconsistency means these files either don't exist or are in the wrong location. Either move card.scss, stars.scss, and header.scss to the utils folder, or update the import paths to match.

@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

This implementation has two critical issues that must be fixed before approval. First, the stars component has BEM naming violations throughout stars.scss, page.scss, and moyohead.scss — the &-review syntax generates .stars-stars-review instead of the proper BEM element .stars__review, and the selectors don't match the HTML classes .stars-review and .stars-review__star. This means your star styles won't apply to the HTML elements at all. Second, the checklist explicitly requires "repeated sizes and special colors are put to variables," but no _variables.scss file exists — colors like #00acdc, #060b35, #34568b, and the transition duration 300ms are hardcoded throughout all SCSS files instead of being consolidated into variables in :root and imported via index.scss. To fix these issues: 1. BEM violations in stars component — Change &-review to &__review, &-review__star to &__star, and &-review--4 to &--4 inside the star element, then update the HTML classes from .stars-review to .stars__review and .stars-review__star to .stars__star 2. Create _variables.scss — Define all shared values in :root (colors, transitions, spacing) and import it in index.scss to make variables available across all SCSS files


✨ 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/styles/index.scss
body {
margin: 0;
}
@import 'page';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CSS variables defined but not used. Colors should use these variables throughout the codebase instead of hardcoded values

Comment thread src/styles/index.scss
@import 'page';
@import 'card';
@import 'stars';
@import 'moyohead';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gap values should use CSS variables instead of hardcoded values for consistency with requirements

Comment thread src/styles/moyohead.scss
}

.header-site {
display: flex;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BEM violation: &-review generates invalid class .stars-stars-review. Should be &__review to create proper BEM element .stars__review. Current SCSS nesting produces wrong class names that don't match the HTML.

Comment thread src/styles/moyohead.scss
.nav {
margin-left: auto;
margin-right: 50px;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BEM violation: &-review__star generates invalid class name. Should be &__star (element of stars__review) or a proper BEM hierarchy.

Comment thread src/styles/moyohead.scss
transition: color 300ms;
}

.nav__list {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BEM violation: This selector structure is incorrect. &-review--4 &-review__star should be structured as proper BEM modifier on element.

Comment thread src/styles/page.scss
@@ -0,0 +1,12 @@
html {
font-family: Roboto, sans-serif;
color: #060b35;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Task requires transition-duration: 300ms but CSS variable --transition-duration is not defined or used.

Comment thread src/styles/stars.scss
align-items: center;
margin-top: 16px;
justify-content: space-between;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BEM violation: &-review generates .stars-stars-review but HTML uses .stars-review. Change to &__review for proper BEM element .stars__review and update HTML class accordingly.

Comment thread src/styles/stars.scss
margin-left: 16px;
gap: 4px;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BEM violation: &-review__star generates .stars-stars-review__star. Should be &__star for proper BEM element. Update HTML class from .stars-review__star to .stars__star.

Comment thread src/styles/stars.scss
background-position: center;
background-image: url(../images/star.svg);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BEM violation: &-review--4 generates .stars-stars-review--4. For a modifier, this should be &--4 inside &__star or use proper BEM pattern like .stars__star--active.

Comment thread src/styles/stars.scss
Comment on lines +1 to +5
.stars {
display: flex;
flex-direction: row;
align-items: center;
margin-top: 16px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No _variables.scss file exists. Colors like #00acdc and #060b35, transition duration 300ms, and sizes are repeated across files without variables as required by the checklist.

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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