Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add task solution #4840

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add task solution #4840

wants to merge 2 commits into from

Conversation

Uusuff
Copy link

@Uusuff Uusuff commented Nov 11, 2024

mateacademy-ai-mentor
mateacademy-ai-mentor suggested changes Nov 11, 2024
View reviewed changes
Copy link

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

Thank you for your hard work on this project! 😊 While your code is well-structured and mostly aligns with the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

  1. Product Card Content: All product cards currently have identical content. This could significantly impact the user experience, as it doesn't accurately represent different products. Please ensure each card has unique content.

  2. Grid Layout for Cards: The grid layout should adjust the number of columns based on specific screen resolutions. Currently, the implementation does not meet these requirements. Please update the grid to reflect the specified breakpoints.

  3. SCSS Variables: Ensure all repeated sizes and colors across the SCSS files are defined as variables for consistency and maintainability.

  4. Smooth Hover Transitions: Make sure all hover effects have a transition duration of 300ms to meet the task requirements.

  5. Typo in SCSS Variable: Correct the typo in the $color-batton variable to $color-button for clarity and consistency.

  6. Selector Issue: Remove the extra space before :nth-child selectors to ensure they work as intended.

Once these issues are resolved, your code will be in great shape! Keep up the excellent work, and feel free to reach out if you have any questions or need further assistance. 🚀

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

src/index.html
Comment on lines +115 to +381
</article>
<article class="card">
<div class="card__image"></div>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h3>

<p class="card__code">Product code: 195434</p>

<div class="card__assessment">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="card__reviews">Reviews: 5</div>
</div>

<div class="card__price">
<span class="card__price-text">Price:</span>
<span class="card__price-cost">$2,199</span>
</div>

<button
class="card__button"
type="button"
data-qa="card-hover"
>
Buy
</button>
</article>
<article class="card">
<div class="card__image"></div>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h3>

<p class="card__code">Product code: 195434</p>

<div class="card__assessment">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="card__reviews">Reviews: 5</div>
</div>

<div class="card__price">
<span class="card__price-text">Price:</span>
<span class="card__price-cost">$2,199</span>
</div>

<button
class="card__button"
type="button"
data-qa="card-hover"
>
Buy
</button>
</article>
<article class="card">
<div class="card__image"></div>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h3>

<p class="card__code">Product code: 195434</p>

<div class="card__assessment">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="card__reviews">Reviews: 5</div>
</div>

<div class="card__price">
<span class="card__price-text">Price:</span>
<span class="card__price-cost">$2,199</span>
</div>

<button
class="card__button"
type="button"
data-qa="card-hover"
>
Buy
</button>
</article>
Copy link

Choose a reason for hiding this comment

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

All product cards have identical content, including the title, product code, reviews, and price. This might not be intentional and could lead to a poor user experience. Consider updating each card with unique content to accurately represent different products.

src/styles/blocks/card-stars.scss Outdated
margin-right: 0;
}

.stars--1 :nth-child(-n + 1) {
Copy link

Choose a reason for hiding this comment

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

There is an extra space before :nth-child(-n + 1), which may cause the selector to not work as intended. Remove the space to ensure the correct elements are targeted.

src/styles/blocks/card-stars.scss Outdated
background-image: url(../images/star-active.svg);
}

.stars--2 :nth-child(-n + 2) {
Copy link

Choose a reason for hiding this comment

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

There is an extra space before :nth-child(-n + 2), which may cause the selector to not work as intended. Remove the space to ensure the correct elements are targeted.

src/styles/blocks/card-stars.scss Outdated
background-image: url(../images/star-active.svg);
}

.stars--3 :nth-child(-n + 3) {
Copy link

Choose a reason for hiding this comment

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

There is an extra space before :nth-child(-n + 3), which may cause the selector to not work as intended. Remove the space to ensure the correct elements are targeted.

src/styles/blocks/card-stars.scss Outdated
background-image: url(../images/star-active.svg);
}

.stars--4 :nth-child(-n + 4) {
Copy link

Choose a reason for hiding this comment

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

There is an extra space before :nth-child(-n + 4), which may cause the selector to not work as intended. Remove the space to ensure the correct elements are targeted.

src/styles/blocks/card-stars.scss Outdated
background-image: url(../images/star-active.svg);
}

.stars--5 :nth-child(-n + 5) {
Copy link

Choose a reason for hiding this comment

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

There is an extra space before :nth-child(-n + 5), which may cause the selector to not work as intended. Remove the space to ensure the correct elements are targeted.

src/styles/blocks/card-variables.scss Outdated
@@ -0,0 +1,5 @@
$main-gray: #616070;
$color-batton: #00acdc;
Copy link

Choose a reason for hiding this comment

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

The variable name $color-batton seems to be a typo. It should likely be $color-button to accurately describe its purpose and maintain consistency in naming conventions.

src/styles/blocks/card.scss Outdated
font-weight: 700;
font-size: 14px;
line-height: 16px;
background-color: $color-batton;
Copy link

Choose a reason for hiding this comment

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

The variable $color-batton appears to be a typo. It should likely be $color-button to accurately describe its purpose and maintain consistency in naming conventions.

src/styles/blocks/card.scss Outdated
line-height: 16px;
background-color: $color-batton;
border-radius: 5px;
border: 1px solid $color-batton;
Copy link

Choose a reason for hiding this comment

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

The variable $color-batton appears to be a typo. It should likely be $color-button to accurately describe its purpose and maintain consistency in naming conventions.

src/styles/blocks/card.scss Outdated

.card__button:hover {
background-color: $main-white-color;
color: $color-batton;
Copy link

Choose a reason for hiding this comment

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

The variable $color-batton appears to be a typo. It should likely be $color-button to accurately describe its purpose and maintain consistency in naming conventions.

mateacademy-ai-mentor
mateacademy-ai-mentor approved these changes Nov 11, 2024
View reviewed changes
Copy link

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

Great effort on your submission! 🎉 While there are some areas that need improvement, such as ensuring unique content for each product card and adjusting the grid layout for different screen sizes, your work shows a solid understanding of SCSS and HTML. The use of variables for colors and sizes is commendable, and the hover transitions are well-implemented. Please review the comments to refine your code further. Addressing these issues will enhance the functionality and user experience of your project. Keep up the good work and continue learning from these experiences! 🚀

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

src/index.html
Comment on lines +115 to +381
</article>
<article class="card">
<div class="card__image"></div>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h3>

<p class="card__code">Product code: 195434</p>

<div class="card__assessment">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="card__reviews">Reviews: 5</div>
</div>

<div class="card__price">
<span class="card__price-text">Price:</span>
<span class="card__price-cost">$2,199</span>
</div>

<button
class="card__button"
type="button"
data-qa="card-hover"
>
Buy
</button>
</article>
<article class="card">
<div class="card__image"></div>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h3>

<p class="card__code">Product code: 195434</p>

<div class="card__assessment">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="card__reviews">Reviews: 5</div>
</div>

<div class="card__price">
<span class="card__price-text">Price:</span>
<span class="card__price-cost">$2,199</span>
</div>

<button
class="card__button"
type="button"
data-qa="card-hover"
>
Buy
</button>
</article>
<article class="card">
<div class="card__image"></div>

<h3 class="card__title">
APPLE A1419 iMac 27" Retina 5K Monoblock (MNED2UA/A)
</h3>

<p class="card__code">Product code: 195434</p>

<div class="card__assessment">
<div class="stars stars--4">
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
<div class="stars__star"></div>
</div>
<div class="card__reviews">Reviews: 5</div>
</div>

<div class="card__price">
<span class="card__price-text">Price:</span>
<span class="card__price-cost">$2,199</span>
</div>

<button
class="card__button"
type="button"
data-qa="card-hover"
>
Buy
</button>
</article>
Copy link

Choose a reason for hiding this comment

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

All product cards have identical content. Ensure each card has unique content, such as different product names, codes, prices, and reviews, to improve user experience and meet the task requirements.

src/styles/blocks/card-stars.scss
Comment on lines +19 to +37
.stars--1:nth-child(-n + 1) {
background-image: url(../images/star-active.svg);
}

.stars--2:nth-child(-n + 2) {
background-image: url(../images/star-active.svg);
}

.stars--3:nth-child(-n + 3) {
background-image: url(../images/star-active.svg);
}

.stars--4:nth-child(-n + 4) {
background-image: url(../images/star-active.svg);
}

.stars--5:nth-child(-n + 5) {
background-image: url(../images/star-active.svg);
}
Copy link

Choose a reason for hiding this comment

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

The :nth-child selector is used incorrectly here. The :nth-child selector applies styles to the children of an element, but it seems you want to apply styles to the first few stars based on the rating. Consider using a different approach, such as applying a class to the stars that should be active.

src/styles/blocks/card.scss
Comment on lines +104 to +107
.card__button:hover {
background-color: $main-white-color;
color: $color-button;
}
Copy link

Choose a reason for hiding this comment

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

Ensure that the hover effect on the .card__button also has a transition duration of 300ms to maintain consistency with other hover effects.

src/styles/blocks/catalog.scss
Comment on lines +3 to +4
display: grid;
grid-template-columns: repeat(auto-fill, minmax(200px, 200px));
Copy link

Choose a reason for hiding this comment

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

The grid layout should adjust the number of columns based on specific screen resolutions. Consider using media queries to define different grid-template-columns for various breakpoints to meet the task requirements.

src/styles/blocks/header.scss
Comment on lines +56 to +58
.is-active {
color: #00acdc;
}
Copy link

Choose a reason for hiding this comment

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

Remove the extra space before the :nth-child selectors to ensure they work as intended. This aligns with the previous feedback regarding selector issues.

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

@mateacademy-ai-mentor mateacademy-ai-mentor mateacademy-ai-mentor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Uusuff @mateacademy-ai-mentor